12:25 p.m.
Saggi Mizrahi has uploaded a new change for review.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Don't forget to close thread pool when stopping the SPM mailer
Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Signed-off-by: Saggi Mizrahi <smizrahi(a)redhat.com>
---
M vdsm/storage/storage_mailbox.py
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/80/8980/1
diff --git a/vdsm/storage/storage_mailbox.py b/vdsm/storage/storage_mailbox.py
index c537883..041e872 100644
--- a/vdsm/storage/storage_mailbox.py
+++ b/vdsm/storage/storage_mailbox.py
@@ -687,5 +687,6 @@
time.sleep(self._monitorInterval)
finally:
self._stopped = True
+ self._mailman.tp.joinAll(waitForTasks=False)
self.log.info("SPM_MailMonitor - Incoming mail monitoring thread
stopped")
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
5:27 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Don't forget to close thread pool when stopping the SPM mailer
This causes thread leak anytime we call startSpm and an error is raised
Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=872935
Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Signed-off-by: Saggi Mizrahi <smizrahi(a)redhat.com>
---
M tests/Makefile.am
A tests/storageMailboxTests.py
M vdsm/storage/storage_mailbox.py
3 files changed, 62 insertions(+), 1 deletion(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
5:27 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 8: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
4:42 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 7: Fails
why was patcset 7 sent?
make[1]: *** No rule to make target `storageMailBoxTests.py', needed by `distdir'.
Stop.
make[1]: Leaving directory `~/vdsm.git/tests'
make: *** [distdir] Error 1
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
8:46 a.m.
Shu Ming has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 6: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
7:24 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 6: Looks good to me, but someone else must approve
Eduardo, I have no idea which infrastructural solution you are referring to.
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
12:59 p.m.
Saggi Mizrahi has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 5:
No, it's the user's responsibility top shut down the thread pool.
It's a resource and should be managed by the caller
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
3:42 a.m.
Yaniv Bronhaim has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 5: Verified; Looks good to me, but someone else must approve
This fix is part of fixing the use of threadPool to avoid spm mailer thread leak. My
opinion is that we should use this fix and continue to work on fixing other uses of
threadPool that can cause leaks in another patch.
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
3:23 a.m.
Eduardo has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(By Barak request.)
This is not the only point where we can leak threads.
The leak should be avoided as part of the ThreadPool infra.
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
3:05 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 4: (2 inline comments)
....................................................
Commit Message
Line 3: AuthorDate: 2012-11-01 13:25:07 -0400
Line 4: Commit: Saggi Mizrahi <smizrahi(a)redhat.com>
Line 5: CommitDate: 2012-11-01 17:28:04 -0400
Line 6:
Line 7: Don't forget to close thread pool when stopping the SPM mailer
why not explain what forgetting this does to vdsm?
Line 8:
Line 9: Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
....................................................
File tests/storageMailboxTests.py
Line 43: with open(os.path.join(self.__masterDir, fname), "w") as
f:
Line 44: f.write("DATA")
Line 45:
Line 46: def __del__(self):
Line 47: shutil.rmtree(os.path.join(self.storage_repository, self.spUUID))
no need to join - self.storage_repository itself should be dropped, too.
Line 48:
Line 49:
Line 50: class SPM_MailMonitorTests(TestCaseBase):
Line 51: def testThreadLeak(self):
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
3:41 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: Don't forget to close thread pool when stopping the SPM mailer
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(3 inline comments)
....................................................
Commit Message
Line 5: CommitDate: 2012-11-01 14:35:35 -0400
Line 6:
Line 7: Don't forget to close thread pool when stopping the SPM mailer
Line 8:
Line 9: Bug-URL: https://bugzilla.redhat.com/show_bug.cgi?id=870024
this bug seems unrelated.
could you explain the functional effect of the issue you are now solving?
Line 10: Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
....................................................
File tests/storageMailboxTests.py
Line 32:
Line 33: class StoragePoolStub(object):
Line 34: def __init__(self):
Line 35: self.spUUID = str(uuid4())
Line 36: self.storage_repository = "/tmp/fakepool"
would break if Jenkins runs two tests on the same machine. please use tempfile.
Line 37: self.__masterDir = os.path.join(self.storage_repository, self.spUUID,
Line 38: "mastersd", DOMAIN_META_DATA)
Line 39:
Line 40: os.makedirs(self.__masterDir)
Line 45: def __del__(self):
Line 46: shutil.rmtree(os.path.join(self.storage_repository, self.spUUID))
Line 47:
Line 48:
Line 49: class SPM_MailMonitorTests(TestCaseBase):
Hallelujah, it's been anticipated for long!
Line 50: def testThreadLeak(self):
Line 51: mailer = sm.SPM_MailMonitor(StoragePoolStub(), 100)
Line 52: threadCount = len(threading.enumerate())
Line 53: mailer.stop()
--
To view, visit http://gerrit.ovirt.org/8980
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fd696a20254380e0a0067dd33c7709445e38afb
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
4225
days inactive
4230
days old
vdsm-patches@lists.fedorahosted.org
10 comments
5 participants
participants (5)
-
Dan Kenigsberg -
ewarszaw＠redhat.com -
shuming＠linux.vnet.ibm.com -
smizrahi＠redhat.com -
ybronhei＠redhat.com