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

Dan Kenigsberg danken at redhat.com
Tue Sep 6 12:04:39 UTC 2011


New patch submitted by Dan Kenigsberg (danken at redhat.com)

You can review this change at: http://gerrit.usersys.redhat.com/744

commit 29e231dbb48ebea2ad539c97c1c87845d6ac526c
Author: Saggi Mizrahi <smizrahi at redhat.com>
Date:   Mon Jul 25 16:18:36 2011 +0300

    BZ#725967 - 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 b76f7af..12675f0 100644
--- a/vdsm/config.py
+++ b/vdsm/config.py
@@ -166,9 +166,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 60869cf..9977609 100644
--- a/vdsm/storage/outOfProcess.py
+++ b/vdsm/storage/outOfProcess.py
@@ -22,15 +22,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)
 
@@ -87,6 +90,33 @@ 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 6c6632d..74cede5 100644
--- a/vdsm/storage/processPool.py
+++ b/vdsm/storage/processPool.py
@@ -43,7 +43,14 @@ class ProcessPool(object):
         self._maxSubProcess = maxSubProcess
         self._gracePeriod = gracePeriod
         self.timeout = timeout
-        self._helperPool = [None] * self._maxSubProcess
+        # We start all the helpers at once because of fork() semantics.
+        # Every time you fork() the memory of the application is shared
+        # with the child process until one of the processes writes to it.
+        # In our case VDSM will probably rewrite the mem pretty quickly
+        # and all the mem will just get wasted untouched on the child's side.
+        # What we count on is having all the child processes share the mem.
+        # This is best utilized by starting all child processes at once.
+        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 f37b507..f7755d0 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -234,7 +234,8 @@ SD_MD_FIELDS = {
 class ProcessPoolDict(dict):
     def __init__(self):
         dict.__init__(self)
-        self._lock = threading.Lock()
+        self._lock = threading.RLock()
+        self._pool = None
 
     def __getitem__(self, key):
         try:
@@ -245,9 +246,19 @@ class ProcessPoolDict(dict):
                     self[key] = self._createProcessPool(key)
                 return dict.__getitem__(self, key)
 
+    def init(self):
+        with self._lock:
+            if self._pool is None:
+                self._pool = ProcessPool(oop.MAX_HELPERS, oop.GRACE_PERIOD, oop.DEFAULT_TIMEOUT)
+
     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:
+            self.init()
+
+        return oop.OopWrapper(oop.ProcessPoolLimiter(self._pool, oop.HELPERS_PER_DOMAIN))
 
 # Dictionary for process pools per sdUUID
 processPoolDict = ProcessPoolDict()
diff --git a/vdsm/vdsm b/vdsm/vdsm
index 2aee6f4..1c97694 100755
--- a/vdsm/vdsm
+++ b/vdsm/vdsm
@@ -33,6 +33,11 @@ def serve_clients(log):
         if cif and cif.irs:
             cif.irs.spmStop(cif.irs.hsm.pools.keys()[0])
 
+    # This has to happen before the signal handlers are registered
+    # so that the children will not inherit them.
+    import storage.sd
+    storage.sd.processPoolDict.init()
+
     signal.signal(signal.SIGTERM, sigtermHandler)
     signal.signal(signal.SIGUSR1, sigusr1Handler)
 




More information about the vdsm-patches mailing list