Flaky test pingNotAllowedInSubscriptionState

[sancar]

Bug Report

Current Behavior

Test pingNotAllowedInSubscriptionState fails randomly in our tests against Upstash Redis. After investigating if our implementation of Redis has a problem, we have concluded that the problem in lettuce side.

Test failure happens in the following assertion

  assertThatThrownBy(() -> TestFutures.getOrTimeout(pubsub.echo("ping"))).isInstanceOf(RedisException.class)
                .hasMessageContaining("not allowed");

Cause Of the Problem

The test fails because of a race condition between two threads. More specifically: The future for subscribe is completed after we are updating the internal state of PubsubEndpoint(adding a new channel to the channels set). Because of that, in some unlucky timing, the test (PubSubCommandHandler#notifyPushListeners) fails.

1. subscribe future is completed.
2. new channel is not put into the channels map in PubsubEndpoint
3. Test continues to send an echo to the server.
4. Client side for PubSubEndpint#isSubscribed returns false because channels and patterns are empty.
5. Server runs the command and does not return an error because the client is talking with resp3 and all commands are allowed.
6. Even if we were talking in resp2 test would fail because of the error message of the exception.

Possible Solution

~Remove local checks for allowed commands and rely on the exception from redis.~ ~This way, we can both remove the bug and simplify the library.~ ~More details in pr that I have prepared for the proposed solution:~ ~https://github.com/lettuce-io/lettuce-core/pull/2314~ This solution is eliminated because the local checks are needed see https://github.com/lettuce-io/lettuce-core/issues/579