matryer / vice

Go channels at horizontal scale (powered by message queues)
https://medium.com/@matryer/introducing-vice-go-channels-across-many-machines-bcac1147d7e2
Apache License 2.0
1.54k stars 79 forks source link

nsq: empty buffer on Stop #23

Closed matryer closed 7 years ago

matryer commented 7 years ago

like https://github.com/matryer/vice/pull/20 we need to empty the channel before actually stopping.

HeavyHorst commented 7 years ago

I noticed that the nsq implementation doesn't use a buffered channel.

I tested the nsq performance (like here: #12) with and without the buffered channel and the performance is nearly the same:

Without buffer

go test -v .
=== RUN   TestTransport
=== RUN   TestTransport/testStandardTransportBehaviour
=== RUN   TestTransport/testSendChannelsDontBlock
--- PASS: TestTransport (1.77s)
    --- PASS: TestTransport/testStandardTransportBehaviour (1.76s)
    --- PASS: TestTransport/testSendChannelsDontBlock (0.00s)
=== RUN   TestSend
2017/08/03 22:40:48 @@ NSQTestConsumer Consuming  hello vice
2017/08/03 22:40:48 stopping NSQTestConsumer
2017/08/03 22:40:48 NSQTestConsumer stopped
--- PASS: TestSend (0.11s)
=== RUN   TestReceive
2017/08/03 22:40:48 NSQTestProducer: send hello vice
2017/08/03 22:40:48 INF   15 (localhost:4150) connecting to nsqd
2017/08/03 22:40:48 NSQTestProducer:  ^ published
--- PASS: TestReceive (0.11s)
PASS
ok      github.com/matryer/vice/queues/nsq      1.983s

With buffer

go test -v .
=== RUN   TestTransport
=== RUN   TestTransport/testStandardTransportBehaviour
=== RUN   TestTransport/testSendChannelsDontBlock
--- PASS: TestTransport (1.73s)
    --- PASS: TestTransport/testStandardTransportBehaviour (1.73s)
    --- PASS: TestTransport/testSendChannelsDontBlock (0.00s)
=== RUN   TestSend
2017/08/03 22:39:38 @@ NSQTestConsumer Consuming  hello vice
2017/08/03 22:39:38 stopping NSQTestConsumer
2017/08/03 22:39:38 NSQTestConsumer stopped
--- PASS: TestSend (0.11s)
=== RUN   TestReceive
2017/08/03 22:39:38 NSQTestProducer: send hello vice
2017/08/03 22:39:38 INF   15 (localhost:4150) connecting to nsqd
2017/08/03 22:39:38 NSQTestProducer:  ^ published
--- PASS: TestReceive (0.11s)
PASS
ok      github.com/matryer/vice/queues/nsq      1.949s

I think we can close this issue and leave the nsq transport as is to keep the code simple.