Change in vdsm[master]: utils: Consider sleep time in deadline calculation

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................

utils: Consider sleep time in deadline calculation

This patches fixes 2 utils.retry() broken tests:

- The special case when deadline has reached when an operation was done.
  Previously we use to sleep and perform another retry, now we bail out.

- The special case when deadline was not reached when an operation was
  done, but we don't have time for sleep (sleeping will reach or exceed
  the deadline). Previously we used to sleep and perform another retry,
  now we bail out.

Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M lib/vdsm/utils.py
M tests/utilsTests.py
2 files changed, 3 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/46400/1

diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index caf4cc2..4a4b153 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -938,10 +938,7 @@
     if tries in [0, None]:
         tries = -1
 
-    if timeout in [0, None]:
-        timeout = -1
-
-    startTime = monotonic_time()
+    deadline = monotonic_time() + timeout if timeout else None
 
     while True:
         tries -= 1
@@ -951,7 +948,7 @@
             if tries == 0:
                 raise
 
-            if (timeout > 0) and ((monotonic_time() - startTime) > timeout):
+            if deadline and monotonic_time() + sleep >= deadline:
                 raise
 
             if stopCallback is not None and stopCallback():
diff --git a/tests/utilsTests.py b/tests/utilsTests.py
index d213de5..fd4f42e 100644
--- a/tests/utilsTests.py
+++ b/tests/utilsTests.py
@@ -44,7 +44,7 @@
 from testlib import permutations, expandPermutations
 from testlib import VdsmTestCase as TestCaseBase
 from testValidation import checkSudo
-from testValidation import brokentest, stresstest
+from testValidation import stresstest
 from multiprocessing import Process
 
 EXT_SLEEP = "sleep"
@@ -86,7 +86,6 @@
         # Make sure we had the proper amount of iterations before failing
         self.assertEquals(counter[0], limit)
 
-    @brokentest("deadline is not respected")
     @MonkeyPatch(utils, 'monotonic_time', FakeTime(0))
     @MonkeyPatch(time, 'sleep', fake_sleep)
     def testTimeoutDeadlineReached(self):
@@ -104,7 +103,6 @@
                           timeout=3, sleep=1)
         self.assertEqual(utils.monotonic_time.now, 3)
 
-    @brokentest("sleep is not considered in deadline calculation")
     @MonkeyPatch(utils, 'monotonic_time', FakeTime(0))
     @MonkeyPatch(time, 'sleep', fake_sleep)
     def testTimeoutNoTimeForSleep(self):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


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/46400
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Patch Set 1:

Here uses of utils.retry with a timeout (code not using timeouts filtered out).

$ git grep utils.retry lib
lib/vdsm/libvirtconnection.py:    return utils.retry(libvirtOpen, timeout=10, sleep=0.2)
lib/vdsm/supervdsm.py:            utils.retry(self._manager.connect, Exception,
timeout=60, tries=3)

$ git grep utils.retry tests
tests/remoteFileHandlerTests.py:        utils.retry(test, AssertionError, timeout=4,
sleep=0.1)

Some funcional tests are using utils.retry via self.retryAssert:

$ git grep -A1 retryAssert
tests/functional/networkTests.py:            self.retryAssert(
tests/functional/networkTests.py-                assertTestDevStatsReported, timeout=20)
--
tests/functional/networkTests.py:            self.retryAssert(assertStatsInRange,
timeout=3)
tests/functional/networkTests.py-
--
tests/functional/storageTests.py:        self.retryAssert(assertTaskOK, timeout=60)
tests/functional/storageTests.py-
--
tests/functional/storageTests.py:        self.asserts.retryAssert(
tests/functional/storageTests.py-            partial(self._assertBackendConnected,
connections),
--
tests/functional/storageTests.py:        iqnDevs =
self.asserts.retryAssert(partial(self._getIqnDevs, iqns),
tests/functional/storageTests.py-                                           timeout=30)
--
tests/functional/virtTests.py:       
self.retryAssert(partial(self.assertQemuSetupComplete, vmid),
tests/functional/virtTests.py-                         timeout=10)
tests/functional/virtTests.py:        self.retryAssert(partial(self.assertVmBooting,
vmid),
tests/functional/virtTests.py-                         timeout=3)
tests/functional/virtTests.py:        self.retryAssert(partial(self.assertVmUp, vmid),
tests/functional/virtTests.py-                         timeout=10)
--
tests/functional/virtTests.py:        self.retryAssert(partial(self.assertGuestUp, vmid,
targetUptime),
tests/functional/virtTests.py-                         timeout=math.ceil(targetUptime *
1.2))
--
tests/functional/virtTests.py:        self.retryAssert(partial(self.assertVmDown, vmid),
tests/functional/virtTests.py-                         timeout=10)
--
tests/functional/virtTests.py:               
self.retryAssert(partial(self.vdsm.hotplugNic,
tests/functional/virtTests.py-                                        
deviceDef['hotplugNic']), timeout=10)
tests/functional/virtTests.py:               
self.retryAssert(partial(self.vdsm.hotunplugNic,
tests/functional/virtTests.py-                                        
deviceDef['hotplugNic']), timeout=10)
--
tests/functional/virtTests.py:               
self.retryAssert(partial(self.vdsm.hotplugDisk,
tests/functional/virtTests.py-                                        
deviceDef['hotplugDisk']), timeout=10)
tests/functional/virtTests.py:               
self.retryAssert(partial(self.vdsm.hotunplugDisk,
tests/functional/virtTests.py-                                        
deviceDef['hotplugDisk']), timeout=10)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


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/46400
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Patch Set 3:

* 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/46400
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Patch Set 4:

* 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/46400
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Patch Set 4: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Jenkins CI RO]

Jenkins CI RO has abandoned this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Abandoned

Abandoned due to no activity - please restore if still relevant

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has restored this change.

Change subject: utils: Consider sleep time in deadline calculation
......................................................................


Restored

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

Gerrit-MessageType: restore
Gerrit-Change-Id: I7f41c6b21e3432159c13d46cfe75d1f6236cbb8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;