Change in vdsm[master]: [wip] sdcache: avoid extra refresh due samplingmethod

fsimonce at redhat.com fsimonce at redhat.com
Thu Nov 15 18:08:10 UTC 2012


Federico Simoncelli has uploaded a new change for review.

Change subject: [wip] sdcache: avoid extra refresh due samplingmethod
......................................................................

[wip] sdcache: avoid extra refresh due samplingmethod

In order to avoid an extra iscsi rescan (symptomatic of samplingmethod)
an additional lock has been introduced to queue the requests when the
storage is flagged as stale.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=870768
Change-Id: If178a8eaeb94f1dfe9e0957036dde88f6a22829c
Signed-off-by: Federico Simoncelli <fsimonce at redhat.com>
---
M vdsm/storage/sdc.py
1 file changed, 25 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/9274/1

diff --git a/vdsm/storage/sdc.py b/vdsm/storage/sdc.py
index f2f4534..978e3fa 100644
--- a/vdsm/storage/sdc.py
+++ b/vdsm/storage/sdc.py
@@ -62,32 +62,27 @@
 
     STORAGE_UPDATED = 0
     STORAGE_STALE = 1
-    STORAGE_REFRESHING = 2
 
     def __init__(self, storage_repo):
-        self._syncroot = threading.Condition()
+        self._syncDomain = threading.Condition()
+        self._syncRefresh = threading.Lock()
         self.__domainCache = {}
         self.__inProgress = set()
         self.__staleStatus = self.STORAGE_STALE
         self.storage_repo = storage_repo
 
     def invalidateStorage(self):
-        with self._syncroot:
-            self.__staleStatus = self.STORAGE_STALE
+        self.log.debug("The storages have been invalidated")
+        self.__staleStatus = self.STORAGE_STALE
 
     @misc.samplingmethod
     def refreshStorage(self):
-        self.__staleStatus = self.STORAGE_REFRESHING
-
+        # We need to set the __staleStatus value at the beginning because we
+        # want to keep track of the future invalidateStorage calls that might
+        # arrive during the rescan procedure.
+        self.__staleStatus = self.STORAGE_UPDATED
         multipath.rescan()
         lvm.invalidateCache()
-
-        # If a new invalidateStorage request came in after the refresh
-        # started then we cannot flag the storages as updated (force a
-        # new rescan later).
-        with self._syncroot:
-            if self.__staleStatus == self.STORAGE_REFRESHING:
-                self.__staleStatus = self.STORAGE_UPDATED
 
     def produce(self, sdUUID):
         domain = DomainProxy(self, sdUUID)
@@ -98,7 +93,7 @@
         return domain
 
     def _realProduce(self, sdUUID):
-        with self._syncroot:
+        with self._syncDomain:
             while True:
                 domain = self.__domainCache.get(sdUUID)
 
@@ -109,25 +104,29 @@
                     self.__inProgress.add(sdUUID)
                     break
 
-                self._syncroot.wait()
+                self._syncDomain.wait()
 
         try:
-            # If multiple calls reach this point and the storage is not
-            # updated the refreshStorage() sampling method is called
-            # serializing (and eventually grouping) the requests.
-            if self.__staleStatus != self.STORAGE_UPDATED:
-                self.refreshStorage()
+            # Here we cannot take full advantage of the refreshStorage
+            # samplingmethod since we might be scheduling an unneeded
+            # extra rescan. We need an additional lock (_syncRefresh)
+            # to make sure that __staleStatus is taken in account
+            # (without affecting all the other external refreshStorage
+            # calls as it would be if we move this check there).
+            with self._syncRefresh:
+                if self.__staleStatus != self.STORAGE_UPDATED:
+                    self.refreshStorage()
 
             domain = self._findDomain(sdUUID)
 
-            with self._syncroot:
+            with self._syncDomain:
                 self.__domainCache[sdUUID] = domain
                 return domain
 
         finally:
-            with self._syncroot:
+            with self._syncDomain:
                 self.__inProgress.remove(sdUUID)
-                self._syncroot.notifyAll()
+                self._syncDomain.notifyAll()
 
     def _findDomain(self, sdUUID):
         import blockSD
@@ -162,16 +161,16 @@
         return uuids
 
     def refresh(self):
-        with self._syncroot:
+        with self._syncDomain:
             lvm.invalidateCache()
             self.__domainCache.clear()
 
     def manuallyAddDomain(self, domain):
-        with self._syncroot:
+        with self._syncDomain:
             self.__domainCache[domain.sdUUID] = domain
 
     def manuallyRemoveDomain(self, sdUUID):
-        with self._syncroot:
+        with self._syncDomain:
             try:
                 del self.__domainCache[sdUUID]
             except KeyError:


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If178a8eaeb94f1dfe9e0957036dde88f6a22829c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>


More information about the vdsm-patches mailing list