feat(snapshot-backfill): measure accumulated row count for each vnode stream #20016
+236
−29
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 stack of pull requests is managed by Graphite. Learn more about stacking. |
9 tasks
hzxa21
approved these changes
Jan 3, 2025
| // The pending futures will be stored in a separate linked list and will not be polled until the special cx is awakened | ||
| // and move the awakened future from the linked list to a ready queue. | ||
| // | ||
| // However, here if we use `FuturesUnordered::iter_mut` to in place access the pending futures and call `peek` directly, |
There was a problem hiding this comment.
We use iter_mut and peek prior to this pr as well. Does it mean that this issue existed before but we didn't hit it in test?
There was a problem hiding this comment.
Yes, peek is used in for_vnode_pk_progress. The method is implemented but not called yet.
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.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Part of #19720
Measure the row count of each vnode stream, so that the row count can be saved in the state to be introduced in #19720.
This PR will also fix a bug found in the CI #19720. In short, the bug is caused by the use of
FuturesUnordered. InFuturesUnordered, pending futures will be stored separately and will not be polled until a special cx generated inFuturesUnordered::poll_nextis awakened. However, when we usePeekable::peekto peek the latest progress of the stream, the special cx will be overwritten when we poll thePeekfuture, and then even the future is ready after peeked, it is still not in the ready queue, and will not be polled and get stuck forever.Tests are added to cover the bug.
Checklist
Documentation
Release note