Change in vdsm[master]: sp: improve domainStateChange event handling
ishaby at redhat.com
ishaby at redhat.com
Wed Jan 6 13:25:26 UTC 2016
Idan Shaby has posted comments on this change.
Change subject: sp: improve domainStateChange event handling
......................................................................
Patch Set 3:
(9 comments)
https://gerrit.ovirt.org/#/c/51393/3//COMMIT_MSG
Commit Message:
Line 8:
Line 9: onDomainStateChange is registered in the constructor of StoragePool.
Line 10: This is a waste of resources since these events can be called only after
Line 11: the connect method is called (specifically, after __rebuild is called
Line 12: from within connect).
> Correction - the main issue is an issue - we do a new registeration for eac
Done
Line 13:
Line 14: This patch moves the registration to onDomainStateChange to be right
Line 15: before it can actually be called (in connect), and calls unregister if
Line 16: an error has occurred in __rebuild.
Line 18: won't be listening to events that cannot be called anymore.
Line 19:
Line 20: In addition, this patch locks the pool and the domain id's in
Line 21: _domainStateChange to prevent concurrent calls to _refreshDomainLinks
Line 22: from other threads.
> We take a shared lock on the pool since we use query pool state, and an exc
Done
Line 23:
Line 24: Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee
https://gerrit.ovirt.org/#/c/51393/3/vdsm/storage/sp.py
File vdsm/storage/sp.py:
Line 640
Line 641
Line 642
Line 643
Line 644
> Add comment about starting monitoring before __rebuild, since it starts (in
Done
Line 641
Line 642
Line 643
Line 644
Line 645
> If this raises, the pool is not considered connected, and we should stop wa
Done
Line 139:
Line 140: def _domainStateChange(self, sdUUID, isValid):
Line 141: if not isValid:
Line 142: return
Line 143:
> Produce the domain here, add comment about producing it before taking locks
Done
Line 144: with rmanager.acquireResource(STORAGE, self.spUUID,
Line 145: rm.LockType.shared):
Line 146: if sdUUID not in self.getDomains():
Line 147: self.log.debug("Domain %s is not a member of pool %s, "
Line 150: return
Line 151: with rmanager.acquireResource(STORAGE, sdUUID,
Line 152: rm.LockType.exclusive):
Line 153: self.log.debug("Refreshing domain links for %s", sdUUID)
Line 154: self._refreshDomainLinks(sdCache.produce(sdUUID))
> We should produce the domain before locking, since it may block for long ti
Done
Line 155:
Line 156: def _upgradePoolDomain(self, sdUUID, isValid):
Line 157: # This method is called everytime the onDomainStateChange
Line 158: # event is emitted, this event is emitted even when a domain goes
Line 663: return True
Line 664:
Line 665: @unsecured
Line 666: def _startWatchingDomainsState(self):
Line 667: self.log.debug("Starting to monitor domains state")
> Starting to monitor -> Start monitoring
Done
Line 668: self.domainMonitor.onDomainStateChange.register(
Line 669: self._domainStateCallback)
Line 670:
Line 671: @unsecured
Line 669: self._domainStateCallback)
Line 670:
Line 671: @unsecured
Line 672: def _stopWatchingDomainsState(self):
Line 673: self.log.debug("Stopping to monitor domains state")
> Stopping to monitor -> Stop monitoring
Done
Line 674: self.domainMonitor.onDomainStateChange.unregister(
Line 675: self._domainStateCallback)
Line 676:
Line 677: @unsecured
Line 671: @unsecured
Line 672: def _stopWatchingDomainsState(self):
Line 673: self.log.debug("Stopping to monitor domains state")
Line 674: self.domainMonitor.onDomainStateChange.unregister(
Line 675: self._domainStateCallback)
> You should handle KeyError here, see _finalizePoolUpgradeIfNeeded, but log
Done
Line 676:
Line 677: @unsecured
Line 678: def stopMonitoringDomains(self):
Line 679: self.domainMonitor.stopMonitoring(self.domainMonitor.poolDomains)
--
To view, visit https://gerrit.ovirt.org/51393
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list