changefeedccl: only checkpoint leading spans above lead threshold #138790
Conversation
This file contains 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
|
This change is |
andyyang890
force-pushed
the
20250109-checkpoint-lead-threshold
branch
8 times, most recently
from
11daf77 to
8644262
Compare
andyyang890
changed the title
This patch renames `changefeed.frontier_checkpoint_max_bytes` to `changefeed.span_checkpoint.max_bytes` for consistency. Release note: None
andyyang890
force-pushed
the
20250109-checkpoint-lead-threshold
branch
from
8644262 to
d2d3f8f
Compare
This patch renames `changefeed.frontier_checkpoint_frequency` to `changefeed.span_checkpoint.interval` for consistency. Release note: None
andyyang890
force-pushed
the
20250109-checkpoint-lead-threshold
branch
from
d2d3f8f to
8ec98be
Compare
andyyang890
requested review from
asg0451 and
wenyihu6
and removed request for
a team
stevendanna
reviewed
Jan 14, 2025
There was a problem hiding this comment.
I haven't looked closely so take this with a grain of salt, but I think this is the kind of change were we would benefit from the "painful elaboration of the obvious" in the commit message so that in a few months anyone partially familiar with changefeeds can understand we expect this change to do.
asg0451
reviewed
Jan 14, 2025
andyyang890
force-pushed
the
20250109-checkpoint-lead-threshold
branch
2 times, most recently
from
c7d49f2 to
f07beab
Compare
Thanks for the feedback, added a longer explanation in the commit message. |
…reshold setting This patch renames `changefeed.frontier_highwater_lag_checkpoint_threshold` to `changefeed.span_checkpoint.lead_threshold` for consistency. Release note (ops change): The cluster setting `changefeed.frontier_highwater_lag_checkpoint_threshold` has been renamed to `changefeed.span_checkpoint.lead_threshold`. The old name remains available for backwards-compatibility.
This patch updates the span-level checkpointing code to only checkpoint spans that are above the lead threshold (formerly lag threshold) so that we can at least maintain some minimal level of progress for spans that we go through the effort of checkpointing. Longer explanation: We create span-level checkpoints (at the configured interval) if we are either in a backfill or there's at least one span that is more than the lag/lead threshold ahead of the frontier min ts. Before this change, in the latter case, we would checkpoint all spans that are above the frontier min ts regardless of whether they are at least the lag/lead threshold ahead. This change makes it so that when we are creating a span-level checkpoint due to the lag/lead threshold, we will only include spans that are ahead of the frontier min ts by at least the lag/lead threshold so that we can maintain some minimal level of progress for the spans that we checkpoint. Release note: None
andyyang890
force-pushed
the
20250109-checkpoint-lead-threshold
branch
from
f07beab to
bdd0cd4
Compare
|
We discussed this change offline and decided the risks associated with this change aren't worth it given the planned work to improve span-level checkpoints. I pulled the first 3 commits renaming settings into a separate PR (#139064) and I'll close this PR soon. |
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.
See individual commits
Fixes #137857