consumer: Reset backoff counter when no connections left. #106
Conversation
| if r.backoffCounter > 0 { | ||
| r.backoffCounter-- | ||
| } | ||
| r.backoffMtx.Unlock() |
There was a problem hiding this comment.
Thanks for identifying this bug.
Unfortunately, I'm not convinced this is the right fix. I think the correct behavior should be for this to loop until it has a connection (also paying attention to an exit signal), and then send RDY 1.
Thoughts @jehiah
There was a problem hiding this comment.
If we treat backoff as backing off from downstream systems, then i agree. It should persist beyond upstream nsqd reconnects
Since this code block is in a time.After we should be able to refactor to just re-set that function recursively (and have it end on exit flags appropriately).
There was a problem hiding this comment.
I should have some time to work on a fix tomorrow (GMT).
|
this LGTM, mind squashing the commits down? |
This fixes the following scenario: 1. Consumer is connected to one NSQD host using ConnectToNSQD. 2. Consumer receives message and handler returns an error. 3. Consumer backs off. 4. DisconnectFromNSQD called before backoff closure wakes up. 5. Backoff closure wakes up, sees no connections, returns. 6. Consumer connects to NSQD host using ConnectToNSQD. 7. backoffCounter still > 0 so never sends RDY count (in maybeUpdateRDY). 8. Consumer is now stuck and never sends RDY > 0.
|
Squashed :) |
consumer: Reset backoff counter when no connections left.
|
thanks! |
This fixes the following scenario:
maybeUpdateRDY).