refactor: PingingPool pings sessions using SELECT 1#75
refactor: PingingPool pings sessions using SELECT 1#75gcf-merge-on-green[bot] merged 6 commits intogoogleapis:masterfrom
Conversation
hengfengli
left a comment
There was a problem hiding this comment.
Generally, it looks good to me. Please resolve the comments.
hengfengli
left a comment
There was a problem hiding this comment.
LGTM. So when a session is deleted, calling session.ping() will cause a NotFound error, is this right?
Yes. However there seems to be a delay before the error appears. Given the following code, the error only occurs when DELAY is over ~35s |
|
It makes sense. The deletion needs time to be propagated. So if a session is deleted, when we still execute a transaction on it, what will happen then? When it gets a SessionNotFound error, will it try to get a new session? |
Currently, it will not. However, if we keep the |
hengfengli
left a comment
There was a problem hiding this comment.
LGTM. So now, pool.get() uses session.exists() to verify the validness of a session instead of session.ping(). Can we leave a comment over there why we don't use session.ping()?
The reason why we don't use session.ping() is that it has a cached/outdated result, which is inaccurate, is this correct?
Good suggestion! Done :)
Yes, the backend uses a cached value which means a delete may have occurred but not propagated yet. |
Currently, PingingPool pings sessions in the background by calling
session.exists()which callsGetSession.Using
SELECT 1is preferred and is used in other client libraries such as Go: