Change in vdsm[master]: Implement a zombie reaper

peter.portante at redhat.com peter.portante at redhat.com
Wed Oct 31 01:03:22 UTC 2012


Peter Portante has posted comments on this change.

Change subject: Implement a zombie reaper
......................................................................


Patch Set 1: (3 inline comments)

Hi Saggi, this caught my eye. Hope you don't mind an out of band review. This seems like the kind of thing that would have been implemented before. Kudos for doing this.

Also, do you folks use nose at all?

....................................................
File tests/zombieReaperTests.py
Line 24: import zombieReaper
Line 25: from vdsm.betterPopen import BetterPopen
Line 26: 
Line 27: 
Line 28: class zombieReaperTests(TestCaseBase):
Is it worth having a test where one registers a pid to be reaped, and then reap it to see how the reaper handles it? Perhaps it would be nice to show the defined behavior in that case so that folks that make a mistake have expected behavior.
Line 29:     def setUp(self):
Line 30:         zombieReaper.registerSignalHandler()
Line 31: 
Line 32:     def tearDown(self):


....................................................
File vdsm/zombieReaper.py
Line 6: 
Line 7: def autoReapPID(pid):
Line 8:     _trackedPids.add(pid)
Line 9:     # SIGCHLD happend before we added the pid to the set
Line 10:     _tryReap(pid)
Perhaps a unit test for this case?
Line 11: 
Line 12: 
Line 13: def _tryReap(pid):
Line 14:         try:


Line 15:             pid, rv = os.waitpid(pid, os.WNOHANG)
Line 16:             if pid != 0:
Line 17:                 _trackedPids.discard(pid)
Line 18:         except OSError:
Line 19:             _trackedPids.discard(pid)
So are there any errors that could occur where the process could still be reaped later?
Line 20: 
Line 21: 
Line 22: def _zombieReaper(signum, frame):
Line 23:     for pid in _trackedPids.copy():


--
To view, visit http://gerrit.ovirt.org/8937
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0fdf96e195135284b2733b21f611918ffbf69791
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Peter Portante <peter.portante at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>


More information about the vdsm-patches mailing list