deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 41 forks source link

fix(cleaner): builder taking high CPU #283

Closed kmala closed 8 years ago

kmala commented 8 years ago

Summary of Changes

This PR fixes the issue builder taking high CPU by waiting on the channel stream using select.

Issue(s) that this PR Closes

Fixes #280

Testing Instructions

After the fix:

CONTAINER           CPU %               MEM USAGE / LIMIT     MEM %               NET I/O             BLOCK I/O
5be5bad7ce1a        0.32%               22.45 MB / 3.892 GB   0.58%               0 B / 0 B           0 B / 0 B

Testing:

  1. Create a Deis Cluster
  2. Deploy an App
  3. Destroy the App
  4. Check the /home/git/ folder and it shouldn't contain the created app git repo

    Pull Request Hygiene TODOs

Please make sure the below checklist is complete.

arschles commented 8 years ago

@kmala great catch, and I take full responsibility for making the mistake in the first place (doh!) I've marked LGTM, and checked off the boxes above (under Pull Request Hygiene TODOs - in the future, can you check those off when they're done?)

Thanks!

arschles commented 8 years ago

@kmala I see in #280, @MaxenceAdnot says the following: I noticed that a defunct child process linked to boot was consuming a lot of CPU. I agree that this PR cleans up the wait logic a bit, but does it solve the defunct process issue?

arschles commented 8 years ago

@kmala I added a few comments just now, so removed my LGTM. Sorry for the flip-flopping here.

kmala commented 8 years ago

i don't see any defunct process attached to the boot. If you see the issue description in #280 the boot is the one which clearly is taking most of the CPU.

arschles commented 8 years ago

@kmala ok, fair enough. let's keep an eye out for the defunct child process issue then

MaxenceAdnot commented 8 years ago

I'll try to reproduce the case with the defunct child process on my current cluster and I'll let you know. The root cause of these two issues could be different.

EDIT : @arschles @kmala : Here are the defunct child processes I've seen but, my bad, they cannot consume CPU nor RAM by nature and they are probably a kind of trace from a failed push I think.

root     12342 92.9  0.2  23052 18956 ?        Ssl  Mar30 1921:16  \_ boot server
root     25779  0.0  0.0      0     0 ?        Z    Mar30   0:00      \_ [pre-receive] <defunct>
root     26513  0.0  0.0      0     0 ?        Z    Mar30   0:00      \_ [pre-receive] <defunct>
kmala commented 8 years ago

@MaxenceAdnot Yes...your defunct process are from git push failure