Change in vdsm[master]: executor: Fix race when putting tasks

nsoffer at redhat.com nsoffer at redhat.com
Sat Jul 18 01:08:15 UTC 2015


Nir Soffer has uploaded a new change for review.

Change subject: executor: Fix race when putting tasks
......................................................................

executor: Fix race when putting tasks

When putting tasks into a TaskQueue, we used check the current number of
tasks in a thread-unsafe way.  We assume that the thread-unsafe check
can result in the worst case in few extra tasks in the task queue, which
is not very interesting.

However, if all workers are busy, and the task queue is almost full, it
is possible that two threads will pass the size check, and the queue
will contain more tasks then the maximum value. Once the queue size
exceeded the limit, we allow any number of additional tasks.

While this condition is unlikely, there is not reason to allow this
behavior. Now we check the queue size in a thread safe way and we don't
have to maintain the comment about the possible race.

Since we take the condition anyway on every put, I don't expect a
noticeable performance differences by taking the condition before the
size check.

Change-Id: Ie9ce28985e18629abb812e05752df90d05800c86
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M lib/vdsm/executor.py
1 file changed, 3 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/43789/1

diff --git a/lib/vdsm/executor.py b/lib/vdsm/executor.py
index 2639b26..3e96a75 100644
--- a/lib/vdsm/executor.py
+++ b/lib/vdsm/executor.py
@@ -231,13 +231,10 @@
         Put a new task in the queue.
         Do not block when full, raises TooManyTasks instead.
         """
-        # There is a race here but we don't really mind. Worst case scenario,
-        # we will have a bit more task then the configured maximum, but we
-        # just want to avoid to have indefinite amount of tasks.
-        if len(self._tasks) == self._max_tasks:
-            raise TooManyTasks()
-        self._tasks.append(task)
         with self._cond:
+            if len(self._tasks) == self._max_tasks:
+                raise TooManyTasks()
+            self._tasks.append(task)
             self._cond.notify()
 
     def get(self):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie9ce28985e18629abb812e05752df90d05800c86
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list