nsqd: fixing race with sending messages and topic/channel associations changing #828
Conversation
| case msg = <-memoryMsgChan: | ||
| case buf = <-backendChan: | ||
| msg, err = decodeMessage(buf) | ||
| if err != nil { | ||
| t.ctx.nsqd.logf("ERROR: failed to decode message - %s", err) | ||
| continue | ||
| } | ||
| case <-t.channelUpdateChan: |
There was a problem hiding this comment.
I think the crux of the problem was actually that the select statement here could possibly never choose this path, or more likely, choose one of the message chan paths before choosing this one.
| @@ -263,34 +269,49 @@ func (t *Topic) messagePump() { | |||
| goto exit | |||
| } | |||
|
|
|||
| for i, channel := range chans { | |||
There was a problem hiding this comment.
and then this loop happens while some other process could be trying to change the set of channels. I'm not sure that's a problem, but putting this under the topic's lock will help make channel operations more atomic.
|
@stephensearles thanks for the PR and the investigation. You're correct that it's theoretically possible for channel creation to be handled out-of-order from message routing (in the Go runtime we trust), but I don't think it's a practical issue worth solving and certainly not at the expense of larger critical sections between locks. More specifically, the edge cases where this would occur imply that other conditions exist that could cause messages to be "lost" to a new channel (e.g. publishing to the topic while creating said channel). I'm 👎 |
fixes #826