Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: retry connection errors with memcache #645

Merged
merged 1 commit into from
May 10, 2021

Conversation

chrisrossi
Copy link
Contributor

The most common transient error with memcache is ConnectionResetError,
which wasn't included in exceptions to retry. Now all connection errors
are retried.

Fixes #620

The most common transient error with memcache is `ConnectionResetError`,
which wasn't included in exceptions to retry. Now all connection errors
are retried.

Fixes googleapis#620
@chrisrossi chrisrossi requested a review from andrewsg as a code owner May 7, 2021 13:34
@chrisrossi chrisrossi requested a review from a team May 7, 2021 13:34
@chrisrossi chrisrossi requested a review from a team as a code owner May 7, 2021 13:34
@product-auto-label product-auto-label bot added the api: datastore Issues related to the googleapis/python-ndb API. label May 7, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 7, 2021
@chrisrossi chrisrossi requested review from craiglabenz and crwilcox May 7, 2021 13:35
@@ -27,6 +27,10 @@
import pymemcache
import redis as redis_module

# Python 2.7 doesn't have ConnectionError. In Python 3, ConnectionError is subclass of
# OSError, which Python 2.7 does have.
ConnectionError = getattr(__builtins__, "ConnectionError", OSError)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do redis / memchache raise a bare OSError on 2.7?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure. I'll see if I can flush out the actual behavior.

Copy link
Contributor Author

@chrisrossi chrisrossi May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really able to reproduce the error with the Python 2.7 runtime. It would seem that the underlying issue causing all the ConnectionResetError events with memcache is unique to the Python 3 runtime on App Engine. I think erring on the side of a broad catch, here, is OK.

@chrisrossi chrisrossi merged commit 06b466a into googleapis:master May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/python-ndb API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve handling of Google App Engine connection reset events.
2 participants