Connection pooling and incomplete transactions

[Per Lundberg]

Hi,

I have a question that I don't really see fits the GitHub issue tracker so here goes.

We have been using Lettuce for a few months in a scalability project we are working on (offloading RPC calls to use cached Redis data instead of having to call the application backend directly). This is working quite well, but we've ran into an issue with transactions lately.

Because of what we do, we bulk-load some data into Redis at startup + schedule full loads at certain times. We are not talking about huge amounts of data (a few hundred thousand records per data type, about 800k records in total at the moment) so we use the Lettuce API for this; we do not use the wire protocol directly because it's much more convenient to use Lettuce for us.

Doing all this in a MULTI/SET/SET/SET/.../EXEC sequence is much more efficient than running non-transactional. We are talking about 15 seconds vs about 2 minutes for a full load.

But, there is a significant risk with the MULTI-approach: if a connection starts a MULTI sequence, and other threads try to run non-transactional Redis commands in the middle, this will obviously not work (which is mentioned in the Lettuce documentation: https://lettuce.io/core/release/reference/#asynchronous-api.impact-of-asynchronicity-to-the-synchronous-api). We discovered this the hard way :) and added connection pooling support, to mitigate this.

Things are working better now, but there is still a potential issue which is why I am writing this: if something goes wrong in the middle of the transaction, there is a risk that the connection is handed back to the connection pool while still in a transactional state. We have code like this to workaround it (slightly edited for posting):

private void transactionWrapped( Consumer<RedisCommands<String, byte[]>> consumer ) throws SomeException {

redisClientManager.withRedisConnection( redisCommands -> {

try {

redisCommands.multi();

consumer.accept( redisCommands );

redisCommands.exec();

}

catch ( RedisException exception ) {

try {

redisCommands.discard();

}

catch ( RedisException innerException ) {

logger.warn( "Error attempting to discard Redis transaction", innerException );

}

throw new SomeException( exception );

}

} );

}

This works fine for most normal scenarios, but if the command timeout is set very low (5ms in our tests) there is a risk that the following happens:

• redisCommands.multi() succeeds
• Some other Redis calls are performed
• The code tries to call redisCommands.exec() which times out
• The code tries to call redisCommands.discard() which also times out

What happens then is that the connection is left in a transactional state and handed back to the connection pool. This is of course not so great, because it means that other (non-transactional) code might borrow this connection from the pool and get completely broken scenarios: SET commands are succeeding without `OK`, but in reality, the data is just left hanging in the stale Redis transaction.

So my two questions are:

1.  Are there any good ways to work around this?
2.  Can you detect an ongoing transaction in RedisCommands somehow? I am thinking: my withRedisConnection method above could detect that the connection coming back is in an invalid state and either discard the transaction itself or even close the connection forcibly.

Other suggestions are highly welcome. Thanks for your support and thanks a lot for writing a great Redis Java driver.

Best regards,

Per

[mpa...@pivotal.io]

Hi Per, 

Thanks for using Lettuce.

There are multiple aspects here:

1. Have you tried using a dedicated connection (instead of a shared one) for initial bulk loading? Doing so has the effect that you could leave the shared connection as-is, without the need to introduce pooling and without the danger of leaving a connection in a transactional state.

2. There's no way to "force discard" a transaction except for closing the connection. Transactional state is held within the Redis server and within the connection object on the client-side and both states must be in sync. There's no way to know whether Redis has received or even processed a DISCARD/EXEC command when running into a timeout.

3. StatefulRedisConnection has a method isMulti() to check whether the connection is known to be in a transactional state.

Let me know if that helps.

Cheers, 

Mark

[Per Lundberg]

Hi Mark,

Thanks for a fast reply. We discussed something similar to your option 1 before we decided to implement pooling, but decided on the pooling approach since we thought it would be simpler from the calling side. We were thinking of caching the connections by name, and using pooling felt like more risk-free in terms of inadvertently re-using a transactional connection in a non-transactional context. Our code base is quite large (a few hundred thousand lines of Java code) so not all developers can be expected to have the full picture in their head all the time. It's easy to forget details like this in 6 months, 1 year, 3 years... :-) So, we try to make things as fool-proof as we reasonably can.

Having that said, your points are fully valid and we will discuss this internally again and see what we will do. Your point #3 is good to know of as well, since it means we could probably detect incorrect usage of a "shared" connection as well (if we introduce the concept of shared/non-shared connections in our Lettuce connection wrapper). It seems like the safest way then (based on point #2) is to just close the connection and reopen it?

Thanks a lot for your help. Wishing you a nice weekend.

Best regards,

Per