Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(stream): shortcircuit flushing chunks on barrier #20523

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

kwannoel
Copy link
Contributor

@kwannoel kwannoel commented Feb 18, 2025

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

No need to iterate through the rest of the buffer after hitting a flushed item / chunk. Because the rest of the chunks should already be flushed

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

  • My PR needs documentation updates.
Release note

Copy link
Contributor Author

kwannoel commented Feb 18, 2025

@kwannoel kwannoel changed the title shortcircuit flush tracing refactor(stream): shortcircuit flushing chunks on barrier Feb 18, 2025
@kwannoel kwannoel marked this pull request as ready for review February 18, 2025 04:15
@kwannoel kwannoel requested a review from wenym1 February 18, 2025 04:45
@kwannoel kwannoel force-pushed the kwannoel/handle-pause branch from aa35e12 to 78b0623 Compare February 25, 2025 05:34
@kwannoel kwannoel force-pushed the kwannoel/sync-improve branch from 593e344 to fffaf55 Compare February 25, 2025 05:34
@kwannoel kwannoel requested a review from wenym1 February 25, 2025 05:48
writer.write_chunk(chunk, *epoch, *start_seq_id, *end_seq_id)?;
*flushed = true;
if barrier.is_checkpoint() {
for (epoch, item) in &mut buffer.buffer {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that when we add items to buffer, we use buffer.push_back(...), which means a newer item is at the back. Therefore, should we iterate from back to front instead?

@kwannoel kwannoel force-pushed the kwannoel/sync-improve branch from 637427a to a3e8c86 Compare February 25, 2025 12:49
@kwannoel kwannoel force-pushed the kwannoel/handle-pause branch from 78b0623 to 78df558 Compare February 25, 2025 12:49
@kwannoel kwannoel force-pushed the kwannoel/handle-pause branch from 78df558 to e6c6017 Compare February 26, 2025 07:34
@kwannoel kwannoel force-pushed the kwannoel/sync-improve branch from a3e8c86 to 099c0fc Compare February 26, 2025 07:34
@kwannoel kwannoel force-pushed the kwannoel/handle-pause branch from e6c6017 to 80359d9 Compare February 27, 2025 06:43
@kwannoel kwannoel force-pushed the kwannoel/sync-improve branch from d17eae3 to 494bafc Compare February 27, 2025 06:43
@kwannoel kwannoel force-pushed the kwannoel/handle-pause branch from 80359d9 to 52d2229 Compare February 27, 2025 08:41
@kwannoel kwannoel force-pushed the kwannoel/sync-improve branch from 494bafc to f582b85 Compare February 27, 2025 08:41
@kwannoel kwannoel force-pushed the kwannoel/handle-pause branch from 52d2229 to d318c81 Compare February 27, 2025 08:47
@kwannoel kwannoel force-pushed the kwannoel/sync-improve branch from f582b85 to d2f28f0 Compare February 27, 2025 08:47
Base automatically changed from kwannoel/handle-pause to main February 27, 2025 16:37
@kwannoel kwannoel added this pull request to the merge queue Feb 27, 2025
Merged via the queue into main with commit 66ffccf Feb 27, 2025
28 of 29 checks passed
@kwannoel kwannoel deleted the kwannoel/sync-improve branch February 27, 2025 17:20
Standing-Man pushed a commit to Standing-Man/risingwave that referenced this pull request Feb 28, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants