Skip to content

Conversation

@zuston
Copy link
Member

@zuston zuston commented Nov 22, 2022

What changes were proposed in this pull request?

  1. Create a extra reference copy of inFlushed blocks to avoid breaking down original sequence, if not, it will cause missed data when reading due to the sequence reading when getting memory data.

Why are the changes needed?

Fix the bug of missed data due to block sorting

Does this PR introduce any user-facing change?

No

How was this patch tested?

  1. UTs

@zuston
Copy link
Member Author

zuston commented Nov 22, 2022

PTAL @jerqi

Copy link
Contributor

@jerqi jerqi 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 @zuston , wait for CI.

@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2022

Codecov Report

Merging #347 (bee5df2) into master (8ff41a5) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #347      +/-   ##
============================================
+ Coverage     58.39%   58.42%   +0.02%     
- Complexity     1552     1554       +2     
============================================
  Files           193      193              
  Lines         10761    10763       +2     
  Branches        937      937              
============================================
+ Hits           6284     6288       +4     
+ Misses         4103     4102       -1     
+ Partials        374      373       -1     
Impacted Files Coverage Δ
...rg/apache/uniffle/server/buffer/ShuffleBuffer.java 93.04% <100.00%> (+1.89%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zuston zuston merged commit 5b5acb5 into apache:master Nov 22, 2022
@zuston
Copy link
Member Author

zuston commented Nov 22, 2022

Merged. Thanks @jerqi for catching this bug.

@jerqi
Copy link
Contributor

jerqi commented Nov 22, 2022

@zuston
Copy link
Member Author

zuston commented Nov 23, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants