Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Eliminate unecessary logging on restore #43

Merged
merged 1 commit into from

2 participants

@bshi
Collaborator

Resolves #38

Tested on ubuntu 12.04 / boto 2.2

@fdr
Owner

Oh wait no I'm just blind. Checking patch...

@fdr
Owner

LGTM, squash the two together before commit if you would, please: it'd be easier to read in one shot.

@bshi bshi Eliminate unecessary logging on restore
Tweak race error message by bumping severity to WARNING and note in the
message copy that the URI at one point existed but can no longer be
found.
879124a
@bshi bshi merged commit 473b2b3 into wal-e:master
@bshi
Collaborator

Squashed. Well balls, github decided also purge the review history :/

@fdr
Owner
@bshi bshi deleted the bshi:wale-38 branch
@fdr
Owner

I recently realized this patch probably makes an extra S3 API request (for pre-checking .exists) when it could be avoided. That kind of sucks for the common case, given the expense of a TLS handshake.

Well, let's leave it in, but it seems like retaining the advantages but getting rid of the ugly stack trace ought to be possible.

@bshi
Collaborator

Hrm, good point. I was under the impression that boto did some rudimentary connection pooling. Keep-alive would at least alleviate the handshake overhead (if not the extra request) no?

@fdr
Owner
@bshi
Collaborator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 19, 2013
  1. @bshi

    Eliminate unecessary logging on restore

    bshi authored
    Tweak race error message by bumping severity to WARNING and note in the
    message copy that the URI at one point existed but can no longer be
    found.
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 11 deletions.
  1. +20 −11 wal_e/worker/s3_worker.py
View
31 wal_e/worker/s3_worker.py
@@ -312,8 +312,20 @@ def standard_detail_message(prefix=''):
@retry(retry_with_count(log_wal_fetch_failures_on_error))
def download():
+ missing_uri_hint = ('This can be normal when Postgres is trying to '
+ 'detect what timelines are available during '
+ 'restoration.')
with open(path, 'wb') as decomp_out:
key = uri_to_key(aws_access_key_id, aws_secret_access_key, s3_url)
+ if not key.exists():
+ # Do not retry if the key not present, this can happen
+ # under normal situations.
+ logger.info(
+ msg='could not locate object while performing wal restore',
+ detail=('The absolute URI that could not be located '
+ 'is {url}.'.format(url=s3_url)),
+ hint=missing_uri_hint)
+ return False
pipeline = get_download_pipeline(PIPE, decomp_out, decrypt)
g = gevent.spawn(write_and_close_thread, key, pipeline.stdin)
@@ -323,17 +335,14 @@ def download():
g.get()
except boto.exception.S3ResponseError, e:
if e.status == 404:
- # Do not retry if the key not present, this can happen
- # under normal situations.
- logger.info(
- msg=('could not locate object while performing wal '
- 'restore'),
- detail=('The absolute URI that could not be located '
- 'is {url}.'.format(url=s3_url)),
- hint=('This can be normal when Postgres is trying to '
- 'detect what timelines are available during '
- 'restoration.'))
-
+ # Short circuit any re-try attempts under certain race
+ # conditions.
+ logger.warn(
+ msg=('could no longer locate object while performing '
+ 'wal restore'),
+ detail=('The URI at {url} no longer '
+ 'exists.'.format(url=s3_url)),
+ hint=missing_uri_hint)
return False
else:
raise
Something went wrong with that request. Please try again.