Characterize stalled backlog reads after reload threshold changes - #12015
Draft
taylan-oai wants to merge 1 commit into
Draft
Characterize stalled backlog reads after reload threshold changes#12015taylan-oai wants to merge 1 commit into
taylan-oai 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.
Summary
Characterize a missed priority-reader wakeup when
matching.getTasksReloadAtchanges above the loaded-task count, so a follow-up fix has a deterministic reproduction.Problem
priTaskReader.completeTaskrequests another persistence read only when the remaining loaded count equals the current reload threshold. The pump can discard its previous notification while the buffer exceeds the old threshold. Raising the setting above the remaining count then allows successful completions to drain the buffer without requesting another read.For example, the test persists six tasks, loads four with threshold one, and completes two. Changing the threshold to three and completing the remaining loaded tasks leaves two persisted tasks unread, with an empty notification channel. Removing a temporary override to restore three produces the same result. Worker polls do not signal this pump, and the periodic metadata sync does not read tasks.
Approach
Add a comment at the equality check and a passing characterization test using
testing/synctest, the real dynamic-config getter, active reader loops, andtaskQueueDBbacked by the existing in-memory test manager. It covers unchanged, increased, restored, and decreased thresholds for both priority and fair readers. The stalled priority cases also demonstrate recovery through a subsequent persisted write and its reader notification.These assertions document current behavior; a future fix should change the stalled cases to require progress without another write. Production behavior is unchanged.
Two contained options for a follow-up, neither implemented or validated here:
Validation
go test -p 4 -tags test_dep ./service/matching -run '^TestReaderReloadThresholdChange$' -count=100passed all eight cases on each run.go test -p 4 -tags test_dep ./service/matching -run '^TestReaderReloadThresholdChange$' -race -count=20passed.make lint-code-fastpassed with the repository lint config,GOLANGCI_LINT_BASE_REV=1f7c7b0faba0270ea5cbc3798bbd574215d4a43b, andGOLANGCI_LINT_FIX=false; no new issues.The test exercises reader scheduling and the persistence wrapper; it does not use an external datastore, worker RPCs, or a production configuration client.
References
PR #10091 changes cold-start priority ordering. Its current diff leaves the completion-time equality check unchanged. PR #7429 introduced the dynamic reload setting.