Refine RedisException instantiation to avoid exception instances if they are not used

[mindas]

Bug Report

Current Behavior

We are using NewRelic to monitor our AWS Redis cluster which uses Lettuce as the client. Upon upgrading NewRelic client to the newest, it has suddenly started to warn us about high volume of RedisExceptions. All of these exceptions did not come from our code but instead from a separate thread (I assume this thread is launched/by Lettuce). I've tried to reproduce it locally and ended up with the following code:

Input Code

public class Redis {
    private static final String CHANNEL = "c1";

    public static void main(String[] args) throws IOException, ExecutionException, InterruptedException {
        Random r = new Random(System.nanoTime());
        RedisURI redisUri = RedisURI.Builder.redis("localhost").withPort(7000).build();
        RedisClusterClient redisCluster = RedisClusterClient.create(redisUri);
        CountDownLatch l1 = new CountDownLatch(1);

        Runnable r1 = () -> {
            StatefulRedisClusterPubSubConnection<String, String> connection = redisCluster.connectPubSub();
            connection.addListener(new RedisClusterPubSubAdapter<>() {
                @Override
                public void message(RedisClusterNode node, String pattern, String channel, String message) {
                    this.message(node, channel, message);
                }

                @Override
                public void message(RedisClusterNode node, String channel, String message) {
                    System.out.println("Received: " + channel + " " + message);
                }
            });
            connection.async().subscribe(CHANNEL);
            System.out.println("Subscribed to channel: " + CHANNEL);
            l1.countDown();
        };
        Thread pubSubThread = new Thread(r1);
        pubSubThread.setDaemon(true);
        pubSubThread.start();
        l1.await();  // block until r1 finishes subscribing

        CountDownLatch l2 = new CountDownLatch(1);
        Runnable r2 = () -> {
            for (int i=0; i<10; i++) {
                try (StatefulRedisClusterConnection<String, String> connection = redisCluster.connect()) {
                    connection.sync().publish(CHANNEL, "blah" + Math.abs(r.nextInt()));
                    LockSupport.parkNanos(1000);
                }
            }
            l2.countDown();
        };
        Thread t2 = new Thread(r2);
        t2.setDaemon(true);
        t2.start();
        l2.await();  // block until r2 finishes sending
    }
}

The code above uses local Redis cluster (docker run -e "IP=0.0.0.0" -p 7000-7005:7000-7005 grokzen/redis-cluster:latest).
I ran this code with IntelliJ and asked it to log a line every time RedisException is initialized. The output is as follows:

Connected to the target VM, address: '127.0.0.1:36377', transport: 'socket'
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Subscribed to channel: c1
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Received: c1 blah735923104
Breakpoint reached at Redis$1.message(Redis.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Received: c1 blah1177016861
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah1510619890
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah1238218257
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah2118425883
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah668476499
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah618495494
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah232634554
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah855934428
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at Redis$1.message(Redis.java:33)
Received: c1 blah1261709274
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Breakpoint reached at io.lettuce.core.RedisException.<init>(RedisException.java:33)
Disconnected from the target VM, address: '127.0.0.1:36377', transport: 'socket'

Process finished with exit code 0

Expected behavior/code

Above suggests RedisException was constructed (not necessarily thrown!) 41 times.
The exception message is always the same, "Connection closed".
Can you comment whether such liberal usage of exceptions is by design or is this a bug?

I guess we could block this exception in NewRelic but this would likely shield us from spotting actual Redis errors.

Environment

• Java version: 11.0.6
• Lettuce version: lettuce-core-6.2.3.RELEASE.jar
• Redis version: 6.2.0

Possible Solution

Avoid excessive usage of exceptions for they are expensive to create (Effective Java et al).

[mp911de]

Can you comment whether such liberal usage of exceptions is by design or is this a bug?

Our purpose is to exhaust client memory as much as we can just to annoy the hell out of people.

More seriously, the exception you're referring to is being instantiated in our notifyChannelInactive handling to cancel lingering commands. It is created eagerly to avoid recreating the same exception multiple times. The downside is that exceptions with stack traces are allocated even if exceptions are not thrown.

Refining our approach and creating these exceptions lazily for improved throughput makes sense.

[mp911de]

That's in place now.