openstack
/
cinder
Code Issues Proposed changes

Switch from retrying to tenacity 

The retrying library is no longer maintained and users are encouraged to
migrate to tenacity.

This has a small behavior change in that before we were applying an
exponential backoff to the first time a retry was needed. This no longer
happens, but retries will exponentially back off with subsequent each
retry.

Change-Id: I25ca007386a7b8ca6a7d572742334ba3d7dcbf1e
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
This commit is contained in:
Sean McGinnis 2018-09-07 11:21:44 -05:00
parent d4eb4a9ba1
commit ec6fb2da52
No known key found for this signature in database
GPG Key ID: CE7EE4BFAF8D70C8
10 changed files with 85 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cinder/tests/unit/test.py
View File

@ -144,6 +144,10 @@ class TestCase(testtools.TestCase):
self.patch('cinder.rpc.get_notifier',
side_effect=self._get_joined_notifier)
# Protect against any case where someone doesn't directly patch a retry
# decorated call.
self.patch('cinder.utils._time_sleep')
if self.MOCK_WORKER:
# Mock worker creation for all tests that don't care about it
clean_path = 'cinder.objects.cleanable.CinderCleanableObject.%s'

21
cinder/tests/unit/test_utils.py
View File

@ -23,7 +23,6 @@ from unittest import mock
import ddt
from oslo_utils import timeutils
import six
from six.moves import range
import webob.exc
import cinder
@ -939,7 +938,7 @@ class TestRetryDecorator(test.TestCase):
def test_no_retry_required(self):
self.counter = 0
with mock.patch.object(time, 'sleep') as mock_sleep:
with mock.patch('cinder.utils._time_sleep') as mock_sleep:
@utils.retry(exception.VolumeBackendAPIException,
interval=2,
retries=3,
@ -956,7 +955,7 @@ class TestRetryDecorator(test.TestCase):
def test_no_retry_required_random(self):
self.counter = 0
with mock.patch.object(time, 'sleep') as mock_sleep:
with mock.patch('cinder.utils._time_sleep') as mock_sleep:
@utils.retry(exception.VolumeBackendAPIException,
interval=2,
retries=3,
@ -977,7 +976,7 @@ class TestRetryDecorator(test.TestCase):
backoff_rate = 2
retries = 3
with mock.patch.object(time, 'sleep') as mock_sleep:
with mock.patch('cinder.utils._time_sleep') as mock_sleep:
@utils.retry(exception.VolumeBackendAPIException,
interval,
retries,
@ -993,7 +992,7 @@ class TestRetryDecorator(test.TestCase):
self.assertEqual('success', ret)
self.assertEqual(2, self.counter)
self.assertEqual(1, mock_sleep.call_count)
mock_sleep.assert_called_with(interval * backoff_rate)
mock_sleep.assert_called_with(interval)
def test_retries_once_random(self):
self.counter = 0
@ -1001,7 +1000,7 @@ class TestRetryDecorator(test.TestCase):
backoff_rate = 2
retries = 3
with mock.patch.object(time, 'sleep') as mock_sleep:
with mock.patch('cinder.utils._time_sleep') as mock_sleep:
@utils.retry(exception.VolumeBackendAPIException,
interval,
retries,
@ -1026,7 +1025,7 @@ class TestRetryDecorator(test.TestCase):
interval = 2
backoff_rate = 4
with mock.patch.object(time, 'sleep') as mock_sleep:
with mock.patch('cinder.utils._time_sleep') as mock_sleep:
@utils.retry(exception.VolumeBackendAPIException,
interval,
retries,
@ -1040,17 +1039,17 @@ class TestRetryDecorator(test.TestCase):
self.assertEqual(retries, self.counter)
expected_sleep_arg = []
for i in range(retries):
if i > 0:
interval *= backoff_rate
interval *= (backoff_rate ** (i - 1))
expected_sleep_arg.append(float(interval))
mock_sleep.assert_has_calls(map(mock.call, expected_sleep_arg))
mock_sleep.assert_has_calls(
list(map(mock.call, expected_sleep_arg)))
def test_wrong_exception_no_retry(self):
with mock.patch.object(time, 'sleep') as mock_sleep:
with mock.patch('cinder.utils._time_sleep') as mock_sleep:
@utils.retry(exception.VolumeBackendAPIException)
def raise_unexpected_error():
raise WrongException("wrong exception")

7
cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py
View File

@ -2383,13 +2383,6 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
expected +
self.standard_logout)
def _FakeRetrying(wait_func=None,
original_retrying=hpecommon.utils.retrying.Retrying,
*args, **kwargs):
return original_retrying(wait_func=lambda *a, **k: 0,
*args, **kwargs)
@mock.patch('retrying.Retrying', _FakeRetrying)
def test_delete_volume_online_active_done(self):
# setup_mock_client drive with default configuration
# and return the mock HTTP 3PAR client

7
cinder/tests/unit/volume/drivers/huawei/test_huawei_drivers.py
View File

@ -24,7 +24,6 @@ from xml.etree import ElementTree
import ddt
import requests
import retrying
from cinder import context
from cinder import exception
@ -2368,7 +2367,7 @@ class HuaweiTestBase(test.TestCase):
@ddt.data('1', '', '0')
def test_copy_volume(self, input_speed):
self.driver.configuration.lun_copy_wait_interval = 0
self.driver.configuration.lun_copy_wait_interval = 1
self.volume.metadata = {'copyspeed': input_speed}
mocker = self.mock_object(
@ -4121,7 +4120,7 @@ class HuaweiISCSIDriverTestCase(HuaweiTestBase):
with mock.patch.object(rest_client.RestClient, 'get_lun_info',
return_value=offline_status):
self.assertRaises(retrying.RetryError,
self.assertRaises(exception.VolumeDriverException,
replica.wait_volume_online,
self.driver.client,
lun_info)
@ -4136,7 +4135,7 @@ class HuaweiISCSIDriverTestCase(HuaweiTestBase):
return_value={'SECRESACCESS': access_ro})
common_driver.wait_second_access(pair_id, access_ro)
self.assertRaises(retrying.RetryError,
self.assertRaises(exception.VolumeDriverException,
common_driver.wait_second_access, pair_id, access_rw)
def test_wait_replica_ready(self):

7
cinder/tests/unit/volume/drivers/test_rbd.py
View File

@ -91,14 +91,7 @@ def common_mocks(f):
The point of doing these mocks here is so that we don't accidentally set
mocks that can't/don't get unset.
"""
def _FakeRetrying(wait_func=None,
original_retrying = driver.utils.retrying.Retrying,
*args, **kwargs):
return original_retrying(wait_func=lambda *a, **k: 0,
*args, **kwargs)
def _common_inner_inner1(inst, *args, **kwargs):
@mock.patch('retrying.Retrying', _FakeRetrying)
@mock.patch.object(driver.RBDDriver, '_get_usage_info')
@mock.patch('cinder.volume.drivers.rbd.RBDVolumeProxy')
@mock.patch('cinder.volume.drivers.rbd.RADOSClient')

62
cinder/utils.py
View File

@ -17,7 +17,6 @@
"""Utilities and helper functions."""
import abc
from collections import OrderedDict
import contextlib
@ -29,7 +28,6 @@ import math
import operator
import os
import pyclbr
import random
import re
import shutil
import stat
@ -50,12 +48,30 @@ from oslo_utils import excutils
from oslo_utils import importutils
from oslo_utils import strutils
from oslo_utils import timeutils
import retrying
import six
from cinder import exception
from cinder.i18n import _
# The following section is needed to be able to mock out the sleep calls that
# happen in the tenacity retry handling. We save off the real time.sleep for
# later and so it can be mocked in unit tests for the sleep call that tenacity
# makes. But we don't want all time.sleep calls to be modified, so after
# loading the tenacity module, we restore things back to normal. End result is
# only the tenacity sleep calls go through the method that we can mock,
# everything else works as normal.
_time_sleep = time.sleep
def _sleep(duration):
"""Helper class to make it easier to mock retry sleeping."""
_time_sleep(duration)
time.sleep = _sleep
import tenacity # noqa
time.sleep = _time_sleep
# End of time
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
@ -658,41 +674,27 @@ class ComparableMixin(object):
def retry(exceptions, interval=1, retries=3, backoff_rate=2,
wait_random=False):
def _retry_on_exception(e):
return isinstance(e, exceptions)
def _backoff_sleep(previous_attempt_number, delay_since_first_attempt_ms):
exp = backoff_rate ** previous_attempt_number
wait_for = interval * exp
if wait_random:
random.seed()
wait_val = random.randrange(interval * 1000.0, wait_for * 1000.0)
else:
wait_val = wait_for * 1000.0
LOG.debug("Sleeping for %s seconds", (wait_val / 1000.0))
return wait_val
def _print_stop(previous_attempt_number, delay_since_first_attempt_ms):
delay_since_first_attempt = delay_since_first_attempt_ms / 1000.0
LOG.debug("Failed attempt %s", previous_attempt_number)
LOG.debug("Have been at this for %s seconds",
delay_since_first_attempt)
return previous_attempt_number == retries
if retries < 1:
raise ValueError('Retries must be greater than or '
'equal to 1 (received: %s). ' % retries)
if wait_random:
wait = tenacity.wait_random_exponential(multiplier=interval)
else:
wait = tenacity.wait_exponential(
multiplier=interval, min=0, exp_base=backoff_rate)
def _decorator(f):
@six.wraps(f)
def _wrapper(*args, **kwargs):
r = retrying.Retrying(retry_on_exception=_retry_on_exception,
wait_func=_backoff_sleep,
stop_func=_print_stop)
r = tenacity.Retrying(
before_sleep=tenacity.before_sleep_log(LOG, logging.DEBUG),
after=tenacity.after_log(LOG, logging.DEBUG),
stop=tenacity.stop_after_attempt(retries),
reraise=True,
retry=tenacity.retry_if_exception_type(exceptions),
wait=wait)
return r.call(f, *args, **kwargs)
return _wrapper

34
cinder/volume/drivers/huawei/huawei_utils.py
View File

@ -15,10 +15,10 @@
import hashlib
import json
import math
from oslo_log import log as logging
from oslo_utils import strutils
import retrying
import six
from cinder import context
@ -26,6 +26,7 @@ from cinder import exception
from cinder.i18n import _
from cinder import objects
from cinder.objects import fields
from cinder import utils
from cinder.volume.drivers.huawei import constants
from cinder.volume import qos_specs
from cinder.volume import volume_types
@ -65,17 +66,30 @@ def old_encode_host_name(name):
def wait_for_condition(func, interval, timeout):
def _retry_on_result(result):
return not result
"""Wait for ``func`` to return True.
def _retry_on_exception(result):
return False
This retries running func until it either returns True or raises an
exception.
:param func: The function to call.
:param interval: The interval to wait in seconds between calls.
:param timeout: The maximum time in seconds to wait.
"""
if interval == 0:
interval = 1
if timeout == 0:
timeout = 1
r = retrying.Retrying(retry_on_result=_retry_on_result,
retry_on_exception=_retry_on_exception,
wait_fixed=interval * 1000,
stop_max_delay=timeout * 1000)
r.call(func)
@utils.retry(exception.VolumeDriverException,
interval=interval,
backoff_rate=1,
retries=(math.ceil(timeout / interval)))
def _retry_call():
result = func()
if not result:
raise exception.VolumeDriverException(
_('Timed out waiting for condition.'))
_retry_call()
def _get_volume_type(volume):

19
cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py
View File

@ -31,7 +31,6 @@ from oslo_utils import excutils
from oslo_utils import strutils
from oslo_utils import units
import paramiko
from retrying import retry
import six
from cinder import context
@ -1208,15 +1207,15 @@ class StorwizeHelpers(object):
{'volume_name': volume_name, 'host_name': host_name})
return int(result_lun)
def _retry_on_exception(e):
if hasattr(e, 'msg') and 'CMMVC5879E' in e.msg:
return True
return False
class _RetryableVolumeDriverException(
exception.VolumeBackendAPIException):
"""Exception to identify which types of errors to retry."""
pass
@retry(retry_on_exception=_retry_on_exception,
stop_max_attempt_number=3,
wait_random_min=1,
wait_random_max=10)
@cinder_utils.retry(_RetryableVolumeDriverException,
interval=2,
retries=3,
wait_random=True)
def make_vdisk_host_map():
try:
result_lun = self._get_unused_lun_id(host_name)
@ -1233,6 +1232,8 @@ class StorwizeHelpers(object):
message=_('CMMVC6071E The VDisk-to-host mapping was '
'not created because the VDisk is already '
'mapped to a host.'))
if hasattr(ex, 'msg') and 'CMMVC5879E' in ex.msg:
raise _RetryableVolumeDriverException(ex)
with excutils.save_and_reraise_exception():
LOG.error('Error mapping VDisk-to-host.')

2
lower-constraints.txt
View File

@ -147,7 +147,7 @@ stevedore==1.20.0
tabulate==0.8.5
taskflow==3.2.0
Tempita==0.5.2
tenacity==4.9.0
tenacity==6.0.0
testrepository==0.0.20
testtools==2.2.0
tooz==1.58.0

2
requirements.txt
View File

@ -43,7 +43,6 @@ python-novaclient>=9.1.0 # Apache-2.0
python-swiftclient>=3.2.0 # Apache-2.0
pytz>=2013.6 # MIT
requests>=2.14.2,!=2.20.0 # Apache-2.0
retrying!=1.3.0,>=1.2.3 # Apache-2.0
Routes>=2.3.1 # MIT
taskflow>=3.2.0 # Apache-2.0
rtslib-fb>=2.1.65 # Apache-2.0
@ -52,6 +51,7 @@ SQLAlchemy!=1.1.5,!=1.1.6,!=1.1.7,!=1.1.8,>=1.0.10 # MIT
sqlalchemy-migrate>=0.11.0 # Apache-2.0
stevedore>=1.20.0 # Apache-2.0
tabulate>=0.8.5 # MIT
tenacity>=6.0.0 # Apache-2.0
WebOb>=1.7.1 # MIT
oslo.i18n>=3.15.3 # Apache-2.0
oslo.vmware>=2.35.0 # Apache-2.0