FailoverConnectionPlugin SessionStateTransferCallable capability

[scott-long-adp]

hi -- We are trying to validate the SessionStateTransferCallable configuration

It looks like SessionStateTransferCallable is called only by switchCurrentConnectionTo(), which requires a non-null value for this.pluginService.getCurrentConnection() in order to call transferSessionState() to copy properties from the old connection to the new.

But switchCurrentConnectionTo() is only actually called when this.pluginService.getCurrentConnection() is null.

This, I think, means switchCurrentConnectionTo() will never call transferSessionState() to copy properties from the old connection to the new.

As a result the failover just always goes through failoverReader() or failoverWriter(), which functionally does recover, but the switchCurrentConnectionTo() path is never taken, as far as I can tell --

Am I reading this correctly?

https://github.com/awslabs/aws-advanced-jdbc-wrapper/blob/main/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java#L863

[brunos-bq]

Hello @scott-long-adp,

As you have mentioned, SessionStateTransferCallable transfer connection properties when switchCurrentConnectionTo() is called. switchCurrentConnectionTo() is usually called when pickNewConnection() is called, and a the new connection then gets all its properties transfered. Meanwhile, RestoreSessionStateCallable will set locally stored properties in a new connection during a writer or reader failover, when failoverReader() and failoverWriter() is called and when the original connection is lost.

One of the questions here on whether they should both transfer the same set of attributes, is what are the attributes do we believe the driver should keep track internally in the case of a change in a connection. Could you provide a more in-depth description of your use case so we could better understand which properties you might want to carry over through connections.

Thanks!

[scott-long-adp]

thanks @brunos-bq -- the main properties we would transfer are readonly, autocommit, and transaction isolation.

But my question is different -- it looks to me like it is impossible for SessionStateTransferCallable, or even the default SessionStateHelper.transferSessionState() to ever be called in the first place.

SessionStateTransferCallable is called by transferSessionState() if this.pluginService.getCurrentConnection() is non-null when switchCurrentConnectionTo() is called.

However switchCurrentConnectionTo() is only called when this.pluginService.getCurrentConnection() is null (instead of non-null)

Therefore it looks like it is impossible for SessionStateTransferCallable to ever be invoked.

Am i reading the code correctly?

[wettowelreactor]

SessionStateTransferCallable is called by transferSessionState() if this.pluginService.getCurrentConnection() is non-null when switchCurrentConnectionTo() is called.

transferSessionState() is called by switchCurrentConnectionTo() when this.pluginService.getCurrentConnection() is not equal to the connection passed into switchCurrentConnectionTo(). [1] (https://github.com/awslabs/aws-advanced-jdbc-wrapper/blob/9599da2e6072a1e0e8307333bdf3cbe13e25d5ad/wrapper/src/main/java/software/amazon/jdbc/plugin/failover/FailoverConnectionPlugin.java#L550)

Where are you seeing that it only gets called when this.pluginService.getCurrentConnection() is non-null?

[scott-long-adp]

thanks -- yes the issue is getting it to call switchCurrentConnectionTo()

switchCurrentConnectionTo() is only called by connectTo()
connectTo() is only called by pickNewConnection(), and only if the current connection is null.

This seems to be why the failover path that just discards the connection and establishes a new one in the pool is the only recovery we are seeing in testing.

It does recover by discarding the connection and we come in and create a new connection for the pool in the normal manner, which may actually be ok, it just doesn't appear to take the path of replacing a wrapped connection and keeping it in the pool in any of our tests, and looking at the code this may be the reason.

[brunos-bq]

Hello @scott-long-adp,

Thank you for reporting this issue. We might have to look a bit back into it to figure out how both functions are working and whether there is an issue with the connection switch. We have opened an internal investigation on that and will keep you updated about it.

Thanks!

[scott-long-adp]

thanks @brunos-bq -- let me know what you learn. Our testing so far with mysql and RDS failover seems to show both SessionStateTransferCallable and RestoreSessionStateCallable paths not being taken and just falling thru to create the new connection via the normal pool replenishment. It is failing over fine and recovering, just not taking advantage of these two possible short-circuits.

[jpollard-cs]

I found this all a bit confusing, as well. It's worth pointing out that this has since had a significant refactor. My use case was that I am using the aws-advanced-jdbc-wrapper with HikariCP and have an initial session state configured to be applied for all connections in the pool (e.g. I might have an initialization query that sets a max execution time for connections in a read pool), but the wrapper does not keep track of all relevant session state changes. I suspect this is the more common use case for most folks. If you have some bit of session state that you're not setting at initialization and for the lifetime of the connection or that you expect to live beyond the scope of a single transaction (which won't live across a failover anyways) I'd suspect that to be a fairly exotic use case? Initially I thought I may need to use the new Driver.setTransferSessionStateOnSwitchFunc, but then I came across this doc which seems to indicate you simply need to use the AwsWrapperDataSource to achieve this. Again assuming this is likely the more common use case and my understanding is correct I'd be happy to update the docs to help clarify.