Improve command encoding of singular commands and command batches

[cwolfinger]

Hello with certain sized arguments the code always produces a BufferOverflow exception and triggers a buffer resize. The following program illustrates this behavior

    RedisClient client = RedisClient.create(RedisURI.create("localhost", 6379));
    for (int i = 0; i < 2048; i ++)
    {
        client.connect(ByteArrayCodec.INSTANCE).async().rpush("abc".getBytes(), new byte[i]);
    }

When i hits 17 the overflow occurs.

It seems to me there is a bug in the first line of comparison:

if (buffer.remaining() < arg.remaining()) {
int estimate = buffer.remaining() + arg.remaining() + 10;
realloc(max(buffer.capacity() * 2, estimate));
}

    while (true) {
        try {
            ByteBuffer toWrite = arg.duplicate();
            buffer.put((byte) '$');
            write(toWrite.remaining());
            buffer.put(CRLF);
            buffer.put(toWrite);
            buffer.put(CRLF);
            break;
        } catch (BufferOverflowException e) {
            buffer.reset();
            realloc(buffer.capacity() * 2);
        }
    }

It seems that it should be something like this:

if (buffer.remaining() < arg.remaining()+10)

I am not sure all of the combinations this is hit quite a bit ...

[cwolfinger]

In looking at the code - i think the write block needs to change to:

    private CommandArgs<K, V> write(ByteBuffer arg) {
        buffer.mark();

        if (buffer.remaining() < arg.remaining() + 10) {
            int estimate = buffer.capacity() + arg.remaining() + 10;
            realloc(max(buffer.capacity() * 2, estimate));
        }

This eliminates the exception. It would also be useful to have an option to set the start value for the buffer since in general the calling program has a pretty good idea of the basic size of the command plus value.

[mp911de]

Hi @cwolfinger,
thanks for your report. I actually started a couple days ago looking into CommandArgs because the real issue is that itself allocates memory. That's quite costly. When writing a command, the command encoder needs to allocate memory anyways and it's cheaper to allocate memory once.

Btw. there's an issue in your code since you create 2048 connections and you don't close them.

[cwolfinger]

Great - just tested and this seems to get rid of the exceptions. The test code was quick and dirty so not unexpected it leaked connections.

[mp911de]

Refactored command encoding, command args and command decoding.