[NEW PATCH] BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown (via gerrit-bot)

New patch submitted by Saggi Mizrahi (smizrahi(a)redhat.com)

You can review this change at: http://gerrit.usersys.redhat.com/863

commit b705b2249e4635a1e3f3c4feeede53081c8a6a52
Author: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Date:   Mon Aug 29 16:13:24 2011 +0300

    BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown
    
    This patch touches a protected variable in the python multiprocessing
    framework. This is not ideal but necessary to keep VDSM from getting
    stuck waiting for a child.
    
    Change-Id: I7b96b6ac5b35e28cb2f8232f580651a3a36db295

diff --git a/vdsm/storage/processPool.py b/vdsm/storage/processPool.py
index fc843c4..6c6632d 100644
--- a/vdsm/storage/processPool.py
+++ b/vdsm/storage/processPool.py
@@ -18,7 +18,7 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-from multiprocessing import Pipe, Process
+from multiprocessing import Pipe, Process, current_process
 from threading import Lock
 import os
 import signal
@@ -111,6 +111,11 @@ class ProcessPool(object):
         # The locks remain locked of purpose so no one will
         # be able to run further commands
 
+def disown(proc):
+    # I know touching _children is wrong but there is no public API for
+    # disowning a child
+    current_process()._children.discard(proc)
+
 class Helper(object):
     def __init__(self):
         self.lifeline, childsLifeline = os.pipe()
@@ -118,6 +123,8 @@ class Helper(object):
         self.proc = Process(target=_helperMainLoop, args=(hisPipe, childsLifeline,
self.lifeline))
         self.proc.daemon = True
         self.proc.start()
+        disown(self.proc)
+
         os.close(childsLifeline)
 
     def kill(self):

Change in vdsm[master]: BZ#723579 - Disown child procs so they wouldn't hold VDSM sh...

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown
......................................................................


BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown

This patch touches a protected variable in the python multiprocessing
framework. This is not ideal but necessary to keep VDSM from getting
stuck waiting for a child.

Change-Id: I7b96b6ac5b35e28cb2f8232f580651a3a36db295
---
M vdsm/storage/processPool.py
1 file changed, 8 insertions(+), 1 deletion(-)

Approvals:
  Dan Kenigsberg: Looks good to me, approved
  David Naori: Verified
  Ayal Baron: Looks good to me, but someone else must approve


--
To view, visit http://gerrit.usersys.redhat.com/863
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I7b96b6ac5b35e28cb2f8232f580651a3a36db295
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: David Naori <dnaori(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#723579 - Disown child procs so they wouldn't hold VDSM sh...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown
......................................................................


Patch Set 1: Looks good to me, approved

--
To view, visit http://gerrit.usersys.redhat.com/863
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b96b6ac5b35e28cb2f8232f580651a3a36db295
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: David Naori <dnaori(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#723579 - Disown child procs so they wouldn't hold VDSM sh...

[David Naori]

David Naori has posted comments on this change.

Change subject: BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown
......................................................................


Patch Set 1: Verified

Works for me.

--
To view, visit http://gerrit.usersys.redhat.com/863
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b96b6ac5b35e28cb2f8232f580651a3a36db295
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: David Naori <dnaori(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#723579 - Disown child procs so they wouldn't hold VDSM sh...

Saggi Mizrahi has posted comments on this change.

Change subject: BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown
......................................................................


Patch Set 1:

When vdsm exists an exit hook that is set by multiprocessing is run. It goes through all
the children and joins with them. This is undesired in our case because a child can be
stuck in an unkillalbe sate. Disowning them is the only way for python NOT to block
waiting for them on vdsm shutdown.

--
To view, visit http://gerrit.usersys.redhat.com/863
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b96b6ac5b35e28cb2f8232f580651a3a36db295
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#723579 - Disown child procs so they wouldn't hold VDSM sh...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#723579 - Disown child procs so they wouldn't hold VDSM shutdown
......................................................................


Patch Set 1: I would prefer that you didn't submit this

Please share a bit more information about the bug before going forward with this. Who
blocks exactly?

--
To view, visit http://gerrit.usersys.redhat.com/863
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b96b6ac5b35e28cb2f8232f580651a3a36db295
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;