Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nsqd: Empty Queue should reset the InFlight count of consumers #160

Merged
merged 1 commit into from
Mar 19, 2013

Conversation

dmarkham
Copy link
Contributor

This will keep them insync after they try and fail to FIN messages
that were InFlight when the Queue was Emptied.

@dmarkham
Copy link
Contributor Author

This is a proposed fix for what I think is a issue with channel.clients staying in sync with a channel after Empty was called. If a channel is actively getting consumed and you empty the Queue the channel.clients can get hung with their In-Flight number above the ready count.

@mreiferson
Copy link
Member

Looks like you're onto something here. Will poke around today.

Thanks @dmarkham

@mreiferson
Copy link
Member

So the fix itself looks good, the only issue I've been mulling over is the name of the method on the client.

I'm leaning towards s/SetReadyCount/Empty (without a parameter) so that the knowledge of what to do when an Empty action occurs is kept on the side of the client code rather than the channel code. This way the code reads as if you're propagating this event to each client rather than updating the clients internal state directly.

Does that make sense? Thoughts?

@dmarkham
Copy link
Contributor Author

the SetInFlightCount was based on SetReadyCount I'm happy to rename it to EmptyFlightCount or ResetFlightCount with no args to look more like a notification to the Client.

@dmarkham
Copy link
Contributor Author

your thinking just Empty, Ok after rereading your post I understand and like it. I'll get the patch ready ASAP.

This will keep them insync after they try and fail to FIN messages
that were InFlight when the Queue was Emptied.
@mreiferson
Copy link
Member

cool, looks great, thanks for identifying and fixing this.

mreiferson added a commit that referenced this pull request Mar 19, 2013
nsqd: Empty Queue should reset the InFlight count of consumers
@mreiferson mreiferson merged commit dc0af0d into nsqio:master Mar 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants