mediocregopher / radix.v2

Redis client for Go
http://godoc.org/github.com/mediocregopher/radix.v2
MIT License
433 stars 92 forks source link

use pipe from pool cause "bytes.Buffer: truncation out of range" #27

Closed wangxingge closed 8 years ago

wangxingge commented 8 years ago

I have a pool connect to a redis instance. I create this pool as following: _pool, err := pool.New("tcp", address, 100)

And there are 300 goroutine use this pool to send command. most of them using pool as following,(we call it: eg1) _pool.Cmd(cmd, args)

And 50 goroutine user this pipe from this pool, like following(we canll it: eg2) c, err :=_pool.Get() c.PipeAppend(cmd, args) c.PipeAppend(cmd, args) c.PipeAppend(cmd, args) c.PipeResp()

Problem is: eg1 will throw a panic error: bytes.Buffer: truncation out of range

CallStack like following.

goroutine 83 [running]: bytes.(_Buffer).Truncate(0xc8205e3500, 0x0) /usr/local/go/src/bytes/buffer.go:69 +0xbf bytes.(_Buffer).WriteTo(0xc8205e3500, 0x7fe8513a6868, 0xc82029c050, 0x33, 0x0, 0x0) /usr/local/go/src/bytes/buffer.go:222 +0x1af github.com/mediocregopher/radix.v2/redis.(_Client).writeRequest(0xc8212a6180, 0xc8500a2ea8, 0x1, 0x1, 0x0, 0x0) /home/stanford/golang/src/github.com/mediocregopher/radix.v2/redis/client.go:172 +0x5e3 github.com/mediocregopher/radix.v2/redis.(_Client).Cmd(0xc8212a6180, 0x9116b8, 0x4, 0xc853140320, 0x1, 0x1, 0x7c6780) /home/stanford/golang/src/github.com/mediocregopher/radix.v2/redis/client.go:85 +0x12b github.com/mediocregopher/radix.v2/pool.(*Pool).Cmd(0xc8212c24e0, 0x9116b8, 0x4, 0xc853140320, 0x1, 0x1, 0x0) /home/stanford/golang/src/github.com/mediocregopher/radix.v2/pool/pool.go:95 +0x104

BTW, I'm not using the lasted version of radix pool.

I'll try last version later.

wangxingge commented 8 years ago

But it's worked when I create a new connection to use pipe. You know it's low efficiency.

mediocregopher commented 8 years ago

Hey there! I'm afraid I'm having a bit of trouble understanding what you're doing. My two suggestions just based on your sample code are to make sure you're calling Put on the Pool once you're done with your Pipe* commands, and to make sure you call PipeResp the same number of times you call PipeAppend. Although neither of those should really cause a panic.

Can you explain what you mean by using a pipe to create a new connection?

On Fri, May 27, 2016 at 12:13 AM wangxingge notifications@github.com wrote:

But it's worked, when I use pipe then to create a new connection. You know it's low efficiency.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mediocregopher/radix.v2/issues/27#issuecomment-222068318, or mute the thread https://github.com/notifications/unsubscribe/AA49IlXIywmQlIRGkfoQZH4XoLnv2YAUks5qFouUgaJpZM4IoM51 .

wangxingge commented 8 years ago

Sorry for reply so late.

I'v review your method of "PipeResp", its send all pending command to redis, by invoke "ReadResp" in loop. So I append many time and just call PipeResp once, then i found if i used the connection from pool, panic will be thrown.

So I create a new connection, and this new connection not get from pool. Create code following: con := redis.DialTimeout("tcp", _workAbleAdd, 10*time.Second)

con.PipeAppend con.PipeAppend con.PipeAppend con.PipeResp

BTW, I'm very appreciate your quick response/

mediocregopher commented 8 years ago

It's true you don't have to do a PipeResp all 3 times, and all the responses will have been read from the connection. But the Client is buffering those answers internally, so if you're going to use that connection again later (which you will, since you're using a pool) you don't want those staying buffered because they'll cause problems later. You should at least call PipeClear to get rid of the buffered answers you don't want.

On Sat, May 28, 2016 at 11:41 PM wangxingge notifications@github.com wrote:

Sorry for reply so late.

I'v review your method of "PipeResp", its send all pending command to redis, and invoke "ReadResp". So I append many time and just call PipeResp once.

Create a new connection means. con := redis.DialTimeout("tcp", _workAbleAdd, 10*time.Second)

con.PipeAppend con.PipeAppend con.PipeAppend con.PipeResp

BTW, I'm very appreciate your quick response/

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/mediocregopher/radix.v2/issues/27#issuecomment-222343998, or mute the thread https://github.com/notifications/unsubscribe/AA49IgKUgKzQChS1caeaAX8e1yA7hkItks5qGScFgaJpZM4IoM51 .

wangxingge commented 8 years ago

Thank you, I got it, it review "PipeClear" function, I'll update to lasted version of radix driver.

Thanks a lot.

mediocregopher commented 8 years ago

No problem, let me know if you have any other questions :)

On Sun, May 29, 2016 at 7:09 PM wangxingge notifications@github.com wrote:

Closed #27 https://github.com/mediocregopher/radix.v2/issues/27.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/mediocregopher/radix.v2/issues/27#event-675557265, or mute the thread https://github.com/notifications/unsubscribe/AA49IkSABZHyHSUEFYN-qtcmgTuQg3QRks5qGjjNgaJpZM4IoM51 .

wangxingge commented 8 years ago

Works now. :+1: