FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: add NOT_FOUND error check in __exit__ method of SessionCheckout. by vi3k6i5 · Pull Request #718 · googleapis/python-spanner · GitHub

This repository was archived by the owner on Jun 8, 2026. It is now read-only.
/ python-spanner Public archive

fix: add NOT_FOUND error check in __exit__ method of SessionCheckout. - #718

Merged
vi3k6i5 merged 4 commits into
googleapis:mainfrom
vi3k6i5:ft-pool
Apr 20, 2022
Merged

vi3k6i5 merged 4 commits into
googleapis:mainfrom
vi3k6i5:ft-pool

Conversation

vi3k6i5 commented Apr 19, 2022

Copy link
Copy Markdown
Contributor

PingingPool currently checks if session exists in .get() method only when _NOW() > ping_after. If the session is used in any query and then returned the pool then the ping_after is reset to _NOW() _ + delta. If the session is deleted in the backend and the checkout gives a not found error the session will still get returned to the pool with a reset for ping_after = _NOW() _ + delta.

As described in the comment in get() method.

Using session.exists() guarantees the returned session exists.
session.ping() uses a cached result in the backend which could
result in a recently deleted session being returned.

Example:
An Application using PingingPool with 1 Session. The session has been killed on the server side, but the ping method has not cleared out the session from the pool yet because ping_after > _NOW() is not True.

Now pool.get() will return the session, and SnapshotCheckout will use the session and fail and return the session back into the pool.

Put Method will put the session in the pool with a new wait_time, and the process will continue.

To avoid this inside the __exit__ method of SessionCheckout we should check if NOT_FOUND error was raised or not. If that error was raised then create a new session and push it inside the pool.

…on was raised for the session and create new session if needed
vi3k6i5 requested review from a team April 19, 2022 11:22
product-auto-label Bot added size: xs Pull request size is extra small. api: spanner Issues related to the googleapis/python-spanner API. labels Apr 19, 2022
vi3k6i5 marked this pull request as draft April 19, 2022 11:36
product-auto-label Bot added size: m Pull request size is medium. and removed size: xs Pull request size is extra small. labels Apr 19, 2022
vi3k6i5 requested review from asthamohta and larkee April 19, 2022 12:46
vi3k6i5 changed the title fix: add session.exists() check in PingingPool.get() method. fix: add NOT_FOUND error check in __exit__ method of SessionCheckout. Apr 19, 2022
product-auto-label Bot added size: s Pull request size is small. and removed size: m Pull request size is medium. labels Apr 19, 2022
vi3k6i5 marked this pull request as ready for review April 19, 2022 16:46

thiagotnunes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

larkee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM 👍 One minor suggestion for another test case

product-auto-label Bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Apr 20, 2022
vi3k6i5 merged commit 265e207 into googleapis:main Apr 20, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/python-spanner API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL