Extend EXECABORT with "previous errors"

[nerg4l]

In a MULTI/EXEC environment, Jedis might receive the following response when an issue is detected:

JedisDataException: EXECABORT Transaction discarded because of previous errors.

In such cases, no information is included in the exception and because of the exception, the redis.clients.jedis.Response value is never set and throws an exception when the user tries to read it.

IllegalStateException: Please close pipeline or multi block before calling this method.

Which is a bit misleading because the multi block was closed.

InpuStream example:

+OK
+QUEUED
-CROSSSLOT Keys in request don't hash to the same slot (context='within MULTI', command='get', original-slot='9842', wrong-slot='5649', first-key='1', violating-key='2')
-CROSSSLOT Keys in request don't hash to the same slot (context='within MULTI', command='get', original-slot='9842', wrong-slot='1584', first-key='1', violating-key='3')

Is it possible to include these in the exception or to populate the Response objects and let the user find the relevant errors?

[uglide]

Hi @nerg4l
Thanks for the report. Let me create some minimal reproducible example with CROSSSLOT violation to understand the limitations we currently have and I'll get back to you.

[nerg4l]

@uglide

Not sure if this will help. You will still need to set up a clustered Redis instance. (I used Redis Enterprise in my testing.)

package org.example;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import redis.clients.jedis.*;

import java.lang.invoke.MethodHandles;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;

public class JedisCrossslotApp {

    private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

    private final URI uri;

    public JedisCrossslotApp(URI uri) {
        this.uri = uri;
    }

    public static void main(String[] args) {
        URI addr = URI.create("redis://127.0.0.1:12000");
        JedisCrossslotApp app = new JedisCrossslotApp(addr);
        try {
            app.run();
        } catch (Throwable e) {
            logger.warn("", e);
        }
    }

    public void run() {
        {
            JedisPoolConfig poolConfig = new JedisPoolConfig();

            JedisPool pool = new JedisPool(
                    poolConfig,
                    uri.getHost(),
                    uri.getPort(),
                    1000
            );

            List<Response<String>> responses = new ArrayList<>();
            try (Jedis jedis = pool.getResource()) {
                try (Transaction tx = jedis.multi()) {
                    responses.add(tx.get("1"));
                    responses.add(tx.get("2"));
                    responses.add(tx.get("3"));
                    tx.exec();
                }
            } catch (Exception e) {
                logger.error("", e);
                logger.error("", e.getCause());
                for (Throwable suppressed : e.getSuppressed()) {
                    logger.error("", suppressed);
                }
            } finally {
                responses.forEach(r -> {
                    logger.info("{}", r.get());
                });
            }
        }
    }
}

[ggivo]

@nerg4l
Thanks for the code example. I was able to reproduce the behavior described.
The current implementation of Transaction disregards QUEUED/ERROR states of individually sent commands and propagates the exception raised when an actual EXEC call is performed. This more or less is in sync with the redis Errors inside a transaction
I think exposing errors accumulated before EXEC and reporting them as part of suppressed exceptions of JedisDataException returned on actual EXEC command invocation make sense and can improve the troubleshooting in case of errors.

Another option would be to propagate the error through the returned command Response's, but this is kind of breaking the existing contract and would not go in that direction.

If you don't need a pipelining, you can also look at redis.clients.jedis.ReliableTransaction, which will send each command immediately and propagate the QUEUED/ERROR state.

[nerg4l]

@ggivo I think adding it as suppressed errors is good enough, and thanks for bringing my attention to ReliableTransaction.