From 2dc08cfdc975a2b83a8647a6120c6354685b9939 Mon Sep 17 00:00:00 2001 From: Thomas Schultz Date: Fri, 29 Jul 2016 15:48:05 -0400 Subject: [PATCH 1/6] Playing with retries. --- system_tests/bigquery.py | 15 ++++++++++-- system_tests/retry.py | 49 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 system_tests/retry.py diff --git a/system_tests/bigquery.py b/system_tests/bigquery.py index ac350ad83ca7..a8812e5bacb8 100644 --- a/system_tests/bigquery.py +++ b/system_tests/bigquery.py @@ -20,7 +20,9 @@ from gcloud import _helpers from gcloud.environment_vars import TESTS_PROJECT from gcloud import bigquery +from gcloud.exceptions import Forbidden +from retry import retry from system_test_utils import unique_resource_id @@ -90,7 +92,12 @@ def test_update_dataset(self): after = [grant for grant in dataset.access_grants if grant.entity_id != 'projectWriters'] dataset.access_grants = after - dataset.update() + + @retry(Forbidden, tries=3, delay=30) + def update_dataset(): + dataset.update() + + update_dataset() self.assertEqual(len(dataset.access_grants), len(after)) for found, expected in zip(dataset.access_grants, after): self.assertEqual(found.role, expected.role) @@ -188,7 +195,11 @@ def test_patch_table(self): def test_update_table(self): dataset = Config.CLIENT.dataset(DATASET_NAME) self.assertFalse(dataset.exists()) - dataset.create() + + @retry(Forbidden, tries=3, delay=20) + def create_dataset(): + dataset.create() + create_dataset() self.to_delete.append(dataset) TABLE_NAME = 'test_table' full_name = bigquery.SchemaField('full_name', 'STRING', diff --git a/system_tests/retry.py b/system_tests/retry.py new file mode 100644 index 000000000000..c5c1e57e71e1 --- /dev/null +++ b/system_tests/retry.py @@ -0,0 +1,49 @@ +import time +from functools import wraps + + +def retry(exception, tries=4, delay=3, backoff=2, logger=None): + """Retry calling the decorated function using an exponential backoff. + + :type exception: Exception or tuple + :param exception: the exception to check. may be a tuple of + exceptions to check + + :type tries: int + :param tries: number of times to try (not retry) before giving up + + :type delay: int + :param delay: initial delay between retries in seconds + + :type backoff: int + :param backoff: backoff multiplier e.g. value of 2 will double the delay + each retry + + :type logger: logging.Logger instance + :param logger: logger to use. If None, print + + :rtype: func + :returns: Retry wrapper function. + """ + def retry_decorator(f): + + @wraps(f) + def f_retry(*args, **kwargs): + mtries, mdelay = tries, delay + while mtries > 1: + try: + return f(*args, **kwargs) + except exception as e: + msg = "%s, Retrying in %d seconds..." % (str(e), mdelay) + if logger: + logger.warning(msg) + else: + print(msg) + time.sleep(mdelay) + mtries -= 1 + mdelay *= backoff + return f(*args, **kwargs) + + return f_retry + + return retry_decorator From 1de7a5ce997f173f2455b4fd67d1e0ff813aa4cf Mon Sep 17 00:00:00 2001 From: Thomas Schultz Date: Mon, 1 Aug 2016 17:34:17 -0400 Subject: [PATCH 2/6] Feedback updates. --- system_tests/bigquery.py | 7 +++++++ system_tests/retry.py | 16 ++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/system_tests/bigquery.py b/system_tests/bigquery.py index a8812e5bacb8..f9120ca87745 100644 --- a/system_tests/bigquery.py +++ b/system_tests/bigquery.py @@ -93,6 +93,9 @@ def test_update_dataset(self): if grant.entity_id != 'projectWriters'] dataset.access_grants = after + # We should try and keep `delay` and tries as low as possible to + # reduce test running time. tries=3 and delay=30 worked consistenly + # when updating a dataset. @retry(Forbidden, tries=3, delay=30) def update_dataset(): dataset.update() @@ -196,9 +199,13 @@ def test_update_table(self): dataset = Config.CLIENT.dataset(DATASET_NAME) self.assertFalse(dataset.exists()) + # We should try and keep `delay` and tries as low as possible to + # reduce test running time. tries=3 and delay=20 worked consistenly + # when creating a dataset. @retry(Forbidden, tries=3, delay=20) def create_dataset(): dataset.create() + create_dataset() self.to_delete.append(dataset) TABLE_NAME = 'test_table' diff --git a/system_tests/retry.py b/system_tests/retry.py index c5c1e57e71e1..82979b4a5739 100644 --- a/system_tests/retry.py +++ b/system_tests/retry.py @@ -5,22 +5,22 @@ def retry(exception, tries=4, delay=3, backoff=2, logger=None): """Retry calling the decorated function using an exponential backoff. - :type exception: Exception or tuple - :param exception: the exception to check. may be a tuple of - exceptions to check + :type exception: Exception or tuple of Exceptions + :param exception: The exception to check or may be a tuple of + exceptions to check. :type tries: int - :param tries: number of times to try (not retry) before giving up + :param tries: Number of times to try (not retry) before giving up. :type delay: int - :param delay: initial delay between retries in seconds + :param delay: Initial delay between retries in seconds. :type backoff: int - :param backoff: backoff multiplier e.g. value of 2 will double the delay - each retry + :param backoff: Backoff multiplier e.g. value of 2 will double the delay + each retry. :type logger: logging.Logger instance - :param logger: logger to use. If None, print + :param logger: Logger to use. If None, print. :rtype: func :returns: Retry wrapper function. From 49b302703d19d725f43260e40fe2a8928be149de Mon Sep 17 00:00:00 2001 From: Thomas Schultz Date: Tue, 2 Aug 2016 15:46:59 -0400 Subject: [PATCH 3/6] Switch retry to Class style decorator. --- system_tests/bigquery.py | 6 +-- system_tests/retry.py | 87 +++++++++++++++++++++++----------------- 2 files changed, 53 insertions(+), 40 deletions(-) diff --git a/system_tests/bigquery.py b/system_tests/bigquery.py index f9120ca87745..ab875b7f736d 100644 --- a/system_tests/bigquery.py +++ b/system_tests/bigquery.py @@ -22,7 +22,7 @@ from gcloud import bigquery from gcloud.exceptions import Forbidden -from retry import retry +from retry import Retry from system_test_utils import unique_resource_id @@ -96,7 +96,7 @@ def test_update_dataset(self): # We should try and keep `delay` and tries as low as possible to # reduce test running time. tries=3 and delay=30 worked consistenly # when updating a dataset. - @retry(Forbidden, tries=3, delay=30) + @Retry(Forbidden, tries=2, delay=30) def update_dataset(): dataset.update() @@ -202,7 +202,7 @@ def test_update_table(self): # We should try and keep `delay` and tries as low as possible to # reduce test running time. tries=3 and delay=20 worked consistenly # when creating a dataset. - @retry(Forbidden, tries=3, delay=20) + @Retry(Forbidden, tries=2, delay=30) def create_dataset(): dataset.create() diff --git a/system_tests/retry.py b/system_tests/retry.py index 82979b4a5739..47cd33d16162 100644 --- a/system_tests/retry.py +++ b/system_tests/retry.py @@ -1,49 +1,62 @@ import time from functools import wraps +import six -def retry(exception, tries=4, delay=3, backoff=2, logger=None): - """Retry calling the decorated function using an exponential backoff. - :type exception: Exception or tuple of Exceptions - :param exception: The exception to check or may be a tuple of - exceptions to check. +class Retry(object): + """Retry class for retrying eventually consistent resources in testing.""" - :type tries: int - :param tries: Number of times to try (not retry) before giving up. + logger = None + exception = None + tries = None + delay = None + backoff = None - :type delay: int - :param delay: Initial delay between retries in seconds. + def __init__(self, exception, tries=4, delay=3, backoff=2, logger=None): + """Retry calling the decorated function using an exponential backoff. - :type backoff: int - :param backoff: Backoff multiplier e.g. value of 2 will double the delay - each retry. + :type exception: Exception or tuple of Exceptions + :param exception: The exception to check or may be a tuple of + exceptions to check. - :type logger: logging.Logger instance - :param logger: Logger to use. If None, print. + :type tries: int + :param tries: Number of times to try (not retry) before giving up. - :rtype: func - :returns: Retry wrapper function. - """ - def retry_decorator(f): + :type delay: int + :param delay: Initial delay between retries in seconds. - @wraps(f) - def f_retry(*args, **kwargs): - mtries, mdelay = tries, delay - while mtries > 1: + :type backoff: int + :param backoff: Backoff multiplier e.g. value of 2 will double the + delay each retry. + + :type logger: logging.Logger instance + :param logger: Logger to use. If None, print. + + :rtype: func + :returns: Retry wrapper function. + """ + + self.exception = exception + self.tries = tries + self.delay = delay + self.backoff = backoff + self.logger = logger.warning if self.logger else six.print_ + + def __call__(self, to_wrap): + @wraps(to_wrap) + def wrapped_function(*args, **kwargs): + while self.tries > 1: try: - return f(*args, **kwargs) - except exception as e: - msg = "%s, Retrying in %d seconds..." % (str(e), mdelay) - if logger: - logger.warning(msg) - else: - print(msg) - time.sleep(mdelay) - mtries -= 1 - mdelay *= backoff - return f(*args, **kwargs) - - return f_retry - - return retry_decorator + return to_wrap(*args, **kwargs) + except self.exception as caught_exception: + msg = ("%s, Trying again in %d seconds..." % + (str(caught_exception), self.delay)) + self.logger(msg) + + time.sleep(self.delay) + self.tries -= 1 + self.delay *= self.backoff + return to_wrap(*args, **kwargs) + + return wrapped_function From c5324449da335d8af008e780bbbae34a23bbfe6f Mon Sep 17 00:00:00 2001 From: Thomas Schultz Date: Tue, 2 Aug 2016 16:22:50 -0400 Subject: [PATCH 4/6] Fix tries counter and change comments. --- system_tests/bigquery.py | 12 ++++++------ system_tests/retry.py | 16 ++++------------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/system_tests/bigquery.py b/system_tests/bigquery.py index ab875b7f736d..0a822c4e9768 100644 --- a/system_tests/bigquery.py +++ b/system_tests/bigquery.py @@ -93,9 +93,9 @@ def test_update_dataset(self): if grant.entity_id != 'projectWriters'] dataset.access_grants = after - # We should try and keep `delay` and tries as low as possible to - # reduce test running time. tries=3 and delay=30 worked consistenly - # when updating a dataset. + # We need to wait for the changes in the dataset to propgate and be + # eventually consistent. The alternative outcome is a 403 Forbidden + # response from upstream. @Retry(Forbidden, tries=2, delay=30) def update_dataset(): dataset.update() @@ -199,9 +199,9 @@ def test_update_table(self): dataset = Config.CLIENT.dataset(DATASET_NAME) self.assertFalse(dataset.exists()) - # We should try and keep `delay` and tries as low as possible to - # reduce test running time. tries=3 and delay=20 worked consistenly - # when creating a dataset. + # We need to wait for the changes in the dataset to propgate and be + # eventually consistent. The alternative outcome is a 403 Forbidden + # response from upstream. @Retry(Forbidden, tries=2, delay=30) def create_dataset(): dataset.create() diff --git a/system_tests/retry.py b/system_tests/retry.py index 47cd33d16162..3da38fac877b 100644 --- a/system_tests/retry.py +++ b/system_tests/retry.py @@ -7,12 +7,6 @@ class Retry(object): """Retry class for retrying eventually consistent resources in testing.""" - logger = None - exception = None - tries = None - delay = None - backoff = None - def __init__(self, exception, tries=4, delay=3, backoff=2, logger=None): """Retry calling the decorated function using an exponential backoff. @@ -32,21 +26,19 @@ def __init__(self, exception, tries=4, delay=3, backoff=2, logger=None): :type logger: logging.Logger instance :param logger: Logger to use. If None, print. - - :rtype: func - :returns: Retry wrapper function. """ self.exception = exception self.tries = tries self.delay = delay self.backoff = backoff - self.logger = logger.warning if self.logger else six.print_ + self.logger = logger.warning if logger else six.print_ def __call__(self, to_wrap): @wraps(to_wrap) def wrapped_function(*args, **kwargs): - while self.tries > 1: + tries_counter = self.tries + while tries_counter > 0: try: return to_wrap(*args, **kwargs) except self.exception as caught_exception: @@ -55,7 +47,7 @@ def wrapped_function(*args, **kwargs): self.logger(msg) time.sleep(self.delay) - self.tries -= 1 + tries_counter -= 1 self.delay *= self.backoff return to_wrap(*args, **kwargs) From f33b2ea4f123b574a3029ea090dc56d63330d9c8 Mon Sep 17 00:00:00 2001 From: Thomas Schultz Date: Tue, 2 Aug 2016 16:44:49 -0400 Subject: [PATCH 5/6] Scoping the settings. --- system_tests/bigquery.py | 4 ++-- system_tests/retry.py | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/system_tests/bigquery.py b/system_tests/bigquery.py index 0a822c4e9768..bb1b9e778cd5 100644 --- a/system_tests/bigquery.py +++ b/system_tests/bigquery.py @@ -95,7 +95,7 @@ def test_update_dataset(self): # We need to wait for the changes in the dataset to propgate and be # eventually consistent. The alternative outcome is a 403 Forbidden - # response from upstream. + # response from upstream due to upstream rate limiting. @Retry(Forbidden, tries=2, delay=30) def update_dataset(): dataset.update() @@ -201,7 +201,7 @@ def test_update_table(self): # We need to wait for the changes in the dataset to propgate and be # eventually consistent. The alternative outcome is a 403 Forbidden - # response from upstream. + # response from upstream due to upstream rate limiting. @Retry(Forbidden, tries=2, delay=30) def create_dataset(): dataset.create() diff --git a/system_tests/retry.py b/system_tests/retry.py index 3da38fac877b..db046dfe414a 100644 --- a/system_tests/retry.py +++ b/system_tests/retry.py @@ -38,17 +38,20 @@ def __call__(self, to_wrap): @wraps(to_wrap) def wrapped_function(*args, **kwargs): tries_counter = self.tries + exception = self.exception + delay = self.delay + backoff = self.backoff while tries_counter > 0: try: return to_wrap(*args, **kwargs) - except self.exception as caught_exception: + except exception as caught_exception: msg = ("%s, Trying again in %d seconds..." % - (str(caught_exception), self.delay)) + (str(caught_exception), delay)) self.logger(msg) - time.sleep(self.delay) + time.sleep(delay) tries_counter -= 1 - self.delay *= self.backoff + delay *= backoff return to_wrap(*args, **kwargs) return wrapped_function From e06815d74261a3009d43cea1e7b6529b61e1ee87 Mon Sep 17 00:00:00 2001 From: Thomas Schultz Date: Tue, 2 Aug 2016 18:25:25 -0400 Subject: [PATCH 6/6] Change comments, Retry class properties. --- system_tests/bigquery.py | 12 ++++++------ system_tests/retry.py | 6 ++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/system_tests/bigquery.py b/system_tests/bigquery.py index bb1b9e778cd5..03de85f815e7 100644 --- a/system_tests/bigquery.py +++ b/system_tests/bigquery.py @@ -93,9 +93,9 @@ def test_update_dataset(self): if grant.entity_id != 'projectWriters'] dataset.access_grants = after - # We need to wait for the changes in the dataset to propgate and be - # eventually consistent. The alternative outcome is a 403 Forbidden - # response from upstream due to upstream rate limiting. + # We need to wait to stay within the rate limits. + # The alternative outcome is a 403 Forbidden response from upstream. + # See: https://cloud.google.com/bigquery/quota-policy @Retry(Forbidden, tries=2, delay=30) def update_dataset(): dataset.update() @@ -199,9 +199,9 @@ def test_update_table(self): dataset = Config.CLIENT.dataset(DATASET_NAME) self.assertFalse(dataset.exists()) - # We need to wait for the changes in the dataset to propgate and be - # eventually consistent. The alternative outcome is a 403 Forbidden - # response from upstream due to upstream rate limiting. + # We need to wait to stay within the rate limits. + # The alternative outcome is a 403 Forbidden response from upstream. + # See: https://cloud.google.com/bigquery/quota-policy @Retry(Forbidden, tries=2, delay=30) def create_dataset(): dataset.create() diff --git a/system_tests/retry.py b/system_tests/retry.py index db046dfe414a..8e1f01a720fe 100644 --- a/system_tests/retry.py +++ b/system_tests/retry.py @@ -38,20 +38,18 @@ def __call__(self, to_wrap): @wraps(to_wrap) def wrapped_function(*args, **kwargs): tries_counter = self.tries - exception = self.exception delay = self.delay - backoff = self.backoff while tries_counter > 0: try: return to_wrap(*args, **kwargs) - except exception as caught_exception: + except self.exception as caught_exception: msg = ("%s, Trying again in %d seconds..." % (str(caught_exception), delay)) self.logger(msg) time.sleep(delay) tries_counter -= 1 - delay *= backoff + delay *= self.backoff return to_wrap(*args, **kwargs) return wrapped_function