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

Improve handling of Google App Engine connection reset events. #620

Closed
crwilcox opened this issue Mar 29, 2021 · 4 comments · Fixed by #645
Closed

Improve handling of Google App Engine connection reset events. #620

crwilcox opened this issue Mar 29, 2021 · 4 comments · Fixed by #645
Labels
api: datastore Issues related to the googleapis/python-ndb API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@crwilcox
Copy link
Collaborator

The GAE standard environment can have transient connection reset events. When using redis-py, it is typically recommended that the user wrap the redis-py client with something that can handle and retry on connection resets. It appears that a redis-py instance can be passed to Cloud NDB but this feels like an experience that could be improved?

On the whole, connection resets are somewhat a fact of life in GAE standard net stack, and users of redis-py connection pools (NDB library looks like one of those "users") frequently grapple with this issue. The ask is whether we might make NDB more resilient (can NDB retry?) in the face of the inevitable resets.

@product-auto-label product-auto-label bot added the api: datastore Issues related to the googleapis/python-ndb API. label Mar 29, 2021
@crwilcox crwilcox added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed api: datastore Issues related to the googleapis/python-ndb API. labels Mar 29, 2021
@chrisrossi
Copy link
Contributor

Possibly this has already been addressed and we just need a release. Or do the specific errors you're discussing not get caught by this?

@crwilcox
Copy link
Collaborator Author

The customer who let me know about this states this does look like it may be the cause and solution.

Glad to do a release. Do you think there are any upcoming changes worth waiting on?

@crwilcox
Copy link
Collaborator Author

crwilcox commented Mar 30, 2021

@chrisrossi a thought came up in conversation: What happens if the default pool size is overridden to be more than 5?

The concern is that, 5 retries is hardcoded, but if a reset happened and there were 5 or more connections, we would exhaust retries before creating a new connection. Is this a concern? If so maybe NDB could calculate # of retries to always exceed the pool? Maybe pool size + 5?

Example:

  • 4 connections exist in the pool
  • stream loss event occurs, and all connections need to be reset on an instance
  • first connection will reset, select next existing connection from pool 3 times
  • this leaves 1 retry

@product-auto-label product-auto-label bot added the api: datastore Issues related to the googleapis/python-ndb API. label Apr 7, 2021
@justinkwaugh
Copy link

I notice with the latest release (v1.8.0) running in an instance on app engine standard that has no load other than me doing light testing that I still get cache flushes regularly because of connection reset events. Definitely edging toward a showstopper for us migrating to python 3.

chrisrossi pushed a commit to chrisrossi/python-ndb that referenced this issue May 7, 2021
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 pushed a commit that referenced this issue May 10, 2021
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
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. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants