Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(streaming): create mview progress report under concurrent checkpoint #3602

Merged
merged 5 commits into from
Jul 2, 2022

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Jul 1, 2022

Signed-off-by: Bugen Zhao i@bugenzhao.com

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

As title. See #3598 for more details.

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Refer to a related PR or issue link (optional)

@BugenZhao BugenZhao requested review from hzxa21 and xxhZs July 1, 2022 11:01
@github-actions github-actions bot added the type/fix Bug fix label Jul 1, 2022
Copy link
Contributor

@MrCroxx MrCroxx left a comment

Choose a reason for hiding this comment

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

Rest LGTM ❤️

@codecov
Copy link

codecov bot commented Jul 1, 2022

Codecov Report

Merging #3602 (8390e18) into main (79e41a5) will decrease coverage by 0.00%.
The diff coverage is 73.52%.

@@            Coverage Diff             @@
##             main    #3602      +/-   ##
==========================================
- Coverage   74.31%   74.30%   -0.01%     
==========================================
  Files         772      772              
  Lines      109170   109180      +10     
==========================================
+ Hits        81128    81129       +1     
- Misses      28042    28051       +9     
Flag Coverage Δ
rust 74.30% <73.52%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/stream/src/executor/rearranged_chain.rs 0.00% <0.00%> (ø)
src/stream/src/task/barrier_manager/progress.rs 61.29% <66.66%> (+0.50%) ⬆️
src/stream/src/executor/chain.rs 90.52% <100.00%> (-0.30%) ⬇️
...c/stream/src/task/barrier_manager/managed_state.rs 73.52% <100.00%> (+0.80%) ⬆️
src/meta/src/manager/id.rs 94.94% <0.00%> (-1.13%) ⬇️
src/meta/src/hummock/mock_hummock_meta_client.rs 40.56% <0.00%> (-0.95%) ⬇️
src/storage/src/hummock/iterator/merge_inner.rs 89.37% <0.00%> (-0.63%) ⬇️
src/frontend/src/expr/utils.rs 98.99% <0.00%> (-0.26%) ⬇️
src/common/src/types/ordered_float.rs 24.70% <0.00%> (-0.20%) ⬇️
src/storage/src/hummock/local_version_manager.rs 80.96% <0.00%> (-0.12%) ⬇️
... and 2 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@xxhZs xxhZs left a comment

Choose a reason for hiding this comment

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

LGTM, I will test it with #3511

Copy link
Collaborator

@hzxa21 hzxa21 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the quick fix!

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao force-pushed the bz/fix-create-mview-report-under-concurrent-checkpoint branch from f26dac6 to fc71592 Compare July 1, 2022 11:23
@BugenZhao
Copy link
Member Author

LGTM, I will test it with #3511

Cool. You may merge this PR after tested.

@xxhZs xxhZs enabled auto-merge (squash) July 2, 2022 01:57
@xxhZs xxhZs disabled auto-merge July 2, 2022 01:58
@xxhZs xxhZs enabled auto-merge (squash) July 2, 2022 01:58
@xxhZs xxhZs disabled auto-merge July 2, 2022 05:09
@xxhZs xxhZs enabled auto-merge (squash) July 2, 2022 05:10
@BugenZhao
Copy link
Member Author

Seems this PR doesn't fully resolve the issue. Will remove "close" in PR body.

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
… of github.com:singularity-data/risingwave into bz/fix-create-mview-report-under-concurrent-checkpoint
@BugenZhao BugenZhao added the mergify/can-merge Indicates that the PR can be added to the merge queue label Jul 2, 2022
@xxhZs xxhZs merged commit faa6c30 into main Jul 2, 2022
@xxhZs xxhZs deleted the bz/fix-create-mview-report-under-concurrent-checkpoint branch July 2, 2022 05:54
huangjw806 pushed a commit that referenced this pull request Jul 5, 2022
…int (#3602)

* fix(streaming): create mview progress report under concurrent checkpoint

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

* refine docs

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

Co-authored-by: Xinhao Xu <84456268+xxhZs@users.noreply.github.com>
nasnoisaac pushed a commit to nasnoisaac/risingwave that referenced this pull request Aug 9, 2022
…int (risingwavelabs#3602)

* fix(streaming): create mview progress report under concurrent checkpoint

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

* refine docs

Signed-off-by: Bugen Zhao <i@bugenzhao.com>

Co-authored-by: Xinhao Xu <84456268+xxhZs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergify/can-merge Indicates that the PR can be added to the merge queue type/fix Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants