ReceiveMessage buffer full

[xtuc]

We are streaming some big JSON objects through the pub/sub mechanism of Redis. Sometimes we hit the internal limit of the ReadLine buffer and the Redis client returns a bufio: buffer full error instead of something.

Note that the default size of the buffer is at 4096 bytes.

It would be great to either handle the isPrefix returned by ReadLine or allow us to configure the buffer passed to ReadLine at initialization.

See

https://github.com/go-redis/redis/blob/74d5147d860804ea34633071d0c91a7f4b0b5c3a/internal/proto/reader.go#L62-L64

If you agree with that, I'll be happy to make a PR since I've already the code to handle that somewhere.

[vmihailenco]

Can we please start with a test that reproduces the issue?

[xtuc]

#642

[xtuc]

Any news from this? Can I help you with something? Do we agree on #642 (comment) now?

[vmihailenco]

You can help by writing an app that uses ReceiveMessage and reproduces this issue.

[xtuc]

From my understanding, that's what I explained in #642 (comment).

PubSub.ReceiveMessage uses your ReadLine wrapper under the hood. I was asking if you agree with that. I just followed the function calls with "go to definition".

[netgusto]

Seems pretty clear from the golang documentation that isPrefix is not semantically an error, but a flag informing the consumer that there are more bytes to read on the line.

https://golang.org/pkg/bufio/#Reader.ReadLine

This could (should?) be handled in this code as proposed by @xtuc
https://github.com/go-redis/redis/blob/74d5147d860804ea34633071d0c91a7f4b0b5c3a/internal/proto/reader.go#L57-L72

[vmihailenco]

I still did not see a program that reproduces this issue.

[netgusto]

Damn, too bad.

[eelcocramer]

I get this issue when retrieving large blobs from redix which is API compliant with redis.

I created a test that reproduces that error. It happens when reading from a byte array which length is larger than 4096.

https://github.com/eelcocramer/redis/blob/46c6ac7debcb631c52554a9a990a1f7d6295a71f/internal/proto/reader_test.go#L30-L41

func TestReader_ReadLine(t *testing.T) {
	original := bytes.Repeat([]byte("a"), 8192)
	r := proto.NewReader(bytes.NewReader(original))
	read, err := r.ReadLine()
	if err != nil {
		t.Errorf("Should be able to read the full buffer: %v", err)
	}

	if bytes.Compare(read, original) != 0 {
		t.Errorf("Values must be equal: %q", read)
	}
}

A fix that works for me is:

https://github.com/eelcocramer/redis/blob/46c6ac7debcb631c52554a9a990a1f7d6295a71f/internal/proto/reader.go#L48-L60

func (r *Reader) ReadLine() ([]byte, error) {
	line, err := r.rd.ReadBytes('\n')
	if err != nil && err != io.EOF {
		return nil, err
	}
	if len(line) == 0 {
		return nil, fmt.Errorf("redis: reply is empty")
	}
	if isNilReply(line) {
		return nil, Nil
	}
	return line, nil
}

edit: added code snippets from the fork