Avoid out-of-sync state by handling java.lang.Error

[tishun]

Improvement Request

There ara multitude of issues reported where the driver would report a variation of the following message:

java.lang.UnsupportedOperationException: io.lettuce.core.output.ValueOutput does not support set(long)

As a hypothesis we believe that some error, such as - but not necessarily - OutOfMemory, causes the event loop to become out of sync.

See #3117 (comment)

Related issues:

• ValueOutput does not support set(long) #2175
• In high concurrency call stringRedisTemplate fails #3117
• java.lang.UnsupportedOperationException: io.lettuce.core.output.ValueOutput does not support set(long) #3121
• use with gateway，but something throw io.lettuce.core.output.ValueOutput does not support set(long) #2864
• Responses getting out of sync with requests #2012

Describe the solution you'd like

A suggested solution is to catch such errors, terminate the connection and complete any remaining commands exceptionally.

Describe alternatives you've considered

N/A

[henriklundstrom]

@tishun

As a hypothesis we believe that some error, such as - but not necessarily - OutOfMemory, causes the event loop to become out of sync.

I hope this can be helpful in some way:

The out-of-sync state can be reproduced in the existing test case subscriberCompletingWithExceptionShouldBeHandledSafely by substituting this RuntimeException with any kind of JVM fatal error, such as a LinkageError. It produces this failure:

[ERROR] Failures: 
[ERROR]   ReactiveConnectionIntegrationTests.subscriberCompletingWithExceptionShouldBeHandledSafely expectation "expectNext(valueB)" failed (expected value: valueB; actual value: valueA)

Alternatively, this more extensive test case can be used to illustrate the behaviour where GET(keyX) -> valueY:

@Test
@Inject
void showcaseInconsistentResponsesOnJvmFatal(RedisClient client) {

    // A separate reactive connection is created to avoid interference with other tests
    RedisReactiveCommands<String, String> reactiveCommands = client.connect().reactive();

    // Prepare a state in the Redis
    StepVerifier.create(Flux.concat(
                    reactiveCommands.set("keyA", "valueA"),
                    reactiveCommands.set("keyB", "valueB"),
                    reactiveCommands.set("keyC", "valueC"),
                    reactiveCommands.set("keyD", "valueD")
                    )
            ).expectNextCount(4)
            .verifyComplete();

    /**
     * Purposefully throw a JVM fatal error in the onNext callback.
     *
     * The error is not propagated to the subscriber.
     * Therefore, the StepVerifier can not be used, as it would hang indefinitely.
     *
     * The throwable needs to be a "jvmFatal" as defined by:
     * {@link reactor.core.Exceptions#isJvmFatal(Throwable)}
     */
    reactiveCommands.get("keyA").doOnNext(foo -> { throw new LinkageError("jvmFatal1"); }).subscribe();

    // Unlike a normal test case, these assertions are used to demonstrate the inconsistent behavior
    StepVerifier.create(reactiveCommands.get("keyB")).expectError(LinkageError.class).verify();
    StepVerifier.create(reactiveCommands.get("keyC")).expectNext("valueB").verifyComplete();
    StepVerifier.create(reactiveCommands.get("keyD")).expectNext("valueC").verifyComplete();
}

[tishun]

Hey @henriklundstrom ,

thanks for the sample code! As soon as the team has prioritized this item we will go over it and try to provide a fix.

[tishun]

See also https://github.com/nordnet/lettuce-out-of-order