Change in vdsm[master]: jsonrpc: executor based thread factory

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Fri Jul 17 14:22:27 UTC 2015


Piotr Kliczewski has posted comments on this change.

Change subject: jsonrpc: executor based thread factory
......................................................................


Patch Set 1:

(8 comments)

https://gerrit.ovirt.org/#/c/43759/1/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 204: 
Line 205:         ('worker_threads', '8',
Line 206:             'Number of worker threads to serve jsonrpc server.'),
Line 207: 
Line 208:         ('tasks_per_worker', '100',
> Why do you want to queue 800 requests? I would use 10 here :-)
Done
Line 209:             'Max number of tasks which can be queued per workers.'),
Line 210:     ]),
Line 211: 
Line 212:     # Section: [mom]


https://gerrit.ovirt.org/#/c/43759/1/vdsm/rpc/bindingjsonrpc.py
File vdsm/rpc/bindingjsonrpc.py:

Line 31: _TASKS = _THREADS * _TASK_PER_WORKER
Line 32: 
Line 33: 
Line 34: _scheduler = schedule.Scheduler(name="jsonrpc.Scheduler",
Line 35:                                 clock=monotonic_time)
> We already have a scheduler for virt periodic tasks, I think we can use sin
Done
Line 36: 
Line 37: _executor = executor.Executor(name="jsonrpc.Executor",
Line 38:                               workers_count=_THREADS,
Line 39:                               max_tasks=_TASKS,


Line 36: 
Line 37: _executor = executor.Executor(name="jsonrpc.Executor",
Line 38:                               workers_count=_THREADS,
Line 39:                               max_tasks=_TASKS,
Line 40:                               scheduler=_scheduler)
> The scheduler and the executor belongs to BindingJsonRpc class, just like t
Done
Line 41: 
Line 42: 
Line 43: class _RequestWorker(object):
Line 44: 


Line 39:                               max_tasks=_TASKS,
Line 40:                               scheduler=_scheduler)
Line 41: 
Line 42: 
Line 43: class _RequestWorker(object):
> This class is not needed. Executor is supporting callable objects so you ca
Done
Line 44: 
Line 45:     _log = logging.getLogger("jsonrpc._RequestWorker")
Line 46: 
Line 47:     def __init__(self, func):


Line 50:     def __call__(self):
Line 51:         try:
Line 52:             self._func()
Line 53:         except Exception:
Line 54:             self._log.exception("%s processing request failed", self._func)
> Executor handle exceptions in function, we don't need this. If executor exc
Done
Line 55: 
Line 56: 
Line 57: def _executorFactory(func):
Line 58:     _executor.dispatch(_RequestWorker(func))


Line 54:             self._log.exception("%s processing request failed", self._func)
Line 55: 
Line 56: 
Line 57: def _executorFactory(func):
Line 58:     _executor.dispatch(_RequestWorker(func))
> This function is not needed.
Done
Line 59: 
Line 60: 
Line 61: class BindingJsonRpc(object):
Line 62:     log = logging.getLogger('BindingJsonRpc')


Line 64:     def __init__(self, bridge, subs, timeout):
Line 65:         _scheduler.start()
Line 66:         _executor.start()
Line 67: 
Line 68:         self._server = JsonRpcServer(bridge, timeout, _executorFactory)
> But note that the executor may raise TooManyTasks - you must handle this ex
Done
Line 69:         self._reactor = StompReactor(subs)
Line 70:         self.startReactor()
Line 71: 
Line 72:     def add_socket(self, reactor, client_socket):


Line 64:     def __init__(self, bridge, subs, timeout):
Line 65:         _scheduler.start()
Line 66:         _executor.start()
Line 67: 
Line 68:         self._server = JsonRpcServer(bridge, timeout, _executorFactory)
> Use executor.dispatch as the threadFactory.
Done
Line 69:         self._reactor = StompReactor(subs)
Line 70:         self.startReactor()
Line 71: 
Line 72:     def add_socket(self, reactor, client_socket):


-- 
To view, visit https://gerrit.ovirt.org/43759
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I56b307633a8bf7e4aad8f87cc97a4129c9ed0970
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list