Skip to content

Conversation

@BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Feb 23, 2022

What's changed and what's your intention?

Before, we swallow the conf change barrier for newly created MoM, which will makes the first message of these actors be Chunk instead of Barrier. However, in #511 we assume the first message of any actors must be Barrier.

This PR makes global barrier manager collect from the newly created actors of MoM as well. Also, we make ChainExecutor propagate the conf change barrier to init epoch for all downstream actors.

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

Refer to a related PR or issue link (optional)

#511

@github-actions github-actions bot added the type/feature Type: New feature. label Feb 23, 2022
Signed-off-by: Bugen Zhao <[email protected]>
Copy link
Member

@yezizp2012 yezizp2012 left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Feb 23, 2022

Codecov Report

Merging #512 (b372e73) into main (3a62e59) will decrease coverage by 0.05%.
The diff coverage is 86.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #512      +/-   ##
============================================
- Coverage     72.70%   72.64%   -0.06%     
  Complexity     2709     2709              
============================================
  Files           876      876              
  Lines         48902    48910       +8     
  Branches       1557     1557              
============================================
- Hits          35554    35533      -21     
- Misses        12537    12566      +29     
  Partials        811      811              
Flag Coverage Δ
java 63.40% <ø> (ø)
rust 76.56% <86.66%> (-0.08%) ⬇️

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

Impacted Files Coverage Δ
rust/meta/src/cluster/mod.rs 56.92% <ø> (ø)
rust/meta/src/hummock/hummock_manager.rs 67.98% <ø> (ø)
rust/meta/src/barrier/mod.rs 56.86% <60.00%> (-1.14%) ⬇️
rust/meta/src/barrier/command.rs 42.59% <100.00%> (+4.59%) ⬆️
rust/meta/src/stream/meta.rs 49.39% <100.00%> (+0.61%) ⬆️
rust/stream/src/executor/chain.rs 57.79% <100.00%> (-25.54%) ⬇️
rust/stream/src/executor/barrier_align.rs 78.78% <0.00%> (ø)
rust/common/src/types/ordered_float.rs 25.82% <0.00%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a62e59...b372e73. Read the comment docs.

@BugenZhao BugenZhao marked this pull request as ready for review February 23, 2022 14:11
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.

Thanks for the PR. LGTM.

@BugenZhao BugenZhao merged commit 0fc6522 into main Feb 24, 2022
@BugenZhao BugenZhao deleted the bz/collect-from-creating-actors branch February 24, 2022 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/feature Type: New feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants