Add 503 to list of retriable HTTP response codes

Environment details

OS type and version:
Python version: Python 3.11.4
pip version: pip 23.1.2 from /usr/local/lib/python3.11/site-packages/pip (python 3.11)
google-cloud-storage version: 2.14.0

Steps to reproduce

Usage of Blob.upload_from_filename() seems to sometimes encounter a 503 response

This seems to result in an immediate failure, despite indications in

python-storage/google/cloud/storage/retry.py

Lines 28 to 38 in ae9a53b

    
           _RETRYABLE_TYPES = ( 
        
               api_exceptions.TooManyRequests,  # 429 
        
               api_exceptions.InternalServerError,  # 500 
        
               api_exceptions.BadGateway,  # 502 
        
               api_exceptions.ServiceUnavailable,  # 503 
        
               api_exceptions.GatewayTimeout,  # 504 
        
               ConnectionError, 
        
               requests.ConnectionError, 
        
               requests_exceptions.ChunkedEncodingError, 
        
               requests_exceptions.Timeout, 
        
           )

that api_exceptions.ServiceUnavailable is a 503 and should be retried.

Code example

https://github.com/google/osv.dev/blob/b705d0d0b7450ce94137624118a2b54a7f719147/docker/exporter/exporter.py#L57-L64

  def upload_single(self, bucket, source_path, target_path):
    """Upload a single file to a bucket."""
    logging.info('Uploading %s', target_path)
    try:
      blob = bucket.blob(target_path)
      blob.upload_from_filename(source_path)
    except Exception as e:
      logging.error('Failed to export: %s', e)

Stack trace

Failed to export: 503 POST https://storage.googleapis.com/upload/storage/v1/b/osv-test-vulnerabilities/o?uploadType=multipart: {
  "error": {
    "code": 503,
    "message": "We encountered an internal error. Please try again.",
    "errors": [
      {
        "message": "We encountered an internal error. Please try again.",
        "domain": "global",
        "reason": "backendError"
      }
    ]
  }
}
: ('Request failed with status code', 503, 'Expected one of', <HTTPStatus.OK: 200>)

Perhaps 503 needs to be added to _ADDITIONAL_RETRYABLE_STATUS_CODES also?

As you pointed out, 503 and connection errors are considered retryable in the python client. However, retries are only enabled by default for uploads if the upload is confirmed to be idempotent - that is by including request preconditions if_generation_match on the upload method call.

The retry strategy is outlined in https://cloud.google.com/storage/docs/retry-strategy#python. This docs go into detail about which operations are conditionally idempotent, retryable exceptions and client retry config options. Here is a sample code that demonstrates adding the generation match precondition to an upload.

In your code sample, assuming the upload is for a new file, set if_generation_match=0to enable retries; same goes for #1231

def upload_single(self, bucket, source_path, target_path):
"""Upload a single file to a bucket."""
logging.info('Uploading %s', target_path)
try:
    blob = bucket.blob(target_path)
    blob.upload_from_filename(source_path, if_generation_match=0)
except Exception as e:
    logging.error('Failed to export: %s', e)

@cojenco

Hi @cojenco thank you for your fast response.

My reading of

python-storage/google/cloud/storage/blob.py

Lines 2827 to 2841 in ae9a53b

    
           def upload_from_filename( 
        
               self, 
        
               filename, 
        
               content_type=None, 
        
               num_retries=None, 
        
               client=None, 
        
               predefined_acl=None, 
        
               if_generation_match=None, 
        
               if_generation_not_match=None, 
        
               if_metageneration_match=None, 
        
               if_metageneration_not_match=None, 
        
               timeout=_DEFAULT_TIMEOUT, 
        
               checksum=None, 
        
               retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, 
        
           ):

and the definition of

python-storage/google/cloud/storage/retry.py

Lines 141 to 143 in ae9a53b

    
           DEFAULT_RETRY_IF_GENERATION_SPECIFIED = ConditionalRetryPolicy( 
        
               DEFAULT_RETRY, is_generation_specified, ["query_params"] 
        
           )

was that:

the default retry behaviour for upload_from_filename() is DEFAULT_RETRY_IF_GENERATION_SPECIFIED
the definition of DEFAULT_RETRY_IF_GENERATION_SPECIFIED is the superset of DEFAULT_RETRY and a generation precondition being met

Therefore, I am referring to the DEFAULT_RETRY behaviour being deficient for the handling of 503s and connection errors.

Have I misunderstood something?

In your code sample, assuming the upload is for a new file

No, our use case is to successfully upload new or overwrite existing files, so I don't believe that having a precondition on the generation being zero is going to comprehensively assist in this case, if I understand correctly?

@andrewpollock

Hi @andrewpollock thanks for the follow-up.

the default retry behaviour for upload_from_filename() is DEFAULT_RETRY_IF_GENERATION_SPECIFIED

the definition of DEFAULT_RETRY_IF_GENERATION_SPECIFIED is the superset of DEFAULT_RETRY and a generation precondition being met

Your understanding in (1) and (2) are accurate.

However, to clarify your following point:

Therefore, I am referring to the DEFAULT_RETRY behaviour being deficient for the handling of 503s and connection errors.

DEFAULT_RETRY does handle 503s and connection errors as long as the error is in the retryable excpetions list. As shown below, DEFAULT_RETRY = retry.Retry(predicate=_should_retry) _should_retry predicate will always retry errors that are considered retryable.

python-storage/google/cloud/storage/retry.py

Lines 45 to 61 in ae9a53b

    
           def _should_retry(exc): 
        
               """Predicate for determining when to retry.""" 
        
               if isinstance(exc, _RETRYABLE_TYPES): 
        
                   return True 
        
               elif isinstance(exc, api_exceptions.GoogleAPICallError): 
        
                   return exc.code in _ADDITIONAL_RETRYABLE_STATUS_CODES 
        
               elif isinstance(exc, auth_exceptions.TransportError): 
        
                   return _should_retry(exc.args[0]) 
        
               else: 
        
                   return False 
        
           DEFAULT_RETRY = retry.Retry(predicate=_should_retry) 
        
           """The default retry object. 
        
           This retry setting will retry all _RETRYABLE_TYPES and any status codes from 
        
           _ADDITIONAL_RETRYABLE_STATUS_CODES.

Given your use case has a mix of new and existing objects, a few options worth considering:
(a) add if_generation_match precondition using default retry strategy DEFAULT_RETRY_IF_GENERATION_SPECIFIED

blob = bucket.get_blob(target_path)
generation_match = 0

# For existing objects, get the object generation
if blob is not None:
   generation_match = blob.generation

blob.upload_from_filename(source_path, if_generation_match=generation_match)

(b) adjust the retry strategy to always retry retryable errors

blob.upload_from_filename(source_path, retry=DEFAULT_RETRY)

Setting retry=DEFAULT_RETRY on the upload method call will trigger an exponential retry for 503s and connection errors; though there is the risk of potential race conditions.

Please let me know if this doesn't answer your question.

Your understanding in (1) and (2) are accurate.

Thank you. Today, I'm wondering if this is an "and" versus "or" misunderstanding on my behalf, with respect to (2)?

DEFAULT_RETRY does handle 503s and connection errors as long as the error is in the retryable excpetions list. As shown below, DEFAULT_RETRY = retry.Retry(predicate=_should_retry) _should_retry predicate will always retry errors that are considered retryable.

This was my point. Based on the behaviour I'm reporting here and in #1231 it doesn't appear to be the case? But maybe it's because of my misunderstanding as per above?

adjust retry strategy for `blob.upload_from_filename` (#2046)

adjust retry strategy for `blob.upload_from_filename` (google#2046)

Navigation Menu

Repository navigation

Add 503 to list of retriable HTTP response codes #1232

Environment details

Steps to reproduce

Code example

Stack trace

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	_RETRYABLE_TYPES = (
	api_exceptions.TooManyRequests, # 429
	api_exceptions.InternalServerError, # 500
	api_exceptions.BadGateway, # 502
	api_exceptions.ServiceUnavailable, # 503
	api_exceptions.GatewayTimeout, # 504
	ConnectionError,
	requests.ConnectionError,
	requests_exceptions.ChunkedEncodingError,
	requests_exceptions.Timeout,
	)

	def upload_from_filename(
	self,
	filename,
	content_type=None,
	num_retries=None,
	client=None,
	predefined_acl=None,
	if_generation_match=None,
	if_generation_not_match=None,
	if_metageneration_match=None,
	if_metageneration_not_match=None,
	timeout=_DEFAULT_TIMEOUT,
	checksum=None,
	retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
	):

	DEFAULT_RETRY_IF_GENERATION_SPECIFIED = ConditionalRetryPolicy(
	DEFAULT_RETRY, is_generation_specified, ["query_params"]
	)

	def _should_retry(exc):
	"""Predicate for determining when to retry."""
	if isinstance(exc, _RETRYABLE_TYPES):
	return True
	elif isinstance(exc, api_exceptions.GoogleAPICallError):
	return exc.code in _ADDITIONAL_RETRYABLE_STATUS_CODES
	elif isinstance(exc, auth_exceptions.TransportError):
	return _should_retry(exc.args[0])
	else:
	return False


	DEFAULT_RETRY = retry.Retry(predicate=_should_retry)
	"""The default retry object.

	This retry setting will retry all _RETRYABLE_TYPES and any status codes from
	_ADDITIONAL_RETRYABLE_STATUS_CODES.

Add 503 to list of retriable HTTP response codes #1232

Description

Environment details

Steps to reproduce

Code example

Stack trace

Activity

cojenco commented on Feb 26, 2024

andrewpollock commented on Feb 27, 2024

cojenco commented on Feb 27, 2024

andrewpollock commented on Feb 27, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions