Frontend peek sequencing -- RTR #34238
Merged
+120
−72
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
ggevay
force-pushed
the
frontend-peek-rtr
branch
2 times, most recently
from
8f87429 to
fcbd0aa
Compare
ggevay
commented
Nov 22, 2025
|
|
||
| match result { | ||
| Ok(Some(fut)) => { | ||
| task::spawn(|| "determine real time recent timestamp", async move { |
Contributor
Author
There was a problem hiding this comment.
This corresponds to the spawning in the old peek sequencing's peek_real_time_recency.
ggevay
commented
Nov 22, 2025
| &self, | ||
| source_ids: impl Iterator<Item = GlobalId>, | ||
| real_time_recency_timeout: Duration, | ||
| ) -> Result<Option<BoxFuture<'static, Result<Timestamp, StorageError<Timestamp>>>>, AdapterError> |
Contributor
Author
There was a problem hiding this comment.
This big green block is mostly just code movement from the old determine_real_time_recent_timestamp's inner part.
ggevay
force-pushed
the
frontend-peek-rtr
branch
from
fcbd0aa to
f0b69ff
Compare
aljoscha
approved these changes
Nov 24, 2025
Contributor
aljoscha
left a comment
aljoscha
left a comment
There was a problem hiding this comment.
I think the changes look fine, and I'm counting on tests here to work correctly
| ---- | ||
| COMPLETE 0 | ||
|
|
||
| # TODO(ggevay): I think this is not actually testing real-time recency, because tables don't participate in the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR makes progress on https://github.com/MaterializeInc/database-issues/issues/9593, implementing real-time recency support in the new peek sequencing, so that we have one less fallback to the old peek sequencing.
I decided to not pull the infra that is need for RTR from the Coordinator to the Frontend, but just added a Coordinator command that the Frontend can use to ask the Coordinator for an RTR timestamp. The roundtrip for this command takes some time and could conceivably be a Coordinator main task bottleneck, but should be ok for now. RTR is quite slow anyway, and GA'ing it is not a high priority at the moment, according to the roadmap item in Notion.
Note that we are reaching into
sequencer/innerfrom the outside (fromcommand_handler.rs), which is not nice. I'd like to resolve this later, when we remove the old peek sequencing code, at which point we'll find some nice place for such currentlyinnerfunctions that are accessed by the frontend peek sequencing. I've added this to the "Important refactorings" section of https://github.com/MaterializeInc/database-issues/issues/9593Nightly: https://buildkite.com/materialize/nightly/builds/14153 The failures seem unrelated to this PR.
Note that the PR currently has an extra commit to turn on
enable_frontend_peek_sequencingin CI, so that CI tests the new code. I'll remove this commit before merging the PR, as discussed earlier.