Skip to content

Conversation

@egonelbre
Copy link
Contributor

When all connnections attached to the connector closed then the spanner
client and admin client was closed. This is a problem, because the
database may still hold on to the connector and may want to make new
connections.

This changes the logic such that the connector can reconnect the client
when necessary.

When all connnections attached to the connector closed then the spanner
client and admin client was closed. This is a problem, because the
database may still hold on to the connector and may want to make new
connections.

This changes the logic such that the connector can reconnect the client
when necessary.

Fixes googleapis#288
@egonelbre egonelbre marked this pull request as ready for review August 29, 2024 11:22
@egonelbre egonelbre requested a review from a team as a code owner August 29, 2024 11:22
olavloite and others added 5 commits August 29, 2024 14:39
Implement the Closer interface for connector and use that to remove it
from the list of active connectors. Closing all idle connections of a
connector will close the underlying clients, but allow these to be
re-initialized if the connector is used again.

Re-using a connector that has been closed is not possible.
@olavloite
Copy link
Collaborator

@egonelbre I made a small change to this PR, as removing/adding the connector from the map of driver connectors when all connections were closed caused some flaky test failures, probably due to underlying connections being closed right at the same time as that a new connection was being requested for the same connector.
Instead, I've added a Close() method to connector, so it now implements the Closer interface. This method will automatically be called when Database.Close() is called, and this method removes a connector from the map of active connectors.

@olavloite olavloite merged commit b0cdd9d into googleapis:main Aug 29, 2024
@egonelbre egonelbre deleted the fixidle branch August 29, 2024 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants