From 3a96293d2479a75348f424806028c9b640aff31c Mon Sep 17 00:00:00 2001
From: Ondrej Nosek <onosek@redhat.com>
Date: Tue, 22 Aug 2023 14:48:02 +0200
Subject: [PATCH 3/4] Lookaside cache operations retries
Both upload and download network operations might fail
and in this case, a retry mechanism was implemented.
In case of failure, there is a delay and another attempt(s).
Delays are increasing with every attempt.
JIRA: RHELCMP-11210
Signed-off-by: Ondrej Nosek <onosek@redhat.com>
---
pyrpkg/lookaside.py | 129 ++++++++++++++++++++++++++--------------
tests/test_lookaside.py | 12 ++--
2 files changed, 89 insertions(+), 52 deletions(-)
diff --git a/pyrpkg/lookaside.py b/pyrpkg/lookaside.py
index f94ffdb..01eee4a 100644
--- a/pyrpkg/lookaside.py
+++ b/pyrpkg/lookaside.py
@@ -14,11 +14,13 @@ way it is done by Fedora, RHEL, and other distributions maintainers.
"""
+import functools
import hashlib
import io
import logging
import os
import sys
+import time
import pycurl
import six
@@ -31,7 +33,7 @@ from .errors import (AlreadyUploadedError, DownloadError, InvalidHashType,
class CGILookasideCache(object):
"""A class to interact with a CGI-based lookaside cache"""
def __init__(self, hashtype, download_url, upload_url,
- client_cert=None, ca_cert=None):
+ client_cert=None, ca_cert=None, attempts=None, delay=None):
"""Constructor
:param str hashtype: The hash algorithm to use for uploads. (e.g 'md5')
@@ -45,12 +47,18 @@ class CGILookasideCache(object):
use for HTTPS connexions. (e.g if the server certificate is
self-signed. It defaults to None, in which case the system CA
bundle is used.
+ :param int attempts: repeat network operations after failure. The param
+ says how many tries to do. None = single attempt / no-retrying
+ :param int delay: Initial delay between network operation attempts.
+ Each attempt doubles the previous delay value. In seconds.
"""
self.hashtype = hashtype
self.download_url = download_url
self.upload_url = upload_url
self.client_cert = client_cert
self.ca_cert = ca_cert
+ self.attempts = attempts if attempts is not None and attempts > 1 else 1
+ self.delay_between_attempts = delay if delay is not None and delay >= 0 else 15
self.log = logging.getLogger(__name__)
@@ -170,20 +178,13 @@ class CGILookasideCache(object):
c.setopt(pycurl.PROGRESSFUNCTION, self.print_progress)
c.setopt(pycurl.OPT_FILETIME, True)
c.setopt(pycurl.LOW_SPEED_LIMIT, 1000)
- c.setopt(pycurl.LOW_SPEED_TIME, 300)
+ c.setopt(pycurl.LOW_SPEED_TIME, 60)
c.setopt(pycurl.FOLLOWLOCATION, 1)
- with open(outfile, 'wb') as f:
- c.setopt(pycurl.WRITEDATA, f)
- try:
- c.perform()
- tstamp = c.getinfo(pycurl.INFO_FILETIME)
- status = c.getinfo(pycurl.RESPONSE_CODE)
-
- except Exception as e:
- raise DownloadError(e)
- finally:
- c.close()
+ # call retry method directly instead of @retry decorator - this approach allows passing
+ # object's internal variables into the retry method
+ status, tstamp = self.retry(raises=DownloadError)(self.retry_download)(c, outfile)
+ c.close()
# Get back a new line, after displaying the download progress
if sys.stdout.isatty():
@@ -220,13 +221,8 @@ class CGILookasideCache(object):
c.setopt(pycurl.NOBODY, True)
c.setopt(pycurl.FOLLOWLOCATION, 1)
- try:
- c.perform()
- status = c.getinfo(pycurl.RESPONSE_CODE)
- except Exception as e:
- raise DownloadError(e)
- finally:
- c.close()
+ status = self.retry(raises=DownloadError)(self.retry_remote_file_exists_head)(c)
+ c.close()
if status != 200:
self.log.debug('Unavailable file \'%s\' at %s' % (filename, url))
@@ -275,19 +271,8 @@ class CGILookasideCache(object):
c.setopt(pycurl.HTTPAUTH, pycurl.HTTPAUTH_GSSNEGOTIATE)
c.setopt(pycurl.USERPWD, ':')
- with io.BytesIO() as buf:
- c.setopt(pycurl.WRITEFUNCTION, buf.write)
- try:
- c.perform()
- status = c.getinfo(pycurl.RESPONSE_CODE)
-
- except Exception as e:
- raise UploadError(e)
-
- finally:
- c.close()
-
- output = buf.getvalue().strip()
+ status, output = self.retry(raises=UploadError)(self.retry_remote_file_exists)(c)
+ c.close()
if status != 200:
self.raise_upload_error(status)
@@ -363,19 +348,8 @@ class CGILookasideCache(object):
c.setopt(pycurl.HTTPAUTH, pycurl.HTTPAUTH_GSSNEGOTIATE)
c.setopt(pycurl.USERPWD, ':')
- with io.BytesIO() as buf:
- c.setopt(pycurl.WRITEFUNCTION, buf.write)
- try:
- c.perform()
- status = c.getinfo(pycurl.RESPONSE_CODE)
-
- except Exception as e:
- raise UploadError(e)
-
- finally:
- c.close()
-
- output = buf.getvalue().strip()
+ status, output = self.retry(raises=UploadError)(self.retry_upload)(c)
+ c.close()
# Get back a new line, after displaying the download progress
if sys.stdout.isatty():
@@ -387,3 +361,66 @@ class CGILookasideCache(object):
if output:
self.log.debug(output)
+
+ def retry_download(self, curl, outfile):
+ with open(outfile, 'wb') as f:
+ curl.setopt(pycurl.WRITEDATA, f)
+ curl.perform()
+ tstamp = curl.getinfo(pycurl.INFO_FILETIME)
+ status = curl.getinfo(pycurl.RESPONSE_CODE)
+ return status, tstamp
+
+ def retry_remote_file_exists_head(self, curl):
+ curl.perform()
+ status = curl.getinfo(pycurl.RESPONSE_CODE)
+ return status
+
+ def retry_remote_file_exists(self, curl):
+ with io.BytesIO() as buf:
+ curl.setopt(pycurl.WRITEFUNCTION, buf.write)
+ curl.perform()
+ status = curl.getinfo(pycurl.RESPONSE_CODE)
+ output = buf.getvalue().strip()
+ return status, output
+
+ def retry_upload(self, curl):
+ with io.BytesIO() as buf:
+ curl.setopt(pycurl.WRITEFUNCTION, buf.write)
+ curl.perform()
+ status = curl.getinfo(pycurl.RESPONSE_CODE)
+ output = buf.getvalue().strip()
+ return status, output
+
+ def retry(self, attempts=None, delay_between_attempts=None, wait_on=pycurl.error, raises=None):
+ """A decorator that allows to retry a section of code until success or counter elapses
+ """
+
+ def wrapper(function):
+ @functools.wraps(function)
+ def inner(*args, **kwargs):
+
+ attempts_all = attempts or self.attempts
+ attempts_left = attempts_all
+ delay = delay_between_attempts or self.delay_between_attempts
+ while attempts_left > 0:
+ try:
+ return function(*args, **kwargs)
+ except wait_on as e:
+ self.log.warn("Network error: %s" % (e))
+ attempts_left -= 1
+ self.log.debug("Attempt %d/%d has failed."
+ % (attempts_all - attempts_left, attempts_all))
+ if attempts_left:
+ self.log.info("The operation will be retried in %ds." % (delay))
+ time.sleep(delay)
+ delay *= 2
+ self.log.info("Retrying ...")
+ else:
+ if raises is None:
+ raise # This re-raises the last exception.
+ else:
+ raise raises(e)
+
+ return inner
+
+ return wrapper
diff --git a/tests/test_lookaside.py b/tests/test_lookaside.py
index 35d3499..2fe1bdb 100644
--- a/tests/test_lookaside.py
+++ b/tests/test_lookaside.py
@@ -175,7 +175,7 @@ class CGILookasideCacheTestCase(unittest.TestCase):
return 200 if info == pycurl.RESPONSE_CODE else 0
def mock_perform():
- with open(self.filename) as f:
+ with open(self.filename, "rb") as f:
curlopts[pycurl.WRITEDATA].write(f.read())
def mock_setopt(opt, value):
@@ -200,7 +200,7 @@ class CGILookasideCacheTestCase(unittest.TestCase):
@mock.patch('pyrpkg.lookaside.pycurl.Curl')
def test_download_failed(self, mock_curl):
curl = mock_curl.return_value
- curl.perform.side_effect = Exception(
+ curl.perform.side_effect = pycurl.error(
'Could not resolve host: example.com')
with open(self.filename, 'wb') as f:
@@ -219,7 +219,7 @@ class CGILookasideCacheTestCase(unittest.TestCase):
return 500 if info == pycurl.RESPONSE_CODE else 0
def mock_perform():
- with open(self.filename) as f:
+ with open(self.filename, "rb") as f:
curlopts[pycurl.WRITEDATA].write(f.read())
def mock_setopt(opt, value):
@@ -424,7 +424,7 @@ class CGILookasideCacheTestCase(unittest.TestCase):
@mock.patch('pyrpkg.lookaside.pycurl.Curl')
def test_remote_file_exists_check_failed(self, mock_curl):
curl = mock_curl.return_value
- curl.perform.side_effect = Exception(
+ curl.perform.side_effect = pycurl.error(
'Could not resolve host: example.com')
lc = CGILookasideCache('_', '_', '_')
@@ -452,7 +452,7 @@ class CGILookasideCacheTestCase(unittest.TestCase):
@mock.patch('pyrpkg.lookaside.pycurl.Curl')
def test_remote_file_exists_check_unexpected_error(self, mock_curl):
def mock_perform():
- curlopts[pycurl.WRITEFUNCTION]('Something unexpected')
+ curlopts[pycurl.WRITEFUNCTION](b'Something unexpected')
def mock_setopt(opt, value):
curlopts[opt] = value
@@ -590,7 +590,7 @@ class CGILookasideCacheTestCase(unittest.TestCase):
@mock.patch('pyrpkg.lookaside.pycurl.Curl')
def test_upload_failed(self, mock_curl):
curl = mock_curl.return_value
- curl.perform.side_effect = Exception(
+ curl.perform.side_effect = pycurl.error(
'Could not resolve host: example.com')
lc = CGILookasideCache('_', '_', '_')
--
2.41.0