adapter: retire linearize reads from AdvanceTimelines to fix residual starvation#37512
Draft
antiguru wants to merge 1 commit into
Draft
adapter: retire linearize reads from AdvanceTimelines to fix residual starvation#37512antiguru wants to merge 1 commit into
antiguru wants to merge 1 commit into
Conversation
A strict serializable read that picks a timestamp ahead of the oracle becomes a pending linearize read that retires only once the oracle advances. The oracle advances only via group commit. In the coordinator's biased select, both the branch that enqueues a pending read and the branch that re-checks pending reads sat below user command intake (`cmd_rx`), so a sustained write flood kept `cmd_rx` runnable and starved retirement, even as the same flood advanced the oracle past the read's timestamp on the higher group commit branch. Reads stayed pending until the flood drained. Reordering the select only relocates the victim. Instead couple retirement to its causal signal: run `message_linearize_reads` from the `Message::AdvanceTimelines` handler, which group commit emits on the top-priority internal channel after advancing the oracle. Move the enqueue branch above `cmd_rx`, which is safe because enqueue is causally downstream of `cmd_rx` and cannot outrun it. The re-check branch stays below `cmd_rx` as the idle-period low-latency path. Add the `LinearizeReadStarvation` parallel-benchmark scenario as a regression guard and record the priority-inversion class in the adapter guide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
Seems to fix the issue: https://buildkite.com/materialize/nightly/builds/17070#019f4244-c227-4fa3-b12b-492b699ade91/L340 |
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
1 participant
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.
Follow-up to #37316. That fix stopped the linearize re-check from starving group commit, but left a residual priority inversion: in the biased coordinator select, both the branch enqueuing a pending strict serializable read and the re-check branch sit below
cmd_rx, so a sustained write flood starves retirement even while the same flood advances the oracle past the read's timestamp on the higher group commit branch. The read stays pending until the flood drains. Repro and evidence in #37316 (comment).Reordering only relocates the victim. Instead, retirement rides its causal signal:
message_linearize_readsnow runs from theMessage::AdvanceTimelineshandler, which group commit emits on the top-priority internal channel after advancing the oracle. The enqueue branch moves abovecmd_rx(safe because it is causally downstream ofcmd_rxand cannot outrun it). The re-check branch stays belowcmd_rxas the idle-period path.Adds def-'s
LinearizeReadStarvationparallel-benchmark scenario as a regression guard and records the priority-inversion class in the adapter guide.Draft: local build is currently broken, so the benchmark RED→GREEN has not been re-run locally. Left as draft pending that verification.
🤖 Generated with Claude Code