prototype: autocommit v2 #1357
prototype: autocommit v2 #1357
Conversation
|
|
||
| private <T> T runWithSessionRetry(Function<Session, T> callable) { | ||
| PooledSessionFuture session = getSession(); | ||
| PooledSessionFuture session = getSession(true); |
There was a problem hiding this comment.
This should not be statically true, but an input parameter for the method. This method is not a helper for retrying read/write transactions, but for retrying SessionNotFound errors that could occur for both read-only and read/write requests. (This happens when the backend has garbage collected or otherwise deleted/invalidated sessions without the client knowing about it)
| sess = sessions.poll(); | ||
| moveExpiredWriteSessionsToSessions(); | ||
| if (wantRwTxnId) { | ||
| sess = writeSessions.poll(); |
There was a problem hiding this comment.
I think that we would need to add some more logic here to prevent possible starvation in the following scenario (and similar scenarios):
- Assume that
maxSessions=10 - The client executes 10 read/write transactions in parallel (or enough in parallel to use all 10 different sessions for a r/w transaction). All these sessions have now received a transaction id from the backend, and will be added to the queue of writeSessions and not to the list of sessions.
- The client wants to execute a read-only transaction, but there are no sessions in the normal session list, and
maxSessionshas been reached. It will therefore wait indefinitely.
Also, this list of writeSessions does not seem to be considered when calculating the number of sessions currently in the pool, is not considered for sessions that should be removed from the pool because they have expired, and is not included in the logic for closing the session pool and reporting any leaked sessions. (I know it's prototype so it's logical that not all of this is implemented, but my point is that more complexity than this will still be needed to be added to the session pool.)
|
Closing for now, as some more work is needed. |
Source-Link: googleapis/synthtool@e122cb0 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:387835a1375a0049ec44e02542c844302854c732d8291bdf8e472c0ff70a8f67
Source-Link: googleapis/synthtool@e122cb0 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:387835a1375a0049ec44e02542c844302854c732d8291bdf8e472c0ff70a8f67
…oogleapis#767) Source-Link: googleapis/synthtool@e122cb0 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:387835a1375a0049ec44e02542c844302854c732d8291bdf8e472c0ff70a8f67
@thiagotnunes and @olavloite , here is a crude prototype for autocommit v2.