Conversation
Most errors that occur during preparing a session for a read/write transaction should be considered permanent, and should stop the automatic preparing of sessions. Any subsequent call to get a read/write session will cause an in-process BeginTransaction RPC to be executed. If the problem has been fixed in the meantime, the RPC will succeed and the automatic prepare of sessions will start again. Otherwise, the error is propagated to the user. Fixes #177
388827a to 3cfe106 Compare | // Remove the session from the pool. | ||
| if (isClosed()) { | ||
| decrementPendingClosures(1); | ||
| } | ||
| allSessions.remove(session); | ||
| this.resourceNotFoundException = | ||
| MoreObjects.firstNonNull( | ||
| this.resourceNotFoundException, (ResourceNotFoundException) e); |
There was a problem hiding this comment.
In the previous logic, we executed this logic also for PERMISSION_DENIED errors. It doesn't anymore. Is this a problem?
There was a problem hiding this comment.
No, it is still executed for PERMISSION_DENIED errors, but that is now covered by the general error handling that also handles other types of errors. The specific errors DatabaseNotFound and InstanceNotFound are handled specifically, as these should not allow the normal operation of the session pool to be restarted once the error does no longer occur. The reasoning behind that is that a DatabaseNotFound exception can only be fixed by creating a new database with the same name as the old one, but that is still a new database.
The other errors, such as PERMISSION_DENIED and now also for examle FAILED_PRECONDITION, should allow the session pool to restart the automatic preparing of sessions once the error has been fixed. This could be granting the user the necessary permissions or changing the state of database to fulfill all requirements.
| @@ -469,12 +469,25 @@ public void testDatabaseOrInstanceDoesNotExistOnReplenish() throws Exception { | |||
| @Test | |||
| public void testPermissionDeniedOnPrepareSession() throws Exception { | |||
There was a problem hiding this comment.
This test case covers the PERMISSION_DENIED case discussed above.
Thanks for the clarification @olavloite. Please merge when you're ready. |
Most errors that occur during preparing a session for a read/write transaction should be considered permanent, and should stop the automatic preparing of sessions. Any subsequent call to get a read/write session will cause an in-process BeginTransaction RPC to be executed. If the problem has been fixed in the meantime, the RPC will succeed and the automatic prepare of sessions will start again. Otherwise, the error is propagated to the user.
Fixes #177