when in backoff, make sure that the chosen connection isn't closing #127
Conversation
|
It seems like a case that the fix for #106 did not cover. |
|
It may be better (and avoid a potential race) to have updateRdy return an error if c.IsClosing() and have backoff handle the error appropriately. |
|
Was going to suggest the same, want to give that a try? Thanks for finding this! |
|
I made the switch to checking the returned error. On consumer.go line 694 clears backoffDuration perhaps momentarily. It may be better to move it to the end of backoff(), after backoff is successful? This would avoid mistakenly sending false to a call to inBackoffBlock() https://github.com/jnewmano/go-nsq/blob/master/consumer.go#L694 |
| @@ -709,11 +709,29 @@ func (r *Consumer) backoff() { | |||
| idx := r.rng.Intn(len(conns)) | |||
| choice := conns[idx] | |||
|
|
|||
| // make sure the chosen connection isn't closing | |||
| if choice.IsClosing() { | |||
There was a problem hiding this comment.
I don't think we need this block anymore, do we?
SGTM |
Check updateRDY return error, clear backoffDuration on success in backoff, check error status of updateRDY
|
LGTM |
when in backoff, make sure that the chosen connection isn't closing
Backoff chooses a random connection and tries to update the ready status; however, if the connection is closing, updateRdy will return immediately, leaving the backoffCounter in a bad state. The side effect is that maybeUpdateRdy will never update the ready count due to the incorrect backoffCounter and the consumer will never again consume messages.