Change in vdsm[master]: virt: sampling: replace flag with Semaphore

2015-06-16 Thread danken
Dan Kenigsberg has submitted this change and it was merged.

Change subject: virt: sampling: replace flag with Semaphore
..


virt: sampling: replace flag with Semaphore

In order to detect stuck sampling calls, the SampleVM
operation used a simple boolean flag to track if
the code was running or not.

However, this may lead to races if an operation unblocks
just after the flag was set.
This race is expected to do little harm, but could and should
be improved anyway.

To avoid race, we convert the boolean flag to a Semaphore.
The Semaphore is used as glorified counter, so it should
always be called with blocking=False.

Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Signed-off-by: Francesco Romani 
Reviewed-on: https://gerrit.ovirt.org/40353
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg 
---
M vdsm/virt/sampling.py
1 file changed, 4 insertions(+), 4 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
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[master]: virt: sampling: replace flag with Semaphore

2015-06-16 Thread automation
automat...@ovirt.org has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 3:

* Update tracker::IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
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[master]: virt: sampling: replace flag with Semaphore

2015-06-16 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
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[master]: virt: sampling: replace flag with Semaphore

2015-06-15 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 2: Verified+1

rebased - and fixed conflict, thus losing score.

Verified running patched VM.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
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[master]: virt: sampling: replace flag with Semaphore

2015-06-15 Thread automation
automat...@ovirt.org has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
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[master]: virt: sampling: replace flag with Semaphore

2015-05-12 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
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[master]: virt: sampling: replace flag with Semaphore

2015-04-28 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 1:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18289/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1519/ : 0

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: replace flag with Semaphore

2015-04-28 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 1:

Build Started (2/2)

0 -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1519/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: replace flag with Semaphore

2015-04-28 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 1:

Build Started (1/2) -> 
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18289/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: replace flag with Semaphore

2015-04-28 Thread fromani
Francesco Romani has uploaded a new change for review.

Change subject: virt: sampling: replace flag with Semaphore
..

virt: sampling: replace flag with Semaphore

In order to detect stuck sampling calls, the SampleVM
operation used a simple boolean flag to track if
the code was running or not.

However, this may lead to races if an operation unblocks
just after the flag was set.
This race is expected to do little harm, but could and should
be improved anyway.

To avoid race, we convert the boolean flag to a Semaphore.
The Semaphore is used as glorified counter, so it should
always be called with blocking=False.

Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Signed-off-by: Francesco Romani 
---
M vdsm/virt/sampling.py
1 file changed, 4 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/40353/1

diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index 746f059..2257eec 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -499,15 +499,15 @@
 self._stats_cache = stats_cache
 self._stats_flags = stats_flags
 self._skip_doms = ExpiringCache(timeout)
-self._sampling = False
+self._sampling = threading.Semaphore()  # used as glorified counter
 self._log = logging.getLogger("sampling.SampleVMs")
 
 def __call__(self):
 timestamp = self._stats_cache.clock()
 # we are deep in the hot path. bool(ExpiringCache)
 # *is* costly so we should avoid it if we can.
-fast_path = (not self._sampling and not self._skip_doms)
-self._sampling = True
+fast_path = (
+self._sampling.acquire(blocking=False) and not self._skip_doms)
 try:
 if fast_path:
 # This is expected to be the common case.
@@ -528,7 +528,7 @@
 else:
 self._stats_cache.put(_translate(bulk_stats), timestamp)
 finally:
-self._sampling = False
+self._sampling.release()
 
 def _get_responsive_doms(self):
 vms = self._get_vms()


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: replace flag with Semaphore

2015-04-28 Thread automation
automat...@ovirt.org has posted comments on this change.

Change subject: virt: sampling: replace flag with Semaphore
..


Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7d1b3ea990c1398255b216aeb247a56bd82ba7bd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
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