[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