Change in vdsm[ovirt-3.5]: oop: Use a single instance of IOProcess per SD

ykaplan at redhat.com ykaplan at redhat.com
Thu Jan 15 12:47:55 UTC 2015


Hello Saggi Mizrahi, Dima Kuznetsov,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/36953

to review the following change.

Change subject: oop: Use a single instance of IOProcess per SD
......................................................................

oop: Use a single instance of IOProcess per SD

Next step after switching from RFH to IOPROC is to
use a single IOProcess instance for each SD.

Currently only a single IOProcess instance is used
for all SDs.

To avoid load on the system by holding many unused
IOProcesses we will maintain dictionary:
KEY - domainID, VALUE - (timestamp, IOPROC)
When IOProc for domainID is used we will renew the
timestamp, or delete the item from dictionary

We will also maintain another dictionary:
KEY - domainID, VALUE - IOPROC weakref,
that will allow us to check if IOPROC obj is
still alive or needs to be recreated.

Change-Id: I383fe617ee4ce22de368ba54f980887d70ff37c5
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1126206
Signed-off-by: Yeela Kaplan <ykaplan at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31501
Reviewed-by: Dima Kuznetsov <dkuznets at redhat.com>
Reviewed-by: Saggi Mizrahi <smizrahi at redhat.com>
---
M lib/vdsm/config.py.in
M vdsm/storage/outOfProcess.py
2 files changed, 51 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/36953/1

diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index ab330be..9dcc487 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -315,8 +315,13 @@
 
         ('process_pool_timeout', '60', None),
 
+        ('max_ioprocess_idle_time', '60',
+            'TTL of an unused IOProcess instance'),
+
         ('process_pool_max_slots_per_domain', '10', None),
 
+        ('process_pool_max_queued_slots_per_domain', '10', None),
+
         ('iscsi_default_ifaces', 'default',
             'Comma seperated ifaces to connect with. '
             'i.e. iser,default'),
diff --git a/vdsm/storage/outOfProcess.py b/vdsm/storage/outOfProcess.py
index 17445e6..75fbb14 100644
--- a/vdsm/storage/outOfProcess.py
+++ b/vdsm/storage/outOfProcess.py
@@ -24,6 +24,7 @@
 import stat
 import sys
 import types
+import weakref
 
 from vdsm import constants
 from vdsm.config import config
@@ -45,10 +46,16 @@
 _oopImpl = RFH
 
 DEFAULT_TIMEOUT = config.getint("irs", "process_pool_timeout")
+IOPROC_IDLE_TIME = config.getint("irs", "max_ioprocess_idle_time")
 HELPERS_PER_DOMAIN = config.getint("irs", "process_pool_max_slots_per_domain")
+MAX_QUEUED = config.getint("irs", "process_pool_max_queued_slots_per_domain")
 
-_procLock = threading.Lock()
-_proc = {}
+_procPoolLock = threading.Lock()
+_procPool = {}
+_refProcPool = {}
+_rfhPool = {}
+
+elapsed_time = lambda: os.times()[4]
 
 log = logging.getLogger('Storage.oop')
 
@@ -61,20 +68,44 @@
         _oopImpl = RFH
 
 
-def getProcessPool(clientName):
-    try:
-        return _proc[clientName]
-    except KeyError:
-        with _procLock:
-            if _oopImpl == IOPROC:
-                if GLOBAL not in _proc:
-                    _proc[GLOBAL] = _OopWrapper(IOProcess(DEFAULT_TIMEOUT))
-                _proc[clientName] = _proc[GLOBAL]
-            else:
-                _proc[clientName] = _OopWrapper(
-                    RemoteFileHandlerPool(HELPERS_PER_DOMAIN))
+def cleanIdleIOProcesses(clientName):
+    now = elapsed_time()
+    for name, (eol, proc) in _procPool.items():
+        if (eol < now and name != clientName):
+            del _procPool[name]
 
-            return _proc[clientName]
+
+def _getRfhPool(clientName):
+    with _procPoolLock:
+        try:
+            return _rfhPool[clientName]
+        except KeyError:
+            _rfhPool[clientName] = _OopWrapper(
+                RemoteFileHandlerPool(HELPERS_PER_DOMAIN))
+
+            return _rfhPool[clientName]
+
+
+def _getIOProcessPool(clientName):
+    with _procPoolLock:
+        cleanIdleIOProcesses(clientName)
+
+        proc = _refProcPool.get(clientName, lambda: None)()
+        if proc is None:
+            proc = _OopWrapper(IOProcess(max_threads=HELPERS_PER_DOMAIN,
+                                         timeout=DEFAULT_TIMEOUT,
+                                         max_queued_requests=MAX_QUEUED))
+            _refProcPool[clientName] = weakref.ref(proc)
+
+        _procPool[clientName] = (elapsed_time() + IOPROC_IDLE_TIME, proc)
+        return proc
+
+
+def getProcessPool(clientName):
+    if _oopImpl == IOPROC:
+        return _getIOProcessPool(clientName)
+    else:
+        return _getRfhPool(clientName)
 
 
 def getGlobalProcPool():


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I383fe617ee4ce22de368ba54f980887d70ff37c5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>


More information about the vdsm-patches mailing list