-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-16321. Fix invalid config in TestAvailableSpaceRackFaultTolerantBPP #3655
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me, Should be good to go if no test failures
@Test | ||
public void testDefaultConfigValue() { | ||
Assert.assertEquals( | ||
conf.get("DFS_NAMENODE_AVAILABLE_SPACE_RACK_FAULT_TOLERANT_BLOCK_PLACEMENT_POLICY_BALANCED_SPACE_PREFERENCE_FRACTION_KEY") | ||
,0.6f); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't required, this isn't checking the BPP, but how the conf object works. you can remove this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't required, this isn't checking the BPP, but how the conf object works. you can remove this test.
Thanks @ayushtkn for your review. I have removed the test case, but not sure if the CI will pass if no tests added. hope it works.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@ayushtkn Could you kindly help check the timeout cases, I have tried twice ,the errors seem not related with this patch, and they occurs in different methods, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test failures seems not related.
Changes LGTM
…BPP (#3655). Contributed by guo. Signed-off-by: Ayush Saxena <ayushsaxena@apache.org>
…BPP (apache#3655). Contributed by guo. Signed-off-by: Ayush Saxena <ayushsaxena@apache.org>
TestAvailableSpaceRackFaultTolerantBPP
seems setting invalid param(valid inTestAvailableSpaceBlockPlacementPolicy
), we can fix it to avoid further trouble.