Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Francesco Romani has submitted this change and it was merged. Change subject: executor: Fix thread-unsafe call .. executor: Fix thread-unsafe call Deques support thread-safe, appends and pops from either side of the deque, but deque.clear() is not documented as thread-safe, so we should not assume it is. Now we take the condition before invoking clear, and the comment about thread-safety was fixed to match Python documentation. Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Signed-off-by: Nir Soffer Reviewed-on: https://gerrit.ovirt.org/43790 Continuous-Integration: Jenkins CI Reviewed-by: Francesco Romani Reviewed-by: Dan Kenigsberg Bug-Url: https://bugzilla.redhat.com/1279950 Reviewed-on: https://gerrit.ovirt.org/48197 Tested-by: Piotr Kliczewski Continuous-Integration: Francesco Romani --- M lib/vdsm/executor.py 1 file changed, 6 insertions(+), 4 deletions(-) Approvals: Piotr Kliczewski: Verified Francesco Romani: Looks good to me, approved; Passed CI tests -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
automat...@ovirt.org has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 4: * #1279950::Update tracker: OK * Set MODIFIED::bug 1279950#1279950IGNORE, not oVirt classification but Red Hat -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Francesco Romani has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 3: Continuous-Integration+1 run CI tests manually. -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Francesco Romani has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 3: Rerun-Hooks: all -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
automat...@ovirt.org has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 3: * #1279950::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1279950::OK, public bug * Check Product::#1279950::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TM::#1279950::OK, correct target milestone ovirt-3.5.7 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Francesco Romani has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Piotr Kliczewski has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 3: Verified+1 Rebase only. Copying verification flag. -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
automat...@ovirt.org has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 3: * #1279950::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1279950::OK, public bug * Check Product::#1279950::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TM::#1279950::OK, correct target milestone ovirt-3.5.7 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Francesco Romani has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Hello Dan Kenigsberg, Francesco Romani, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/48197 to review the following change. Change subject: executor: Fix thread-unsafe call .. executor: Fix thread-unsafe call Deques support thread-safe, appends and pops from either side of the deque, but deque.clear() is not documented as thread-safe, so we should not assume it is. Now we take the condition before invoking clear, and the comment about thread-safety was fixed to match Python documentation. Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Signed-off-by: Nir Soffer Reviewed-on: https://gerrit.ovirt.org/43790 Continuous-Integration: Jenkins CI Reviewed-by: Francesco Romani Reviewed-by: Dan Kenigsberg Bug-Url: https://bugzilla.redhat.com/1279950 --- M lib/vdsm/executor.py 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/48197/2 diff --git a/lib/vdsm/executor.py b/lib/vdsm/executor.py index fc7880e..f63adec 100644 --- a/lib/vdsm/executor.py +++ b/lib/vdsm/executor.py @@ -220,9 +220,10 @@ def __init__(self, max_tasks): self._max_tasks = max_tasks self._tasks = collections.deque() -# deque is thread safe - we can append and pop from both ends without -# additional locking. We need this condition only for waiting. See: -# https://docs.python.org/2.6/library/queue.html#Queue.Full +# Deque supports thread-safe append and pop from both ends. We need +# this condition for waking up threads waiting on an empty queue and +# protecting other methods which are not documented as thread-safe. +# https://docs.python.org/2/library/collections.html#deque-objects self._cond = threading.Condition(threading.Lock()) def put(self, task): @@ -252,4 +253,5 @@ self._cond.wait() def clear(self): -self._tasks.clear() +with self._cond: +self._tasks.clear() -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
automat...@ovirt.org has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 2: -Verified * #1279950::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1279950::OK, public bug * Check Product::#1279950::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TM::#1279950::OK, correct target milestone ovirt-3.5.7 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
Piotr Kliczewski has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 2: Verified+1 Verified by performance team -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: executor: Fix thread-unsafe call
automat...@ovirt.org has posted comments on this change. Change subject: executor: Fix thread-unsafe call .. Patch Set 1: Verified-1 * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/48197 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia99d44698bc03bfcbac1a66f151f40220b8b4ef9 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches