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 2021/07/19 08:03:39 UTC
[GitHub] [ozone] bharatviswa504 opened a new pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
bharatviswa504 opened a new pull request #2431:
URL: https://github.com/apache/ozone/pull/2431
## What changes were proposed in this pull request?
Avoid refresh pipeline for headObject API
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5450
## How was this patch tested?
Added a test for new API. S3 test suite should test usage of headObject from s3 interface.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672486208
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1450,4 +1450,27 @@ public OzoneManagerProtocol getOzoneManagerClient() {
public Cache<URI, KeyProvider> getKeyProviderCache() {
return keyProviderCache;
}
+
+ @Override
+ public OzoneKey headObject(String volumeName, String bucketName,
+ String keyName) throws IOException {
+ Preconditions.checkNotNull(volumeName);
+ Preconditions.checkNotNull(bucketName);
+ Preconditions.checkNotNull(keyName);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setRefreshPipeline(false)
+ .setSortDatanodesInPipeline(false)
Review comment:
This should be a no-op as we skip it in OM if `headOp` flag is set, we can skip it here?
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
```suggestion
/**
*
* Returns ObjectKey that contains the application generated/visible metadata for an Ozone Object.
*
* If Key exists, returns ObjectKey.
* If Key does not exist, throws an exception with error code KEY_NOT_FOUND
*
* @param key
* @return OzoneKey which gives basic information about the key.
* @throws IOException
*/
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -736,4 +736,20 @@ OzoneOutputStream createFile(String volumeName, String bucketName,
*/
void setBucketQuota(String volumeName, String bucketName,
long quotaInNamespace, long quotaInBytes) throws IOException;
+
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param volumeName
+ * @param bucketName
+ * @param keyName
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
```suggestion
/**
*
* Returns ObjectKey that contains the application generate/visible metadata for an Object
*
* If Key exists, return basic information about the key.
* If Key does not exist, throws an exception with error code KEY_NOT_FOUND
*
* @param volumeName
* @param bucketName
* @param keyName
* @return OzoneKey which gives basic information about the key.
* @throws IOException
*/
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
Rename to `getOzoneKeyMetadata`?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r673004290
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -389,9 +389,15 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
.setRefreshPipeline(true)
.setSortDatanodesInPipeline(keyArgs.getSortDatanodes())
.setLatestVersionLocation(keyArgs.getLatestVersionLocation())
+ .setHeadOp(keyArgs.getHeadOp())
.build();
OmKeyInfo keyInfo = impl.lookupKey(omKeyArgs);
- resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+
+ if (!omKeyArgs.isHeadOp()) {
+ resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+ } else {
+ resp.setKeyInfo(keyInfo.getProtobuf(true, clientVersion));
+ }
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672449499
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -389,9 +389,15 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
.setRefreshPipeline(true)
.setSortDatanodesInPipeline(keyArgs.getSortDatanodes())
.setLatestVersionLocation(keyArgs.getLatestVersionLocation())
+ .setHeadOp(keyArgs.getHeadOp())
.build();
OmKeyInfo keyInfo = impl.lookupKey(omKeyArgs);
- resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+
+ if (!omKeyArgs.isHeadOp()) {
+ resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+ } else {
+ resp.setKeyInfo(keyInfo.getProtobuf(true, clientVersion));
+ }
Review comment:
This can be simplified:
```suggestion
resp.setKeyInfo(keyInfo.getProtobuf(omKeyArgs.isHeadOp(), clientVersion));
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#issuecomment-883286346
Thank You @adoroszlai and @kerneltime for the review.
Fixed review 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672486208
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1450,4 +1450,27 @@ public OzoneManagerProtocol getOzoneManagerClient() {
public Cache<URI, KeyProvider> getKeyProviderCache() {
return keyProviderCache;
}
+
+ @Override
+ public OzoneKey headObject(String volumeName, String bucketName,
+ String keyName) throws IOException {
+ Preconditions.checkNotNull(volumeName);
+ Preconditions.checkNotNull(bucketName);
+ Preconditions.checkNotNull(keyName);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setRefreshPipeline(false)
+ .setSortDatanodesInPipeline(false)
Review comment:
This should be a no-op as we skip it in OM if `headOp` flag is set, we can skip it here?
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
```suggestion
/**
*
* Returns ObjectKey that contains the application generated/visible metadata for an Ozone Object.
*
* If Key exists, returns ObjectKey.
* If Key does not exist, throws an exception with error code KEY_NOT_FOUND
*
* @param key
* @return OzoneKey which gives basic information about the key.
* @throws IOException
*/
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -736,4 +736,20 @@ OzoneOutputStream createFile(String volumeName, String bucketName,
*/
void setBucketQuota(String volumeName, String bucketName,
long quotaInNamespace, long quotaInBytes) throws IOException;
+
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param volumeName
+ * @param bucketName
+ * @param keyName
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
```suggestion
/**
*
* Returns ObjectKey that contains the application generate/visible metadata for an Object
*
* If Key exists, return basic information about the key.
* If Key does not exist, throws an exception with error code KEY_NOT_FOUND
*
* @param volumeName
* @param bucketName
* @param keyName
* @return OzoneKey which gives basic information about the key.
* @throws IOException
*/
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
Rename to `getOzoneKeyMetadata`?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672449499
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -389,9 +389,15 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
.setRefreshPipeline(true)
.setSortDatanodesInPipeline(keyArgs.getSortDatanodes())
.setLatestVersionLocation(keyArgs.getLatestVersionLocation())
+ .setHeadOp(keyArgs.getHeadOp())
.build();
OmKeyInfo keyInfo = impl.lookupKey(omKeyArgs);
- resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+
+ if (!omKeyArgs.isHeadOp()) {
+ resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+ } else {
+ resp.setKeyInfo(keyInfo.getProtobuf(true, clientVersion));
+ }
Review comment:
This can be simplified:
```suggestion
resp.setKeyInfo(keyInfo.getProtobuf(omKeyArgs.isHeadOp(), clientVersion));
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672998052
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
There is getKey which does the same. To be clear, we can leave it as headObject. (As main use of this API is for S3 headObject)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r674639957
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -386,12 +386,13 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
.setVolumeName(keyArgs.getVolumeName())
.setBucketName(keyArgs.getBucketName())
.setKeyName(keyArgs.getKeyName())
- .setRefreshPipeline(true)
- .setSortDatanodesInPipeline(keyArgs.getSortDatanodes())
Review comment:
Why is "sort datanodes" flag no longer set?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r673335915
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
ok
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #2431: HDDS-5450. Avoid refresh pipeline for S3 headObject
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#issuecomment-887609903
Thanks @bharatviswa504 for the improvement and @kerneltime for the review.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r673003862
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1450,4 +1450,27 @@ public OzoneManagerProtocol getOzoneManagerClient() {
public Cache<URI, KeyProvider> getKeyProviderCache() {
return keyProviderCache;
}
+
+ @Override
+ public OzoneKey headObject(String volumeName, String bucketName,
+ String keyName) throws IOException {
+ Preconditions.checkNotNull(volumeName);
+ Preconditions.checkNotNull(bucketName);
+ Preconditions.checkNotNull(keyName);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setRefreshPipeline(false)
+ .setSortDatanodesInPipeline(false)
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672998052
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
There is OzoneKey which does the same. To be clear, we can leave it as headObject. (As main use of this API is for S3 headObject)
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1450,4 +1450,27 @@ public OzoneManagerProtocol getOzoneManagerClient() {
public Cache<URI, KeyProvider> getKeyProviderCache() {
return keyProviderCache;
}
+
+ @Override
+ public OzoneKey headObject(String volumeName, String bucketName,
+ String keyName) throws IOException {
+ Preconditions.checkNotNull(volumeName);
+ Preconditions.checkNotNull(bucketName);
+ Preconditions.checkNotNull(keyName);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setRefreshPipeline(false)
+ .setSortDatanodesInPipeline(false)
Review comment:
Done
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -736,4 +736,20 @@ OzoneOutputStream createFile(String volumeName, String bucketName,
*/
void setBucketQuota(String volumeName, String bucketName,
long quotaInNamespace, long quotaInBytes) throws IOException;
+
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param volumeName
+ * @param bucketName
+ * @param keyName
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
I believe ObjectKey is OzoneKey. Rest updated as suggested
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -389,9 +389,15 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
.setRefreshPipeline(true)
.setSortDatanodesInPipeline(keyArgs.getSortDatanodes())
.setLatestVersionLocation(keyArgs.getLatestVersionLocation())
+ .setHeadOp(keyArgs.getHeadOp())
.build();
OmKeyInfo keyInfo = impl.lookupKey(omKeyArgs);
- resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+
+ if (!omKeyArgs.isHeadOp()) {
+ resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+ } else {
+ resp.setKeyInfo(keyInfo.getProtobuf(true, clientVersion));
+ }
Review comment:
Done
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1450,4 +1450,27 @@ public OzoneManagerProtocol getOzoneManagerClient() {
public Cache<URI, KeyProvider> getKeyProviderCache() {
return keyProviderCache;
}
+
+ @Override
+ public OzoneKey headObject(String volumeName, String bucketName,
+ String keyName) throws IOException {
+ Preconditions.checkNotNull(volumeName);
+ Preconditions.checkNotNull(bucketName);
+ Preconditions.checkNotNull(keyName);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setRefreshPipeline(false)
+ .setSortDatanodesInPipeline(false)
Review comment:
Done
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
There is getKey which does the same. To be clear, we can leave it as headObject. (As main use of this API is for S3 headObject)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r673004949
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1450,4 +1450,27 @@ public OzoneManagerProtocol getOzoneManagerClient() {
public Cache<URI, KeyProvider> getKeyProviderCache() {
return keyProviderCache;
}
+
+ @Override
+ public OzoneKey headObject(String volumeName, String bucketName,
+ String keyName) throws IOException {
+ Preconditions.checkNotNull(volumeName);
+ Preconditions.checkNotNull(bucketName);
+ Preconditions.checkNotNull(keyName);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setRefreshPipeline(false)
+ .setSortDatanodesInPipeline(false)
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#issuecomment-883286346
Thank You @adoroszlai and @kerneltime for the review.
Fixed review 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672449499
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -389,9 +389,15 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
.setRefreshPipeline(true)
.setSortDatanodesInPipeline(keyArgs.getSortDatanodes())
.setLatestVersionLocation(keyArgs.getLatestVersionLocation())
+ .setHeadOp(keyArgs.getHeadOp())
.build();
OmKeyInfo keyInfo = impl.lookupKey(omKeyArgs);
- resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+
+ if (!omKeyArgs.isHeadOp()) {
+ resp.setKeyInfo(keyInfo.getProtobuf(false, clientVersion));
+ } else {
+ resp.setKeyInfo(keyInfo.getProtobuf(true, clientVersion));
+ }
Review comment:
This can be simplified:
```suggestion
resp.setKeyInfo(keyInfo.getProtobuf(omKeyArgs.isHeadOp(), clientVersion));
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r677179068
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
##########
@@ -386,12 +386,13 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request,
.setVolumeName(keyArgs.getVolumeName())
.setBucketName(keyArgs.getBucketName())
.setKeyName(keyArgs.getKeyName())
- .setRefreshPipeline(true)
- .setSortDatanodesInPipeline(keyArgs.getSortDatanodes())
Review comment:
It was a mistake, thanks for catching
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672998052
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
There is OzoneKey which does the same. To be clear, we can leave it as headObject. (As main use of this API is for S3 headObject)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r672486208
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -1450,4 +1450,27 @@ public OzoneManagerProtocol getOzoneManagerClient() {
public Cache<URI, KeyProvider> getKeyProviderCache() {
return keyProviderCache;
}
+
+ @Override
+ public OzoneKey headObject(String volumeName, String bucketName,
+ String keyName) throws IOException {
+ Preconditions.checkNotNull(volumeName);
+ Preconditions.checkNotNull(bucketName);
+ Preconditions.checkNotNull(keyName);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setRefreshPipeline(false)
+ .setSortDatanodesInPipeline(false)
Review comment:
This should be a no-op as we skip it in OM if `headOp` flag is set, we can skip it here?
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
```suggestion
/**
*
* Returns ObjectKey that contains the application generated/visible metadata for an Ozone Object.
*
* If Key exists, returns ObjectKey.
* If Key does not exist, throws an exception with error code KEY_NOT_FOUND
*
* @param key
* @return OzoneKey which gives basic information about the key.
* @throws IOException
*/
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -736,4 +736,20 @@ OzoneOutputStream createFile(String volumeName, String bucketName,
*/
void setBucketQuota(String volumeName, String bucketName,
long quotaInNamespace, long quotaInBytes) throws IOException;
+
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param volumeName
+ * @param bucketName
+ * @param keyName
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
```suggestion
/**
*
* Returns ObjectKey that contains the application generate/visible metadata for an Object
*
* If Key exists, return basic information about the key.
* If Key does not exist, throws an exception with error code KEY_NOT_FOUND
*
* @param volumeName
* @param bucketName
* @param keyName
* @return OzoneKey which gives basic information about the key.
* @throws IOException
*/
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -529,6 +529,21 @@ public OzoneKeyDetails getKey(String key) throws IOException {
return proxy.getKeyDetails(volumeName, name, key);
}
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param key
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
+ public OzoneKey headObject(String key) throws IOException {
Review comment:
Rename to `getOzoneKeyMetadata`?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #2431: HDDS-5450. Avoid refresh pipeline for S3 headObject
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #2431:
URL: https://github.com/apache/ozone/pull/2431
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2431: HDDS-5450. For S3 Head avoid refresh pipeline to improve perf.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2431:
URL: https://github.com/apache/ozone/pull/2431#discussion_r673004131
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java
##########
@@ -736,4 +736,20 @@ OzoneOutputStream createFile(String volumeName, String bucketName,
*/
void setBucketQuota(String volumeName, String bucketName,
long quotaInNamespace, long quotaInBytes) throws IOException;
+
+ /**
+ *
+ * Return basic information about the key.
+ *
+ * If Key exists, return basic information about the key.
+ * If Key does not exist, throws an exception with error code KEY_NOT_FOUND
+ *
+ * @param volumeName
+ * @param bucketName
+ * @param keyName
+ * @return OzoneKey which gives basic information about the key.
+ * @throws IOException
+ */
Review comment:
I believe ObjectKey is OzoneKey. Rest updated as suggested
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org