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 the NPE in minimizeDataMovement instance assignment strategy #11952

Merged
merged 1 commit into from
Nov 4, 2023

Conversation

jackjlli
Copy link
Contributor

@jackjlli jackjlli commented Nov 3, 2023

This PR fixes the NPE when minimizeDataMovement is enabled and a new pool is added to the existing instance partition assignment.

The problem of replicaGroupId mapping to a different pool will be handled in a separate PR.

Thanks @Ferix9288 for identifying the issue!
#11727

Unit test brought from this PR: #11765

@Ferix9288
Copy link
Contributor

Excellent, thanks Jack 🙌 Keep me updated on the follow-up fix for the shifting replicaGroupId to different pool

@codecov-commenter
Copy link

codecov-commenter commented Nov 3, 2023

Codecov Report

Merging #11952 (1841b56) into master (ee4b834) will increase coverage by 0.00%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master   #11952   +/-   ##
=========================================
  Coverage     61.40%   61.41%           
  Complexity      207      207           
=========================================
  Files          2378     2378           
  Lines        128902   128932   +30     
  Branches      19929    19939   +10     
=========================================
+ Hits          79157    79185   +28     
- Misses        44022    44034   +12     
+ Partials       5723     5713   -10     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (ø)
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 61.35% <100.00%> (-0.01%) ⬇️
java-21 61.29% <100.00%> (+<0.01%) ⬆️
skip-bytebuffers-false 61.40% <100.00%> (+0.01%) ⬆️
skip-bytebuffers-true 61.23% <100.00%> (-0.03%) ⬇️
temurin 61.41% <100.00%> (+<0.01%) ⬆️
unittests 61.41% <100.00%> (+<0.01%) ⬆️
unittests1 46.62% <ø> (+0.03%) ⬆️
unittests2 27.64% <100.00%> (-0.02%) ⬇️

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

Files Coverage Δ
...nstance/InstanceReplicaGroupPartitionSelector.java 91.62% <100.00%> (ø)

... and 23 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@jackjlli jackjlli force-pushed the fix-npe-for-minimizeDataMovement branch from 8315d89 to 1841b56 Compare November 4, 2023 04:19
@Jackie-Jiang Jackie-Jiang merged commit 5904df3 into master Nov 4, 2023
19 checks passed
@jackjlli jackjlli deleted the fix-npe-for-minimizeDataMovement branch November 5, 2023 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants