New subject: Change in vdsm[master]: BZ#725967 - one process pool to rule them all

25 Jul 2011

New patch submitted by Saggi Mizrahi (smizrahi@redhat.com)
You can review this change at: http://gerrit.usersys.redhat.com/744
commit 6978ada2d810ac73e2fda4c67af52e388bdc1c75
Author: Saggi Mizrahi smizrahi@redhat.com
Date:   Mon Jul 25 16:18:36 2011 +0300
BZ#????? - one process pool to rule them all
Instead of having a process pool per domain use one pool but limit the amount of
    workers a domain can hold at any given time.
Change-Id: Ia69071a049761b271a982c600cf0520a782c7eac

diff --git a/vdsm/config.py b/vdsm/config.py
index 488aa7b..dbed034 100644
--- a/vdsm/config.py
+++ b/vdsm/config.py
@@ -146,9 +146,10 @@ config.set('irs', 'gc_blocker_force_collect_interval', '60')
config.set('irs', 'maximum_domains_in_pool', '100')
 # Process Pool Configuration
-config.set('irs', 'process_pool_size', '20')
+config.set('irs', 'process_pool_size', '100')
 config.set('irs', 'process_pool_timeout', '60')
 config.set('irs', 'process_pool_grace_period', '2')
+config.set("irs", "process_pool_max_slots_per_domain", '10')
#####################################################################
 config.add_section('addresses')
diff --git a/vdsm/storage/outOfProcess.py b/vdsm/storage/outOfProcess.py
index 48a556a..e1a4de7 100644
--- a/vdsm/storage/outOfProcess.py
+++ b/vdsm/storage/outOfProcess.py
@@ -2,15 +2,18 @@ import os as mod_os
 import glob as mod_glob
 import types
 from config import config
+import threading
+from functools import wraps
from fileUtils import open_ex
 import fileUtils as mod_fileUtils
-from processPool import ProcessPool
+from processPool import ProcessPool, NoFreeHelpersError
MAX_HELPERS = config.getint("irs", "process_pool_size")
 GRACE_PERIOD = config.getint("irs", "process_pool_grace_period")
 DEFAULT_TIMEOUT = config.getint("irs", "process_pool_timeout")
+HELPERS_PER_DOMAIN = config.getint("irs", "process_pool_max_slots_per_domain")
_globalPool = ProcessPool(MAX_HELPERS, GRACE_PERIOD, DEFAULT_TIMEOUT)
@@ -67,6 +70,32 @@ setattr(os, 'path', _ModuleWrapper(mod_os.path))
fileUtils = _ModuleWrapper(mod_fileUtils)
+class ProcessPoolLimiter(object):
+    def __init__(self, procPool, limit):
+        self._procPool = procPool
+        self._limit = limit
+        self._lock = threading.Lock()
+        self._counter = 0
+
+    def wrapFunction(self, func):
+        @wraps(func)
+        def wrapper(*args, **kwds):
+            return self.runExternally(func, *args, **kwds)
+        return wrapper
+
+    def runExternally(self, *args, **kwargs):
+        with self._lock:
+            if self._counter >= self._limit:
+                raise NoFreeHelpersError("You reached the process limit")
+
+            self._counter += 1
+        try:
+            return self._procPool.runExternally(*args, **kwargs)
+        finally:
+            with self._lock:
+                self._counter -= 1
+
+
 class OopWrapper(object):
     def __init__(self, procPool):
         self._processPool = procPool
diff --git a/vdsm/storage/processPool.py b/vdsm/storage/processPool.py
index f8ef4af..98a32e2 100644
--- a/vdsm/storage/processPool.py
+++ b/vdsm/storage/processPool.py
@@ -23,7 +23,7 @@ class ProcessPool(object):
         self._maxSubProcess = maxSubProcess
         self._gracePeriod = gracePeriod
         self.timeout = timeout
-        self._helperPool = [None] * self._maxSubProcess
+        self._helperPool = [Helper() for i in range(self._maxSubProcess)]
         self._lockPool = [Lock() for i in range(self._maxSubProcess)]
         self._closed = False
diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index 4422487..344d578 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -224,6 +224,7 @@ class ProcessPoolDict(dict):
     def __init__(self):
         dict.__init__(self)
         self._lock = threading.Lock()
+        self._pool = None
def __getitem__(self, key):
         try:
@@ -235,8 +236,14 @@ class ProcessPoolDict(dict):
                 return dict.__getitem__(self, key)
def _createProcessPool(self, key):
-        _domainPool = ProcessPool(oop.MAX_HELPERS, oop.GRACE_PERIOD, oop.DEFAULT_TIMEOUT)
-        return oop.OopWrapper(_domainPool)
+        # I initialize the pool dict on first call
+        # because it's created on import and generating
+        # hundreds of subprocess on import is not recommended
+        if self._pool is None:
+            with self._lock:
+                if self._pool is None:
+                    self._pool = ProcessPool(oop.MAX_HELPERS, oop.GRACE_PERIOD, oop.DEFAULT_TIMEOUT)
+        return oop.OopWrapper(oop.ProcessPoolLimiter(self._pool, oop.HELPERS_PER_DOMAIN))
# Dictionary for process pools per sdUUID
 processPoolDict = ProcessPoolDict()

    

[NEW PATCH] BZ#????? - one process pool to rule them all (via gerrit-bot)