Ensure atomic transactions after reconnect

[gszpak]

I am using version 5.0.1.RELEASE.

I would like to ask about the proper way of executing transactions using asynchronous commands.
Let's take a look at the example from the wiki (modified to be consistent with 5.0.1 version):

RedisAsyncCommands<String, String> async = client.connect().async();
RedisFuture<String> multi = async.multi();
RedisFuture<String> set = async.set("key", "value");
RedisFuture<TransactionResult> exec = async.exec();

My question is: why is the result of MULTI ignored? Shouldn't the rest of the commands and the EXEC be sent after we know, that MULTI command was executed successfully?

On the other hand, if we do it like this:

RedisAsyncCommands<String, String> async = client.connect().async();
RedisFuture<TransactionResult> transactionResultFuture = async
    .multi()
    .thenCompose(multiResult -> {
        if (!multiResult.eqauls("OK")) {
            throw new IllegalStateException("Unexpected MULTI result");
        }
        RedisFuture<String> set = async.set("key", "value");
        return async.exec();
    })

we might cause a race condition (before MULTI is completed, other commands might be sent and executed inside transaction).

Thank you!

[mp911de]

Using the asynchronous API allows you to optimize for various aspects of the actual execution. The example optimizes for throughput (pipelining) without awaiting command completion.

MULTI cannot fail on Redis with a healthy connection. If it would fail, other commands would fail, too. Redis is single-threaded, therefore it processes commands as they arrive. All I/O of a Lettuce Redis connection is handled by a single thread too. The code above runs sequentially so MULTI happens before SET and so on hence the commands are sent and processed sequentially.

In your code thenCompose( multiResult -> if (!multiResult.eqauls("OK")) { can never happen. Execution errors are signalled via RedisCommandExecutionException, you'd rather need to use .handle((multiResult, exception) -> .

[gszpak]

Thank you for your detailed answer!

I'd like to dig a more deeper into this case.

First, let me be more specific about what I meant by a race condition in the second example. Let's consider this simple example:

public class Example {

    private static String KEY = "key";

    private static CompletionStage<TransactionResult> executeTransaction(
            RedisAsyncCommands<String, String> commands, String value) {
        return commands
                .multi()
                .thenCompose(unused -> {
                    // This is executed iff multi succeeds
                    commands.set(KEY, value);
                    return commands.exec();
                });
    }

    public static void main(String[] args) {

        String host = args[0];
        int port = Integer.valueOf(args[1]);
        RedisURI redisURI = RedisURI.Builder.redis(host, port).build();
        RedisClient client = RedisClient.create(redisURI);
        StatefulRedisConnection<String, String> connection = client.connect();
        RedisAsyncCommands<String, String> commands = connection.async();

        CompletionStage<TransactionResult> stage1 = executeTransaction(commands, "value1");
        RedisFuture<String> stage2 = commands.set(KEY, "value2");

        stage1.toCompletableFuture().join();
        stage2.toCompletableFuture().join();

        System.out.println(commands.get(KEY).toCompletableFuture().join());
        connection.close();
        client.shutdown();
    }

}

In this case the final value can be either value1 or value2. So, executing a transaction this way is usually not an option and it has to be executed in the way described in the wiki.

Therefore, can I always assume that failure of MULTI will cause failure of other commands in the transaction? Could you please clarify, what would exactly happen in following two cases:

1. Some temporary network problem occurs
2. (looking a bit into the future) MULTI command gets a timeout. I've noticed you are going to add CommandExpiryWriter in version 5.1, that will cancel a future after a specified period of time. What will happen, if MULTI gets a timeout and is cancelled, but the other commands are not?

Thank you!

[mp911de]

The code above runs into a concurrency race anyway because you're continuing the actual transaction with a context switch. The function passed to thenCompose(…) gets executed on a different thread.

To your questions:

1. Lettuce auto-reconnects by default. Commands get queued in a disconnected state. If the connection gets disconnected, then it's important to understand where it happened and what the consequences are:
   • Disconnected before/while calling MULTI: thenCompose(…) isn't called until the connection reconnects. Reconnect will replay all queued commands and once MULTI finishes execution, it will call thenCompose(…) and resume transactional work.
   • Disconnected after MULTI was called: Queued commands (queued after disconnect) are replayed without a transactional context, EXEC will fail, commands within MULTI until the disconnect are lost.
2. The asynchronous API does not have timeouts as in a synchronous API. Commands run forever (if not completed or canceled). Calling RedisFuture.cancel(…) will mark commands as canceled and not replay these upon reconnect. CommandExpiryWriter builds on cancel to protect asynchronous compositions (CompletableFuture.then…(…), Reactive API) from infinite waiting for completion. Cancelling completes the a with CancellationException).

Cancelling a command has no effect on the actual execution within Redis. Once a command is sent, it's going to be executed. Sending writes the command to the transport and there's no way to reverse the write. The connection is kind of blocked until execution completes. The only way for relief on the client side is to disconnect the connection and start over.

[gszpak]

Thank you for your answer.

Again, let me clarify a couple of things:

• My example in the previous comment was only to show that executing a transaction with thenCompose(...) is not possible as it causes race conditions. By the way, the race condition would happen even if there was only one thread (so if the main thread also took care of Lettuce's Netty connections).
• So, now I assume that the only proper way of executing a transaction is the one from wiki:

RedisFuture<String> multi = async.multi();
RedisFuture<String> set = async.set("key", "value");
...
RedisFuture<TransactionResult> exec = async.exec();

Can you please clarify again what would happen here if the call async.multi() was never actually sent to Redis (e.g. because of network problems)? I am afraid that other commands would be sent (not as transaction, as MULTI never reached Redis), and sending EXEC would cause an error. Is it possible?

[mp911de]

Can you please clarify again what would happen here if the call async.multi() was never actually sent to Redis (e.g. because of network problems)? I am afraid that other commands would be sent (not as transaction, as MULTI never reached Redis), and sending EXEC would cause an error. Is it possible?

See above:

Disconnected after MULTI was called: Queued commands (queued after disconnect) are replayed without a transactional context, EXEC will fail, commands within MULTI until the disconnect are lost.

[gszpak]

That's what I was afraid of in the first place: isn't it a bug, that commands after reconnection are replayed without a transactional context?

[mp911de]

Yes, in the context of auto-reconnect it's a bug because the batch isn't atomic anymore. I adjusted the ticket accordingly.

[gszpak]

Thank you!

Are you sure this happens only after reconnect? What about such a case: MULTI gets SocketTimeout (network is slow for a short while) -> we don't check if MULTI's future completed exceptionally, so following commands are sent and executed without transactional context -> EXEC fails?

[laymain]

Hi, is there any update on this issue?

[mp911de]

Feel free to submit a pull request. This is a community-maintained driver so any contributions can accelerate when this gets fixed.

[tishun]

Since this issue was not very active the last year I am putting it in the Icebox.
If more people run into it or if there are people willing to contribute a solution we can pull it out.