MGET issues: wrong number of returned values; and mismatched values

[Rafi Shamim]

Hi!

I'm on Lettuce 4.1.2.Final, and I'm using Redis 2.8.24 on AWS Elasticache.

I have noticed two pretty weird behaviors relating to the MGET command in the Lettuce client. I don't know if it's something specific to MGET, or something else I've set up incorrectly with Lettuce in my application.

1)  In one case, the number of values returned from mget was less than the number of keys that I called it with. I noticed this because my code iterates over the values and got an IndexOutOfBoundsException (see the for loop in the code below).

2)  In many other cases, the wrong value was returned for a few of the keys that are given as the argument to mget. I noticed this because the value of each object has a field for the object's ID, and this ID is used as the key. My application detected that some of the values returned had a mismatched ID for the key it was requested with, and logged errors. Also, when I noticed these errors, I used the redis-cli to retrieve those keys, and I got back the correct value.

The way I'm using Lettuce is as follows.

I have a singleton class that is defined like below. It is used concurrently by multiple threads.

    private RedisAsyncCommands<ByteBuffer, Value> redis;
    private final RedisClient client;

    public void startUp() {
        final StatefulRedisConnection<ByteBuffer, Value> redisConnection = client.connect(ValueCodec.INSTANCE);
        redis = redisConnection.async();
    }

    public Map<ByteBuffer, Value> getValues(List<ByteBuffer> keys) {
        final RedisFuture<List<Value>> mget = redis.mget(keys.toArray(new ByteBuffer[keys.size()]));
        final List<Value> values;

        try {
            values = LettuceFutures.awaitOrCancel(mget, 500, TimeUnit.MILLISECONDS);
        } catch (RedisException e) {
            return ImmutableMap.of();
        }

        Map<ByteBuffer, Value> keyToValue = new HashMap<>(keys.size());
        for (int i = 0; i < keys.size(); i++) {
            final Value value = values.get(i);
            final ByteBuffer key = keys.get(i);
            if (value != null) {
                keyToValue.put(key, value);
            }
        }

        return keyToValue;
    }

I suspect that some of the weird behavior could be from the fact that I'm using ByteBuffers as keys, and how my ValueCodec is implemented. (In case you're curious, the ByteBuffers later on are deserialized into UUIDs.) Specifically, the de/encode key methods are defined as follows.

    @Override
    public ByteBuffer decodeKey(ByteBuffer key) {
        return key;
    }

    @Override
    public ByteBuffer encodeKey(ByteBuffer key) {
        return key;
    }

Does this seem problematic? My thought was that maybe the ByteBuffer contents should be copied instead of being returned by reference, since things could go bad if the buffers get modified elsewhere.

Unfortunately, the errors I'm describing are quite rare (a couple times over the past few weeks), and I haven't found a way to reproduce them. So I don't feel comfortable just implementing my fix, since I won't have confidence that I really fixed the issue.

So I wanted to check here if someone could confirm that fix idea and perhaps increase my confidence that it would fix the issue I'm seeing. Or maybe point me at a way to reproduce.

Also, this seems like it could explain issue (2), but issue (1) does not appear to be related to this as far as I can tell.

Hoping someone can help shed additional light here!

Thank you

Rafi

[Mark Paluch]

Hi Rafi, 

MGET should report the same number of keys as queried. If it's not doing so, then please file a bug report on Github along a reproducible test case.

Regarding the codec and shared mutable data structures (such as ByteBuffer) it's important to be aware what's happening under the hood. Writing buffers (encode methods) is more or less safe for single writes (commands that complete successfully in the first attempt). Keep in mind that reading a buffer moves the read index until it reaches its limit.

Lettuce retries commands if the connection gets disconnected. In that case, the command might have been encoded once and sent to the server. For the retry, the command is going to be encoded again. With your code from above, the buffer is already consumed from the first encoding pass and no key is written. It's safer to call ByteBuffer.duplicate() to prevent this effect.

On the reading side you need to know that Lettuce uses pooled byte buffers. Once the command is decoded, the buffers are reused to read the next command. This can interfere if you read multiple key commands. Rather create a new, decoupled buffer in decode methods.

Cheers, 

Mark

[Balaji Cherukuri]

Rafi, Mark

Did you find any solutions to this issue, I am facing a similar(possibly same) problem.

Thank you
Balaji