nsq_to_file: with a large max-in-flight some messages unprocessed during a SIGTERM#1521
Draft
danbf wants to merge 1 commit into
Draft
nsq_to_file: with a large max-in-flight some messages unprocessed during a SIGTERM#1521danbf wants to merge 1 commit into
danbf wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause: Go's select chooses randomly when multiple cases are ready simultaneously. At shutdown with 1000 max-in-flight:
The last handler goroutine sends its message to logChan (buffer=1, so it succeeds immediately and the goroutine returns)
go-nsq sees all handlers have returned → closes StopChan
Now both StopChan and logChan are ready at the same time
If select picks StopChan, the message in logChan is abandoned — never written to disk, never Finish()'d
The higher the max-in-flight, the more often this happens because there's more concurrency in the final draining.
Fix: The StopChan case now runs a non-blocking drain loop over logChan before setting sync = true. Since StopChan only closes after all handler goroutines return (and each goroutine sends exactly one message and returns), the buffer holds at most one message at that moment — but the loop is written generically with a labeled break for safety.
NoteL This may not be important as re-queuing should prevent messages from getting dropped.