You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/07/10 10:55:48 UTC
[GitHub] [hadoop-ozone] rakeshadr opened a new pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
rakeshadr opened a new pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192
## What changes were proposed in this pull request?
Similar to OzoneManagerFS#lookupFile(OmKeyArgs args, String clientAddress) interface, "clientAddress" has been passed as an argument to OzoneManagerFS#listStatus(), OzoneManagerFS#getFileStatus() APIs.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3947
## How was this patch tested?
Used existing TestKeyManagerImpl unit test cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#issuecomment-679207273
/pending "@rakeshadr , can you plz add some details here as to why sorting of dns is required ?"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr closed pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
rakeshadr closed pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#issuecomment-686379647
I am afraid something wrong happened with the last rebase/merge:
```
Files changed 737
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on a change in pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#discussion_r462864723
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1016,6 +1016,7 @@ public OzoneFileStatus getOzoneFileStatus(String volumeName,
.setBucketName(bucketName)
.setKeyName(keyName)
.setRefreshPipeline(true)
+ .setSortDatanodesInPipeline(topologyAwareReadEnabled)
Review comment:
@rakeshadr , can you plz add some details here as to why sorting of dns is required ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] bshashikant commented on a change in pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#discussion_r462864723
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1016,6 +1016,7 @@ public OzoneFileStatus getOzoneFileStatus(String volumeName,
.setBucketName(bucketName)
.setKeyName(keyName)
.setRefreshPipeline(true)
+ .setSortDatanodesInPipeline(topologyAwareReadEnabled)
Review comment:
@rakeshadr , can you plz add some details here why sorting of dns is required here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr edited a comment on pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
rakeshadr edited a comment on pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#issuecomment-686430271
yes, I polluted the PR. Thanks @elek for the suggestion. I have created another branch.
@bshashikant , @adoroszlai I have created fresh PR https://github.com/apache/hadoop-ozone/pull/1385 as this PR has been polluted:-)
I have addressed review comments in that new PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr edited a comment on pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
rakeshadr edited a comment on pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#issuecomment-686430271
yes, I polluted the PR. Thanks @elek for the suggestion. I have create another branch.
@bshashikant , @adoroszlai I have created fresh PR https://github.com/apache/hadoop-ozone/pull/1385 as this PR has been polluted:-)
I have addressed review comments in that new PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#discussion_r481025587
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java
##########
@@ -31,7 +31,18 @@
* Ozone Manager FileSystem interface.
*/
public interface OzoneManagerFS extends IOzoneAcl {
- OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException;
+
+ /**
+ * Get file status for a file or a directory.
+ *
+ * @param args the args of the key provided by client.
+ * @param clientAddress a hint to key manager, order the datanode in returned
+ * pipeline by distance between client and datanode.
+ * @return file status.
+ * @throws IOException
+ */
+ OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress)
Review comment:
Should we also keep the two original methods? Even if backwards compatibility is not required for this interface, it would let us avoid changing lots of calls just to pass `null`.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java
##########
@@ -31,7 +31,18 @@
* Ozone Manager FileSystem interface.
*/
public interface OzoneManagerFS extends IOzoneAcl {
- OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException;
+
+ /**
+ * Get file status for a file or a directory.
+ *
+ * @param args the args of the key provided by client.
+ * @param clientAddress a hint to key manager, order the datanode in returned
+ * pipeline by distance between client and datanode.
+ * @return file status.
+ * @throws IOException
Review comment:
Thank you for adding updated doc from `KeyManagerImpl` to the interface. Is there a reason exception is not described (the cases when they are thrown)? Also, can you please remove the old javadoc from the implementation?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#discussion_r482022145
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java
##########
@@ -31,7 +31,18 @@
* Ozone Manager FileSystem interface.
*/
public interface OzoneManagerFS extends IOzoneAcl {
- OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException;
+
+ /**
+ * Get file status for a file or a directory.
+ *
+ * @param args the args of the key provided by client.
+ * @param clientAddress a hint to key manager, order the datanode in returned
+ * pipeline by distance between client and datanode.
+ * @return file status.
+ * @throws IOException
Review comment:
No specific reason, I will describe exception cases.
I will update the KeyManagerImpl implementation as well.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java
##########
@@ -31,7 +31,18 @@
* Ozone Manager FileSystem interface.
*/
public interface OzoneManagerFS extends IOzoneAcl {
- OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException;
+
+ /**
+ * Get file status for a file or a directory.
+ *
+ * @param args the args of the key provided by client.
+ * @param clientAddress a hint to key manager, order the datanode in returned
+ * pipeline by distance between client and datanode.
+ * @return file status.
+ * @throws IOException
+ */
+ OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress)
Review comment:
Agreed, will keep the existing #getFileStatus(args) method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#issuecomment-686430271
@bshashikant , @adoroszlai I have created fresh PR https://github.com/apache/hadoop-ozone/pull/1385 as this PR has been polluted:-)
I have addressed review comments in that new PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek edited a comment on pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
elek edited a comment on pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#issuecomment-686379647
I am afraid something wrong happened with the last rebase/merge:
```
Files changed 737
```
Seems that you rebased the master to your branch instead of rebasing your branch to the master.
The easiest way to fix can be the re-creation of the branch, cherry-picking required commits and doing force push.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1192: HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1192:
URL: https://github.com/apache/hadoop-ozone/pull/1192#discussion_r479996087
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1016,6 +1016,7 @@ public OzoneFileStatus getOzoneFileStatus(String volumeName,
.setBucketName(bucketName)
.setKeyName(keyName)
.setRefreshPipeline(true)
+ .setSortDatanodesInPipeline(topologyAwareReadEnabled)
Review comment:
Thanks @bshashikant for the reviews. This change is basically to make "getFileStatus" and "listStatus" apis similar to "lookupFile()" API, where it does sorting.
Reference:
https://github.com/apache/hadoop-ozone/pull/1164#discussion_r451122204
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org