-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-14889. Ability to check if a block has a replica on provided storage. #1573
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
💔 -1 overall
This message was automatically generated. |
public BlockInfoContiguous(short size) { | ||
super(size); | ||
hasProvidedStorage = false; |
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.
Can we just set by default as false and not do it here?
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.
I removed it for now.
@@ -77,9 +85,31 @@ boolean removeStorage(DatanodeStorageInfo storage) { | |||
setStorageInfo(dnIndex, getStorageInfo(lastNode)); | |||
// set the last entry to null | |||
setStorageInfo(lastNode, null); | |||
if (storage.getStorageType() == StorageType.PROVIDED | |||
&& !hasProvidedStorages()) { |
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.
Shouldn't this already guarantee is false?
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.
I see is the other method which goes over each DN, nevermind.
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.
Can you go over the overhead added to the regular pipeline without Provided?
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.
I removed the check and there is no invocation in the regular pipeline now.
return hasProvidedStorage; | ||
} | ||
|
||
private boolean hasProvidedStorages() { |
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.
High level javadoc.
...adoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockInfoStriped.java
Show resolved
Hide resolved
💔 -1 overall
This message was automatically generated. |
@Override | ||
boolean isProvided() { | ||
int len = getCapacity(); | ||
for(int idx = 0; idx < len; idx++) { |
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.
space after the for
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.
Fixed
int len = getCapacity(); | ||
for(int idx = 0; idx < len; idx++) { | ||
DatanodeStorageInfo cur = getStorageInfo(idx); | ||
if(cur != null) { |
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.
Space after the if
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.
Fixed
💔 -1 overall
This message was automatically generated. |
...t/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockInfo.java
Show resolved
Hide resolved
2b84c64
to
42381be
Compare
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
…rage. Contributed by Ashvin Agrawal. (#1573)"
…rage. Contributed by Ashvin Agrawal. (apache#1573)"
…rage. Contributed by Ashvin Agrawal. (apache#1573)"
Addresses https://issues.apache.org/jira/browse/HDFS-14889.
Adding a method in
BlockInfo
to return true of the block has any replica on a Provided storage.