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/06 06:11:16 UTC
[GitHub] [hadoop-ozone] rakeshadr opened a new pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
rakeshadr opened a new pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164
## What changes were proposed in this pull request?
Refresh pipeline info does a call to SCM and it can be moved outside the BUCKET_LOCK, this would help to improve the performance of read/write mix workloads.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3824
## How was this patch tested?
Using TestOzoneFileSystem 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] rakeshadr commented on a change in pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r452196471
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1706,36 +1718,41 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException {
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
- volumeName, bucketName, keyName);
- OmKeyInfo fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+ volumeName, bucketName, keyName);
+ fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+
+ // Check if the key is a directory.
+ if (fileKeyInfo == null) {
+ String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
+ String dirKeyBytes = metadataManager.getOzoneKey(
+ volumeName, bucketName, dirKey);
+ OmKeyInfo dirKeyInfo = metadataManager.getKeyTable().get(dirKeyBytes);
+ if (dirKeyInfo != null) {
+ return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
+ }
+ }
+ } finally {
+ metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+ bucketName);
+
+ // if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileKeyInfo != null) {
- if (args.getRefreshPipeline()) {
+ if (refreshPipeline) {
refreshPipeline(fileKeyInfo);
}
Review comment:
@xiaoyuyao, IMHO, your proposal is unrelated to this perf changes and for focussed reviews/commits I have created HDDS-3947 jira to handle your comment. Hope this is fine with you. Thanks!
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r452196471
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1706,36 +1718,41 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException {
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
- volumeName, bucketName, keyName);
- OmKeyInfo fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+ volumeName, bucketName, keyName);
+ fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+
+ // Check if the key is a directory.
+ if (fileKeyInfo == null) {
+ String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
+ String dirKeyBytes = metadataManager.getOzoneKey(
+ volumeName, bucketName, dirKey);
+ OmKeyInfo dirKeyInfo = metadataManager.getKeyTable().get(dirKeyBytes);
+ if (dirKeyInfo != null) {
+ return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
+ }
+ }
+ } finally {
+ metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+ bucketName);
+
+ // if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileKeyInfo != null) {
- if (args.getRefreshPipeline()) {
+ if (refreshPipeline) {
refreshPipeline(fileKeyInfo);
}
Review comment:
@xiaoyuyao, IMHO, your proposal is unrelated to this perf changes and to do focussed review/commit, I have created HDDS-3947 jira to handle your comment. Hope this is fine with you. Thanks!
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r451327683
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1706,36 +1718,41 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException {
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
- volumeName, bucketName, keyName);
- OmKeyInfo fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+ volumeName, bucketName, keyName);
+ fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+
+ // Check if the key is a directory.
+ if (fileKeyInfo == null) {
+ String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
+ String dirKeyBytes = metadataManager.getOzoneKey(
+ volumeName, bucketName, dirKey);
+ OmKeyInfo dirKeyInfo = metadataManager.getKeyTable().get(dirKeyBytes);
+ if (dirKeyInfo != null) {
+ return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
+ }
+ }
+ } finally {
+ metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+ bucketName);
+
+ // if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileKeyInfo != null) {
- if (args.getRefreshPipeline()) {
+ if (refreshPipeline) {
refreshPipeline(fileKeyInfo);
}
Review comment:
Thanks for the review. Good point. For sorting dns, we need to introduce a new arg **clientAddress** to APIs [listStatus](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java#L52), [getFileStatus](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java#L34) interfaces.
Need to pass a new argument **clientAddress** like [lookupFile](https://github.com/apache/hadoop-ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java#L45) API. Can we do interface changes in a separate jira task ?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -659,15 +660,6 @@ public OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress)
});
}
}
- // Refresh container pipeline info from SCM
Review comment:
Thanks for the review. Sure, will do it.
----------------------------------------------------------------
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] xiaoyuyao commented on a change in pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r453918089
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1877,26 +1897,18 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
String volumeName = args.getVolumeName();
String bucketName = args.getBucketName();
String keyName = args.getKeyName();
-
- metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
- bucketName);
- try {
- OzoneFileStatus fileStatus = getFileStatus(args);
- if (fileStatus.isFile()) {
- if (args.getRefreshPipeline()) {
- refreshPipeline(fileStatus.getKeyInfo());
- }
- if (args.getSortDatanodes()) {
- sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress);
- }
- return fileStatus.getKeyInfo();
- }
+ OzoneFileStatus fileStatus = getOzoneFileStatus(volumeName, bucketName,
+ keyName, false);
//if key is not of type file or if key is not found we throw an exception
- } finally {
- metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
- bucketName);
+ if (fileStatus != null && fileStatus.isFile()) {
+ if (args.getRefreshPipeline()) {
+ refreshPipeline(fileStatus.getKeyInfo());
+ }
+ if (args.getSortDatanodes()) {
Review comment:
NIT: sortDatanodes can be handled similarly in getOzoneFileStatus?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1877,26 +1897,18 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
String volumeName = args.getVolumeName();
String bucketName = args.getBucketName();
String keyName = args.getKeyName();
-
- metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
- bucketName);
- try {
- OzoneFileStatus fileStatus = getFileStatus(args);
- if (fileStatus.isFile()) {
- if (args.getRefreshPipeline()) {
- refreshPipeline(fileStatus.getKeyInfo());
- }
- if (args.getSortDatanodes()) {
- sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress);
- }
- return fileStatus.getKeyInfo();
- }
+ OzoneFileStatus fileStatus = getOzoneFileStatus(volumeName, bucketName,
+ keyName, false);
//if key is not of type file or if key is not found we throw an exception
- } finally {
- metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
- bucketName);
+ if (fileStatus != null && fileStatus.isFile()) {
+ if (args.getRefreshPipeline()) {
+ refreshPipeline(fileStatus.getKeyInfo());
Review comment:
The last parameter of getOzoneFileStatus() should have refreshPipeline handled already. Can we pass args.getRefreshPipeline() on line 1901?
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r454104113
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1877,26 +1897,18 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
String volumeName = args.getVolumeName();
String bucketName = args.getBucketName();
String keyName = args.getKeyName();
-
- metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
- bucketName);
- try {
- OzoneFileStatus fileStatus = getFileStatus(args);
- if (fileStatus.isFile()) {
- if (args.getRefreshPipeline()) {
- refreshPipeline(fileStatus.getKeyInfo());
- }
- if (args.getSortDatanodes()) {
- sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress);
- }
- return fileStatus.getKeyInfo();
- }
+ OzoneFileStatus fileStatus = getOzoneFileStatus(volumeName, bucketName,
+ keyName, false);
//if key is not of type file or if key is not found we throw an exception
- } finally {
- metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
- bucketName);
+ if (fileStatus != null && fileStatus.isFile()) {
+ if (args.getRefreshPipeline()) {
+ refreshPipeline(fileStatus.getKeyInfo());
+ }
+ if (args.getSortDatanodes()) {
Review comment:
Done
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#issuecomment-657969627
Thanks @xiaoyuyao for the comments. I have updated PR, kindly review it again.
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#issuecomment-659786368
Thanks @xiaoyuyao and @bharatviswa504 for the reviews and committing the 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] codecov-commenter commented on pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#issuecomment-655382465
# [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1164?src=pr&el=h1) Report
> Merging [#1164](https://codecov.io/gh/apache/hadoop-ozone/pull/1164?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/6eebf2f07049f3e9a5788cd43ea9ba3fadc31dcc&el=desc) will **decrease** coverage by `0.13%`.
> The diff coverage is `79.59%`.
[](https://codecov.io/gh/apache/hadoop-ozone/pull/1164?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1164 +/- ##
============================================
- Coverage 73.50% 73.36% -0.14%
+ Complexity 10031 10025 -6
============================================
Files 973 974 +1
Lines 49663 49721 +58
Branches 4881 4896 +15
============================================
- Hits 36503 36477 -26
- Misses 10846 10914 +68
- Partials 2314 2330 +16
```
| [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1164?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...ava/org/apache/hadoop/ozone/om/KeyManagerImpl.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9LZXlNYW5hZ2VySW1wbC5qYXZh) | `63.96% <79.59%> (-0.38%)` | `128.00 <7.00> (+3.00)` | :arrow_down: |
| [...otocol/commands/RetriableDatanodeEventWatcher.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL3Byb3RvY29sL2NvbW1hbmRzL1JldHJpYWJsZURhdGFub2RlRXZlbnRXYXRjaGVyLmphdmE=) | `55.55% <0.00%> (-44.45%)` | `3.00% <0.00%> (-1.00%)` | |
| [...hdds/scm/container/CloseContainerEventHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9DbG9zZUNvbnRhaW5lckV2ZW50SGFuZGxlci5qYXZh) | `72.41% <0.00%> (-17.25%)` | `6.00% <0.00%> (ø%)` | |
| [...hdds/scm/container/common/helpers/ExcludeList.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vY29udGFpbmVyL2NvbW1vbi9oZWxwZXJzL0V4Y2x1ZGVMaXN0LmphdmE=) | `86.95% <0.00%> (-13.05%)` | `19.00% <0.00%> (-3.00%)` | |
| [...ozone/container/ozoneimpl/ContainerController.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvb3pvbmVpbXBsL0NvbnRhaW5lckNvbnRyb2xsZXIuamF2YQ==) | `63.15% <0.00%> (-10.53%)` | `11.00% <0.00%> (-2.00%)` | |
| [...ache/hadoop/ozone/om/codec/S3SecretValueCodec.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9jb2RlYy9TM1NlY3JldFZhbHVlQ29kZWMuamF2YQ==) | `90.90% <0.00%> (-9.10%)` | `3.00% <0.00%> (-1.00%)` | |
| [...e/hadoop/ozone/recon/tasks/OMUpdateEventBatch.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL3JlY29uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3Avb3pvbmUvcmVjb24vdGFza3MvT01VcGRhdGVFdmVudEJhdGNoLmphdmE=) | `75.00% <0.00%> (-8.34%)` | `6.00% <0.00%> (-1.00%)` | |
| [.../transport/server/ratis/ContainerStateMachine.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvQ29udGFpbmVyU3RhdGVNYWNoaW5lLmphdmE=) | `71.52% <0.00%> (-5.39%)` | `63.00% <0.00%> (-4.00%)` | |
| [.../common/volume/RoundRobinVolumeChoosingPolicy.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3ZvbHVtZS9Sb3VuZFJvYmluVm9sdW1lQ2hvb3NpbmdQb2xpY3kuamF2YQ==) | `80.95% <0.00%> (-4.77%)` | `5.00% <0.00%> (-1.00%)` | |
| [...apache/hadoop/hdds/server/events/EventWatcher.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvZnJhbWV3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zZXJ2ZXIvZXZlbnRzL0V2ZW50V2F0Y2hlci5qYXZh) | `77.77% <0.00%> (-4.17%)` | `14.00% <0.00%> (ø%)` | |
| ... and [23 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1164/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1164?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1164?src=pr&el=footer). Last update [6eebf2f...3547106](https://codecov.io/gh/apache/hadoop-ozone/pull/1164?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#issuecomment-656760744
@xiaoyuyao @bharatviswa504,
Please review the updated changes when you get a chance. Thanks!
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r454789813
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -2083,6 +2092,11 @@ private void listStatusFindKeyInTableCache(
metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
bucketName);
}
+ if (args.getRefreshPipeline()) {
+ for(OzoneFileStatus fileStatus : fileStatusList){
+ refreshPipeline(fileStatus.getKeyInfo());
Review comment:
Thanks @xiaoyuyao , I have raised HDDS-3961 to handle this case.
----------------------------------------------------------------
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] xiaoyuyao commented on a change in pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r454547254
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -2083,6 +2092,11 @@ private void listStatusFindKeyInTableCache(
metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
bucketName);
}
+ if (args.getRefreshPipeline()) {
+ for(OzoneFileStatus fileStatus : fileStatusList){
+ refreshPipeline(fileStatus.getKeyInfo());
Review comment:
I'm +1 on this JIRA.
Can we file a follow up JIRA to allow batching refreshPipeline call instead of individual calls to save RPC traffic.
----------------------------------------------------------------
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] xiaoyuyao commented on a change in pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r451122204
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1706,36 +1718,41 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException {
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
- volumeName, bucketName, keyName);
- OmKeyInfo fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+ volumeName, bucketName, keyName);
+ fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+
+ // Check if the key is a directory.
+ if (fileKeyInfo == null) {
+ String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
+ String dirKeyBytes = metadataManager.getOzoneKey(
+ volumeName, bucketName, dirKey);
+ OmKeyInfo dirKeyInfo = metadataManager.getKeyTable().get(dirKeyBytes);
+ if (dirKeyInfo != null) {
+ return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
+ }
+ }
+ } finally {
+ metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+ bucketName);
+
+ // if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileKeyInfo != null) {
- if (args.getRefreshPipeline()) {
+ if (refreshPipeline) {
refreshPipeline(fileKeyInfo);
}
Review comment:
should we order datanodes when the key is a file for caller like getFileStatus?
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#issuecomment-658067954
_Note_:- TestWatchForCommit#test2WayCommitForTimeoutException failure is unrelated to my changes. It is passing in my local env.
----------------------------------------------------------------
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] xiaoyuyao merged pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164
----------------------------------------------------------------
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] xiaoyuyao commented on a change in pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r451122204
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1706,36 +1718,41 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException {
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
- volumeName, bucketName, keyName);
- OmKeyInfo fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+ volumeName, bucketName, keyName);
+ fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes);
+
+ // Check if the key is a directory.
+ if (fileKeyInfo == null) {
+ String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
+ String dirKeyBytes = metadataManager.getOzoneKey(
+ volumeName, bucketName, dirKey);
+ OmKeyInfo dirKeyInfo = metadataManager.getKeyTable().get(dirKeyBytes);
+ if (dirKeyInfo != null) {
+ return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
+ }
+ }
+ } finally {
+ metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+ bucketName);
+
+ // if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileKeyInfo != null) {
- if (args.getRefreshPipeline()) {
+ if (refreshPipeline) {
refreshPipeline(fileKeyInfo);
}
Review comment:
should we order datanodes when the key is a file?
----------------------------------------------------------------
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 #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r454104041
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -1877,26 +1897,18 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
String volumeName = args.getVolumeName();
String bucketName = args.getBucketName();
String keyName = args.getKeyName();
-
- metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
- bucketName);
- try {
- OzoneFileStatus fileStatus = getFileStatus(args);
- if (fileStatus.isFile()) {
- if (args.getRefreshPipeline()) {
- refreshPipeline(fileStatus.getKeyInfo());
- }
- if (args.getSortDatanodes()) {
- sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress);
- }
- return fileStatus.getKeyInfo();
- }
+ OzoneFileStatus fileStatus = getOzoneFileStatus(volumeName, bucketName,
+ keyName, false);
//if key is not of type file or if key is not found we throw an exception
- } finally {
- metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
- bucketName);
+ if (fileStatus != null && fileStatus.isFile()) {
+ if (args.getRefreshPipeline()) {
+ refreshPipeline(fileStatus.getKeyInfo());
Review comment:
Done
----------------------------------------------------------------
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] bharatviswa504 commented on a change in pull request #1164: HDDS-3824: OM read requests should make SCM#refreshPipeline outside BUCKET_LOCK
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1164:
URL: https://github.com/apache/hadoop-ozone/pull/1164#discussion_r451209121
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -659,15 +660,6 @@ public OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress)
});
}
}
- // Refresh container pipeline info from SCM
Review comment:
Can we also move the generation of the secret token also outside of the lock?
----------------------------------------------------------------
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