From 5fc6966d45f898e08e0d9303f77faa93d65e8c79 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Wed, 22 May 2024 12:22:17 +0530 Subject: [PATCH 01/12] HDDS-10572. Implement multiDelete using OMKeysDeleteRequest --- .../hadoop/ozone/client/OzoneBucket.java | 10 +++ .../ozone/client/protocol/ClientProtocol.java | 12 ++++ .../hadoop/ozone/client/rpc/RpcClient.java | 12 ++++ .../om/protocol/OzoneManagerProtocol.java | 15 +++++ ...ManagerProtocolClientSideTranslatorPB.java | 30 +++++++-- .../src/main/proto/OmClientProtocol.proto | 6 ++ .../om/request/key/OMKeysDeleteRequest.java | 15 ++++- .../key/OmKeysDeleteRequestWithFSO.java | 10 ++- .../ozone/s3/endpoint/BucketEndpoint.java | 63 ++++++++++--------- .../ozone/client/ClientProtocolStub.java | 7 +++ 10 files changed, 141 insertions(+), 39 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index 8d153a948c4e..7ae72769aca7 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -635,6 +635,16 @@ public void deleteKeys(List keyList) throws IOException { proxy.deleteKeys(volumeName, name, keyList); } + /** + * Deletes the given list of keys from the bucket. + * @param keyList List of the key name to be deleted. + * @param isQuiet flag to not throw exception if delete fails + * @throws IOException + */ + public Map deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { + return proxy.deleteKeysQuiet(volumeName, name, keyList, isQuiet); + } + /** * Rename the keyname from fromKeyName to toKeyName. * @param fromKeyName The original key name. diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 912a3138c478..613cbbd732e1 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -404,6 +404,18 @@ void deleteKeys(String volumeName, String bucketName, List keyNameList) throws IOException; + /** + * Deletes keys through the list. + * @param volumeName Name of the Volume + * @param bucketName Name of the Bucket + * @param keyNameList List of the Key + * @param isQuiet flag to not throw exception if delete fails + * @throws IOException + */ + Map deleteKeysQuiet(String volumeName, String bucketName, + List keyNameList, Boolean isQuiet) + throws IOException; + /** * Renames an existing key within a bucket. * @param volumeName Name of the Volume diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 2f97f2f3ccc6..ed11a3eac117 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -1609,6 +1609,18 @@ public void deleteKeys( ozoneManagerClient.deleteKeys(omDeleteKeys); } + @Override + public Map deleteKeysQuiet( + String volumeName, String bucketName, List keyNameList, Boolean isQuiet) + throws IOException { + verifyVolumeName(volumeName); + verifyBucketName(bucketName); + Preconditions.checkNotNull(keyNameList); + OmDeleteKeys omDeleteKeys = new OmDeleteKeys(volumeName, bucketName, + keyNameList); + return ozoneManagerClient.deleteKeysQuiet(omDeleteKeys, isQuiet); + } + @Override public void renameKey(String volumeName, String bucketName, String fromKeyName, String toKeyName) throws IOException { diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index ab3f576d4492..1191626cab56 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -21,6 +21,7 @@ import java.io.Closeable; import java.io.IOException; import java.util.List; +import java.util.Map; import java.util.UUID; import jakarta.annotation.Nonnull; @@ -359,6 +360,20 @@ default void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { "this to be implemented, as write requests use a new approach."); } + /** + * Deletes existing key/keys. This interface supports delete + * multiple keys and a single key. Used by deleting files + * through OzoneFileSystem. + * + * @param deleteKeys + * @param isQuiet - flag to not throw exception if delete fails + * @throws IOException + */ + default Map deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) throws IOException { + throw new UnsupportedOperationException("OzoneManager does not require " + + "this to be implemented, as write requests use a new approach."); + } + /** * Deletes an existing empty bucket from volume. diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 61dd3f56602e..ac7f18610bd4 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -19,10 +19,7 @@ import java.io.IOException; import java.time.Instant; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.UUID; +import java.util.*; import java.util.stream.Collectors; import com.google.protobuf.Proto2Utils; @@ -949,6 +946,18 @@ public void deleteKey(OmKeyArgs args) throws IOException { */ @Override public void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { + deleteKeysQuiet(deleteKeys, false); + } + + /** + * Deletes existing key/keys. This interface supports delete + * multiple keys and a single key. + * + * @param deleteKeys + * @throws IOException + */ + @Override + public Map deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) throws IOException { DeleteKeysRequest.Builder req = DeleteKeysRequest.newBuilder(); DeleteKeyArgs deletedKeys = DeleteKeyArgs.newBuilder() .setBucketName(deleteKeys.getBucket()) @@ -958,9 +967,18 @@ public void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { OMRequest omRequest = createOMRequest(Type.DeleteKeys) .setDeleteKeysRequest(req) .build(); + OMResponse omResponse = submitRequest(omRequest); - handleError(submitRequest(omRequest)); - + if (!isQuiet) { + handleError(omResponse); + } + List deleteKeyErrors = + omResponse.getDeleteKeysResponse().getDeleteKeyErrorsList(); + Map keyToErrors = new HashMap<>(); + for (OzoneManagerProtocolProtos.DeleteKeyErrors deleteKeyError : deleteKeyErrors) { + keyToErrors.put(deleteKeyError.getKey(), deleteKeyError.getError()); + } + return keyToErrors; } /** diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 4a18f308c90f..8a461b95db68 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1297,9 +1297,15 @@ message DeleteKeyArgs { repeated string keys = 3; } +message DeleteKeyErrors { + required string key = 1; + required string error = 2; +} + message DeleteKeysResponse { optional DeleteKeyArgs unDeletedKeys = 1; optional bool status = 2; + repeated DeleteKeyErrors deleteKeyErrors = 3; } message DeleteKeyResponse { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java index be89da369cdb..a419e7ee7bca 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java @@ -57,6 +57,7 @@ import java.io.IOException; import java.nio.file.InvalidPathException; import java.util.ArrayList; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -95,6 +96,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn Exception exception = null; OMClientResponse omClientResponse = null; Result result = null; + Map keyToError = new HashMap<>(); OMMetrics omMetrics = ozoneManager.getMetrics(); omMetrics.incNumKeyDeletes(); @@ -150,6 +152,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn objectKey); deleteKeys.remove(keyName); unDeletedKeys.addKeys(keyName); + keyToError.put(keyName, "Key not found"); continue; } @@ -167,6 +170,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn LOG.error("Acl check failed for Key: {}", objectKey, ex); deleteKeys.remove(keyName); unDeletedKeys.addKeys(keyName); + keyToError.put(keyName, "Access denied"); } } @@ -185,7 +189,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn final long volumeId = omMetadataManager.getVolumeId(volumeName); omClientResponse = getOmClientResponse(ozoneManager, omKeyInfoList, dirList, omResponse, - unDeletedKeys, deleteStatus, omBucketInfo, volumeId, dbOpenKeys); + unDeletedKeys, keyToError, deleteStatus, omBucketInfo, volumeId, dbOpenKeys); result = Result.SUCCESS; @@ -199,6 +203,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn // Add all keys which are failed due to any other exception . for (int i = indexFailed; i < length; i++) { unDeletedKeys.addKeys(deleteKeyArgs.getKeys(i)); + keyToError.put(deleteKeyArgs.getKeys(i), "Internal error"); } omResponse.setDeleteKeysResponse( @@ -260,12 +265,18 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, + Map keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; + List deleteKeyErrors = new ArrayList<>(); + for (Map.Entry key : keyToErrors.entrySet()) { + deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyErrors.newBuilder() + .setKey(key.getKey()).setError(key.getValue()).build()); + } omClientResponse = new OMKeysDeleteResponse(omResponse .setDeleteKeysResponse( DeleteKeysResponse.newBuilder().setStatus(deleteStatus) - .setUnDeletedKeys(unDeletedKeys)) + .setUnDeletedKeys(unDeletedKeys).addAllDeleteKeyErrors(deleteKeyErrors)) .setStatus(deleteStatus ? OK : PARTIAL_DELETE).setSuccess(deleteStatus) .build(), omKeyInfoList, ozoneManager.isRatisEnabled(), omBucketInfo.copyObject(), dbOpenKeys); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java index 255a3db9fef3..b9b41f036092 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java @@ -36,7 +36,9 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.ArrayList; import java.util.List; +import java.util.Map; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_DELETE; @@ -146,12 +148,18 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OzoneManagerProtocolProtos.OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, + Map keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; + List deleteKeyErrors = new ArrayList<>(); + for (Map.Entry key : keyToErrors.entrySet()) { + deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyErrors.newBuilder() + .setKey(key.getKey()).setError(key.getValue()).build()); + } omClientResponse = new OMKeysDeleteResponseWithFSO(omResponse .setDeleteKeysResponse( OzoneManagerProtocolProtos.DeleteKeysResponse.newBuilder() - .setStatus(deleteStatus).setUnDeletedKeys(unDeletedKeys)) + .setStatus(deleteStatus).setUnDeletedKeys(unDeletedKeys).addAllDeleteKeyErrors(deleteKeyErrors)) .setStatus(deleteStatus ? OK : PARTIAL_DELETE).setSuccess(deleteStatus) .build(), omKeyInfoList, dirList, ozoneManager.isRatisEnabled(), omBucketInfo.copyObject(), volumeId, dbOpenKeys); diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index ec434e4bb566..cd36e124a119 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -67,6 +67,8 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.HashMap; import java.util.Set; import static org.apache.hadoop.ozone.OzoneConsts.ETAG; @@ -278,6 +280,7 @@ public Response get( getMetrics().incListKeyCount(keyCount); perf.appendCount(keyCount); perf.appendOpLatencyNanos(opLatencyNs); + LOG.info("tej S3G get op: " + response.getKeyCount() + " " + bucketName + " -" + prefix + "- " + startAfter + "--"); AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction, getAuditParameters(), perf)); response.setKeyCount(keyCount); @@ -445,47 +448,47 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, OzoneBucket bucket = getBucket(bucketName); MultiDeleteResponse result = new MultiDeleteResponse(); + Map undeletedKeyResultMap = new HashMap<>(); + if (request.getObjects() != null) { + List deleteKeys = new ArrayList<>(); for (DeleteObject keyToDelete : request.getObjects()) { - long startNanos = Time.monotonicNowNanos(); - try { - bucket.deleteKey(keyToDelete.getKey()); - getMetrics().updateDeleteKeySuccessStats(startNanos); - - if (!request.isQuiet()) { - result.addDeleted(new DeletedObject(keyToDelete.getKey())); - } - } catch (OMException ex) { - if (isAccessDenied(ex)) { - getMetrics().updateDeleteKeyFailureStats(startNanos); - result.addError( - new Error(keyToDelete.getKey(), "PermissionDenied", - ex.getMessage())); - } else if (ex.getResult() != ResultCodes.KEY_NOT_FOUND) { - getMetrics().updateDeleteKeyFailureStats(startNanos); - result.addError( - new Error(keyToDelete.getKey(), "InternalError", - ex.getMessage())); + deleteKeys.add(keyToDelete.getKey()); + } + long startNanos = Time.monotonicNowNanos(); + try { + undeletedKeyResultMap = bucket.deleteKeysQuiet(deleteKeys, true); + for (DeleteObject d : request.getObjects()) { + if (!(undeletedKeyResultMap.containsKey(d.getKey())) || + undeletedKeyResultMap.get(d.getKey()).equals("Key not found")) { + result.addDeleted(new DeletedObject(d.getKey())); } else { - if (!request.isQuiet()) { - result.addDeleted(new DeletedObject(keyToDelete.getKey())); - } - getMetrics().updateDeleteKeySuccessStats(startNanos); + String error = undeletedKeyResultMap.get(d.getKey()); + result.addError(new Error(d.getKey(), error, error)); } - } catch (Exception ex) { - getMetrics().updateDeleteKeyFailureStats(startNanos); - result.addError( - new Error(keyToDelete.getKey(), "InternalError", - ex.getMessage())); } + getMetrics().updateDeleteKeySuccessStats(startNanos); + } catch (IOException ex) { + LOG.error("tej delete key failed: {}", ex.getMessage()); + getMetrics().updateDeleteKeyFailureStats(startNanos); + result.addError( + new Error("ALL", "InternalError", + ex.getMessage())); } } + + Map auditMap = getAuditParameters(); + List keys = new ArrayList<>(); + for (DeleteObject d : request.getObjects()) { + keys.add(d.getKey()); + } + auditMap.put("Keys", keys.toString()); if (result.getErrors().size() != 0) { AUDIT.logWriteFailure(buildAuditMessageForFailure(s3GAction, - getAuditParameters(), new Exception("MultiDelete Exception"))); + auditMap, new Exception("MultiDelete Exception"))); } else { AUDIT.logWriteSuccess( - buildAuditMessageForSuccess(s3GAction, getAuditParameters())); + buildAuditMessageForSuccess(s3GAction, auditMap)); } return result; } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java index bc562d5d9363..82a6ca4b7ec4 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java @@ -57,6 +57,7 @@ import java.io.IOException; import java.net.URI; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -250,6 +251,12 @@ public void deleteKeys(String volumeName, String bucketName, } + @Override + public Map deleteKeysQuiet(String volumeName, String bucketName, + List keyNameList, Boolean isQuiet) throws IOException { + return new HashMap<>(); + } + @Override public void renameKey(String volumeName, String bucketName, String fromKeyName, String toKeyName) From bc3e6b9088b721f691c3af492a6e5420cf723280 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Wed, 22 May 2024 12:51:11 +0530 Subject: [PATCH 02/12] Remove unnecessary logs --- .../OzoneManagerProtocolClientSideTranslatorPB.java | 6 +++++- .../hadoop/ozone/s3/endpoint/BucketEndpoint.java | 10 ++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 8bdc790dbd5b..9c54eab7c5c7 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -19,7 +19,11 @@ import java.io.IOException; import java.time.Instant; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; import java.util.stream.Collectors; import com.google.protobuf.Proto2Utils; diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index cd36e124a119..2cfa4e2d3394 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -68,7 +68,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.HashMap; import java.util.Set; import static org.apache.hadoop.ozone.OzoneConsts.ETAG; @@ -280,7 +279,6 @@ public Response get( getMetrics().incListKeyCount(keyCount); perf.appendCount(keyCount); perf.appendOpLatencyNanos(opLatencyNs); - LOG.info("tej S3G get op: " + response.getKeyCount() + " " + bucketName + " -" + prefix + "- " + startAfter + "--"); AUDIT.logReadSuccess(buildAuditMessageForSuccess(s3GAction, getAuditParameters(), perf)); response.setKeyCount(keyCount); @@ -448,7 +446,7 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, OzoneBucket bucket = getBucket(bucketName); MultiDeleteResponse result = new MultiDeleteResponse(); - Map undeletedKeyResultMap = new HashMap<>(); + Map undeletedKeyResultMap; if (request.getObjects() != null) { List deleteKeys = new ArrayList<>(); @@ -459,8 +457,8 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, try { undeletedKeyResultMap = bucket.deleteKeysQuiet(deleteKeys, true); for (DeleteObject d : request.getObjects()) { - if (!(undeletedKeyResultMap.containsKey(d.getKey())) || - undeletedKeyResultMap.get(d.getKey()).equals("Key not found")) { + if (!request.isQuiet() && (!(undeletedKeyResultMap.containsKey(d.getKey())) || + undeletedKeyResultMap.get(d.getKey()).equals("Key not found"))) { result.addDeleted(new DeletedObject(d.getKey())); } else { String error = undeletedKeyResultMap.get(d.getKey()); @@ -469,7 +467,7 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, } getMetrics().updateDeleteKeySuccessStats(startNanos); } catch (IOException ex) { - LOG.error("tej delete key failed: {}", ex.getMessage()); + LOG.error("Delete key failed: {}", ex.getMessage()); getMetrics().updateDeleteKeyFailureStats(startNanos); result.addError( new Error("ALL", "InternalError", From c6a02ffc389e2de4392058ca424709ca20f75e23 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Thu, 30 May 2024 11:21:24 +0530 Subject: [PATCH 03/12] Fix unit test and add additional checks --- .../ozone/om/request/key/TestOMKeysDeleteRequest.java | 7 +++++++ .../hadoop/ozone/s3/endpoint/TestPermissionCheck.java | 8 ++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java index d0cfd48e35dc..85c12a9ffccc 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java @@ -23,6 +23,7 @@ import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyErrors; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeysRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.junit.jupiter.api.Test; @@ -73,6 +74,9 @@ protected void checkDeleteKeysResponse( omClientResponse.getOMResponse().getDeleteKeysResponse() .getUnDeletedKeys(); assertEquals(0, unDeletedKeys.getKeysCount()); + List keyErrors = omClientResponse.getOMResponse().getDeleteKeysResponse() + .getDeleteKeyErrorsList(); + assertEquals(0, keyErrors.size()); // Check all keys are deleted. for (String deleteKey : deleteKeyList) { @@ -123,6 +127,9 @@ protected void checkDeleteKeysResponseForFailure( .getDeleteKeysResponse().getUnDeletedKeys(); assertEquals(1, unDeletedKeys.getKeysCount()); + List keyErrors = omClientResponse.getOMResponse().getDeleteKeysResponse() + .getDeleteKeyErrorsList(); + assertEquals(1, keyErrors.size()); assertEquals("dummy", unDeletedKeys.getKeys(0)); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java index 04551ac7cc43..6f41e8abb2eb 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java @@ -38,6 +38,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -167,7 +168,10 @@ public void testListKey() throws IOException { public void testDeleteKeys() throws IOException, OS3Exception { when(objectStore.getVolume(anyString())).thenReturn(volume); when(objectStore.getS3Bucket(anyString())).thenReturn(bucket); - doThrow(exception).when(bucket).deleteKey(any()); + Map deleteErrors = new HashMap<>(); + deleteErrors.put("deleteKeyName", "Access denied"); + when(bucket.deleteKeysQuiet(any(), anyBoolean())).thenReturn(deleteErrors); + BucketEndpoint bucketEndpoint = new BucketEndpoint(); bucketEndpoint.setClient(client); MultiDeleteRequest request = new MultiDeleteRequest(); @@ -179,7 +183,7 @@ public void testDeleteKeys() throws IOException, OS3Exception { MultiDeleteResponse response = bucketEndpoint.multiDelete("BucketName", "keyName", request); assertEquals(1, response.getErrors().size()); - assertEquals("PermissionDenied", response.getErrors().get(0).getCode()); + assertEquals("Access denied", response.getErrors().get(0).getCode()); } @Test From 1dac2e6d00dbcab53ab87cebe075475b6819f0d9 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Thu, 30 May 2024 16:41:52 +0530 Subject: [PATCH 04/12] Fix for TestObjectMultiDelete --- .../hadoop/ozone/s3/endpoint/BucketEndpoint.java | 3 ++- .../apache/hadoop/ozone/client/OzoneBucketStub.java | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index 2cfa4e2d3394..cac317fc19b9 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -460,7 +460,8 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, if (!request.isQuiet() && (!(undeletedKeyResultMap.containsKey(d.getKey())) || undeletedKeyResultMap.get(d.getKey()).equals("Key not found"))) { result.addDeleted(new DeletedObject(d.getKey())); - } else { + } else if (undeletedKeyResultMap.containsKey(d.getKey()) && + !undeletedKeyResultMap.get(d.getKey()).equals("Key not found")) { String error = undeletedKeyResultMap.get(d.getKey()); result.addError(new Error(d.getKey(), error, error)); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java index 22b002945eb9..9d03bec25143 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java @@ -358,6 +358,18 @@ public void deleteKey(String key) throws IOException { keyDetails.remove(key); } + @Override + public Map deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { + Map keyErrorMap = new HashMap<>(); + for (String key : keyList) { + if (!keyDetails.containsKey(key)) { + keyErrorMap.put(key, "Key not found"); + } + keyDetails.remove(key); + } + return keyErrorMap; + } + @Override public void renameKey(String fromKeyName, String toKeyName) throws IOException { From 5ad4b1331886c20c5dd16cf7a5db6e81ab849937 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Fri, 31 May 2024 13:15:09 +0530 Subject: [PATCH 05/12] Add errorCode and errorMessage to response --- .../apache/hadoop/ozone/client/OzoneBucket.java | 2 +- .../ozone/client/protocol/ClientProtocol.java | 5 +++-- .../apache/hadoop/ozone/client/rpc/RpcClient.java | 3 ++- .../ozone/om/protocol/OzoneManagerProtocol.java | 4 +++- ...zoneManagerProtocolClientSideTranslatorPB.java | 15 +++++---------- .../src/main/proto/OmClientProtocol.proto | 3 ++- .../ozone/om/request/key/OMKeysDeleteRequest.java | 15 ++++++++------- .../request/key/OmKeysDeleteRequestWithFSO.java | 7 ++++--- .../hadoop/ozone/s3/endpoint/BucketEndpoint.java | 11 ++++++----- .../hadoop/ozone/client/ClientProtocolStub.java | 6 ++++-- .../hadoop/ozone/client/OzoneBucketStub.java | 7 ++++--- .../ozone/s3/endpoint/TestPermissionCheck.java | 7 ++++--- 12 files changed, 46 insertions(+), 39 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index 010eba9bad85..4e3d190439b3 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -681,7 +681,7 @@ public void deleteKeys(List keyList) throws IOException { * @param isQuiet flag to not throw exception if delete fails * @throws IOException */ - public Map deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { + public Map> deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { return proxy.deleteKeysQuiet(volumeName, name, keyList, isQuiet); } diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 1cda723ca5f8..fdc60714f9ce 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -24,6 +24,7 @@ import java.util.Map; import jakarta.annotation.Nonnull; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationFactor; @@ -444,8 +445,8 @@ void deleteKeys(String volumeName, String bucketName, * @param isQuiet flag to not throw exception if delete fails * @throws IOException */ - Map deleteKeysQuiet(String volumeName, String bucketName, - List keyNameList, Boolean isQuiet) + Map> deleteKeysQuiet(String volumeName, String bucketName, + List keyNameList, Boolean isQuiet) throws IOException; /** diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 8a7c15d0a7f0..cb623a741a8c 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -29,6 +29,7 @@ import javax.crypto.Cipher; import javax.crypto.CipherInputStream; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.crypto.CryptoInputStream; import org.apache.hadoop.crypto.CryptoOutputStream; import org.apache.hadoop.crypto.key.KeyProvider; @@ -1643,7 +1644,7 @@ public void deleteKeys( } @Override - public Map deleteKeysQuiet( + public Map> deleteKeysQuiet( String volumeName, String bucketName, List keyNameList, Boolean isQuiet) throws IOException { verifyVolumeName(volumeName); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index 1191626cab56..dfee94ec739b 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -25,6 +25,7 @@ import java.util.UUID; import jakarta.annotation.Nonnull; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.fs.SafeModeAction; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.ozone.OzoneAcl; @@ -369,7 +370,8 @@ default void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { * @param isQuiet - flag to not throw exception if delete fails * @throws IOException */ - default Map deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) throws IOException { + default Map> deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) + throws IOException { throw new UnsupportedOperationException("OzoneManager does not require " + "this to be implemented, as write requests use a new approach."); } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 9c54eab7c5c7..106b8cc13e98 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -29,6 +29,7 @@ import com.google.protobuf.Proto2Utils; import jakarta.annotation.Nonnull; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.fs.SafeModeAction; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.client.ECReplicationConfig; @@ -957,15 +958,9 @@ public void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { deleteKeysQuiet(deleteKeys, false); } - /** - * Deletes existing key/keys. This interface supports delete - * multiple keys and a single key. - * - * @param deleteKeys - * @throws IOException - */ @Override - public Map deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) throws IOException { + public Map> deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) + throws IOException { DeleteKeysRequest.Builder req = DeleteKeysRequest.newBuilder(); DeleteKeyArgs deletedKeys = DeleteKeyArgs.newBuilder() .setBucketName(deleteKeys.getBucket()) @@ -982,9 +977,9 @@ public Map deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQu } List deleteKeyErrors = omResponse.getDeleteKeysResponse().getDeleteKeyErrorsList(); - Map keyToErrors = new HashMap<>(); + Map> keyToErrors = new HashMap<>(); for (OzoneManagerProtocolProtos.DeleteKeyErrors deleteKeyError : deleteKeyErrors) { - keyToErrors.put(deleteKeyError.getKey(), deleteKeyError.getError()); + keyToErrors.put(deleteKeyError.getKey(), Pair.of(deleteKeyError.getErrorCode(), deleteKeyError.getErrorMsg())); } return keyToErrors; } diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index b84b0b241e65..85e205d08593 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1303,7 +1303,8 @@ message DeleteKeyArgs { message DeleteKeyErrors { required string key = 1; - required string error = 2; + required string errorCode = 2; + required string errorMsg = 3; } message DeleteKeysResponse { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java index a419e7ee7bca..f62d1f62ed45 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java @@ -96,7 +96,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn Exception exception = null; OMClientResponse omClientResponse = null; Result result = null; - Map keyToError = new HashMap<>(); + Map> keyToError = new HashMap<>(); OMMetrics omMetrics = ozoneManager.getMetrics(); omMetrics.incNumKeyDeletes(); @@ -152,7 +152,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn objectKey); deleteKeys.remove(keyName); unDeletedKeys.addKeys(keyName); - keyToError.put(keyName, "Key not found"); + keyToError.put(keyName, Pair.of(OMException.ResultCodes.KEY_NOT_FOUND.name(), "Key does not exist")); continue; } @@ -170,7 +170,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn LOG.error("Acl check failed for Key: {}", objectKey, ex); deleteKeys.remove(keyName); unDeletedKeys.addKeys(keyName); - keyToError.put(keyName, "Access denied"); + keyToError.put(keyName, Pair.of(OMException.ResultCodes.ACCESS_DENIED.name(), "ACL check failed")); } } @@ -203,7 +203,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn // Add all keys which are failed due to any other exception . for (int i = indexFailed; i < length; i++) { unDeletedKeys.addKeys(deleteKeyArgs.getKeys(i)); - keyToError.put(deleteKeyArgs.getKeys(i), "Internal error"); + keyToError.put(deleteKeyArgs.getKeys(i), Pair.of(OMException.ResultCodes.INTERNAL_ERROR.name(), + ex.getMessage())); } omResponse.setDeleteKeysResponse( @@ -265,13 +266,13 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, - Map keyToErrors, + Map> keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; List deleteKeyErrors = new ArrayList<>(); - for (Map.Entry key : keyToErrors.entrySet()) { + for (Map.Entry> key : keyToErrors.entrySet()) { deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyErrors.newBuilder() - .setKey(key.getKey()).setError(key.getValue()).build()); + .setKey(key.getKey()).setErrorCode(key.getValue().getLeft()).setErrorMsg(key.getValue().getRight()).build()); } omClientResponse = new OMKeysDeleteResponse(omResponse .setDeleteKeysResponse( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java index b9b41f036092..789aae79ca50 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.ozone.om.request.key; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; @@ -148,13 +149,13 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OzoneManagerProtocolProtos.OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, - Map keyToErrors, + Map> keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; List deleteKeyErrors = new ArrayList<>(); - for (Map.Entry key : keyToErrors.entrySet()) { + for (Map.Entry> key : keyToErrors.entrySet()) { deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyErrors.newBuilder() - .setKey(key.getKey()).setError(key.getValue()).build()); + .setKey(key.getKey()).setErrorCode(key.getValue().getLeft()).setErrorMsg(key.getValue().getRight()).build()); } omClientResponse = new OMKeysDeleteResponseWithFSO(omResponse .setDeleteKeysResponse( diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index cac317fc19b9..2988c17871af 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.s3.endpoint; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.OzoneAcl; @@ -446,7 +447,7 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, OzoneBucket bucket = getBucket(bucketName); MultiDeleteResponse result = new MultiDeleteResponse(); - Map undeletedKeyResultMap; + Map> undeletedKeyResultMap; if (request.getObjects() != null) { List deleteKeys = new ArrayList<>(); @@ -458,12 +459,12 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, undeletedKeyResultMap = bucket.deleteKeysQuiet(deleteKeys, true); for (DeleteObject d : request.getObjects()) { if (!request.isQuiet() && (!(undeletedKeyResultMap.containsKey(d.getKey())) || - undeletedKeyResultMap.get(d.getKey()).equals("Key not found"))) { + undeletedKeyResultMap.get(d.getKey()).getLeft().equals(ResultCodes.KEY_NOT_FOUND.name()))) { result.addDeleted(new DeletedObject(d.getKey())); } else if (undeletedKeyResultMap.containsKey(d.getKey()) && - !undeletedKeyResultMap.get(d.getKey()).equals("Key not found")) { - String error = undeletedKeyResultMap.get(d.getKey()); - result.addError(new Error(d.getKey(), error, error)); + !undeletedKeyResultMap.get(d.getKey()).getLeft().equals(ResultCodes.KEY_NOT_FOUND.name())) { + Pair error = undeletedKeyResultMap.get(d.getKey()); + result.addError(new Error(d.getKey(), error.getLeft(), error.getRight())); } } getMetrics().updateDeleteKeySuccessStats(startNanos); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java index c7eb494b21fb..b95fc4ddff5f 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java @@ -20,6 +20,7 @@ package org.apache.hadoop.ozone.client; import jakarta.annotation.Nonnull; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationFactor; @@ -262,8 +263,9 @@ public void deleteKeys(String volumeName, String bucketName, } @Override - public Map deleteKeysQuiet(String volumeName, String bucketName, - List keyNameList, Boolean isQuiet) throws IOException { + public Map> deleteKeysQuiet(String volumeName, String bucketName, + List keyNameList, Boolean isQuiet) + throws IOException { return new HashMap<>(); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java index 9d03bec25143..f917054ee3ad 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java @@ -38,6 +38,7 @@ import javax.xml.bind.DatatypeConverter; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationFactor; @@ -359,11 +360,11 @@ public void deleteKey(String key) throws IOException { } @Override - public Map deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { - Map keyErrorMap = new HashMap<>(); + public Map> deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { + Map> keyErrorMap = new HashMap<>(); for (String key : keyList) { if (!keyDetails.containsKey(key)) { - keyErrorMap.put(key, "Key not found"); + keyErrorMap.put(key,Pair.of("KEY_NOT_FOUND", "Key does not exist")); } keyDetails.remove(key); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java index 6f41e8abb2eb..22fb90ff1618 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java @@ -19,6 +19,7 @@ */ package org.apache.hadoop.ozone.s3.endpoint; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.client.ObjectStore; @@ -168,8 +169,8 @@ public void testListKey() throws IOException { public void testDeleteKeys() throws IOException, OS3Exception { when(objectStore.getVolume(anyString())).thenReturn(volume); when(objectStore.getS3Bucket(anyString())).thenReturn(bucket); - Map deleteErrors = new HashMap<>(); - deleteErrors.put("deleteKeyName", "Access denied"); + Map> deleteErrors = new HashMap<>(); + deleteErrors.put("deleteKeyName", Pair.of("ACCESS_DENIED", "ACL check failed")); when(bucket.deleteKeysQuiet(any(), anyBoolean())).thenReturn(deleteErrors); BucketEndpoint bucketEndpoint = new BucketEndpoint(); @@ -183,7 +184,7 @@ public void testDeleteKeys() throws IOException, OS3Exception { MultiDeleteResponse response = bucketEndpoint.multiDelete("BucketName", "keyName", request); assertEquals(1, response.getErrors().size()); - assertEquals("Access denied", response.getErrors().get(0).getCode()); + assertEquals("ACCESS_DENIED", response.getErrors().get(0).getCode()); } @Test From 45ae661b3227068c585b76facfc395a2ebfb50ca Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Fri, 31 May 2024 13:31:39 +0530 Subject: [PATCH 06/12] checkstyle fix 1 --- .../java/org/apache/hadoop/ozone/client/OzoneBucketStub.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java index f917054ee3ad..ea8683935e0a 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java @@ -364,7 +364,7 @@ public Map> deleteKeysQuiet(List keyList, B Map> keyErrorMap = new HashMap<>(); for (String key : keyList) { if (!keyDetails.containsKey(key)) { - keyErrorMap.put(key,Pair.of("KEY_NOT_FOUND", "Key does not exist")); + keyErrorMap.put(key,Pair.of("KEY_NOT_FOUND", "Key does not exist")); } keyDetails.remove(key); } From 1b91ed272c8fab9a6c4d60ecc4edff03276db4e8 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Fri, 31 May 2024 13:45:15 +0530 Subject: [PATCH 07/12] checkstyle fix 2 --- .../java/org/apache/hadoop/ozone/client/OzoneBucketStub.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java index ea8683935e0a..60ddc9c573ad 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java @@ -364,7 +364,7 @@ public Map> deleteKeysQuiet(List keyList, B Map> keyErrorMap = new HashMap<>(); for (String key : keyList) { if (!keyDetails.containsKey(key)) { - keyErrorMap.put(key,Pair.of("KEY_NOT_FOUND", "Key does not exist")); + keyErrorMap.put(key, Pair.of("KEY_NOT_FOUND", "Key does not exist")); } keyDetails.remove(key); } From f6901b5da81756395ff390457ad2ad29131e47c8 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Mon, 3 Jun 2024 12:43:49 +0530 Subject: [PATCH 08/12] Enable s3aContract test, change method name --- .../org/apache/hadoop/ozone/client/OzoneBucket.java | 6 +++--- .../hadoop/ozone/client/protocol/ClientProtocol.java | 6 +++--- .../apache/hadoop/ozone/client/rpc/RpcClient.java | 6 +++--- .../ozone/om/protocol/OzoneManagerProtocol.java | 4 ++-- .../OzoneManagerProtocolClientSideTranslatorPB.java | 12 ++++++------ .../dist/src/main/compose/common/s3a-test.sh | 3 +-- .../src/main/proto/OmClientProtocol.proto | 10 +++++----- .../ozone/om/request/key/OMKeysDeleteRequest.java | 6 +++--- .../om/request/key/OmKeysDeleteRequestWithFSO.java | 6 +++--- .../om/request/key/TestOMKeysDeleteRequest.java | 10 +++++----- .../hadoop/ozone/s3/endpoint/BucketEndpoint.java | 2 +- .../hadoop/ozone/client/ClientProtocolStub.java | 4 ++-- .../apache/hadoop/ozone/client/OzoneBucketStub.java | 5 ++--- .../ozone/s3/endpoint/TestPermissionCheck.java | 2 +- 14 files changed, 40 insertions(+), 42 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index 4e3d190439b3..a39682cf1421 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -678,11 +678,11 @@ public void deleteKeys(List keyList) throws IOException { /** * Deletes the given list of keys from the bucket. * @param keyList List of the key name to be deleted. - * @param isQuiet flag to not throw exception if delete fails + * @param quiet flag to not throw exception if delete fails * @throws IOException */ - public Map> deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { - return proxy.deleteKeysQuiet(volumeName, name, keyList, isQuiet); + public Map> deleteKeys(List keyList, boolean quiet) throws IOException { + return proxy.deleteKeys(volumeName, name, keyList, quiet); } /** diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index fdc60714f9ce..26a65e561482 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -442,11 +442,11 @@ void deleteKeys(String volumeName, String bucketName, * @param volumeName Name of the Volume * @param bucketName Name of the Bucket * @param keyNameList List of the Key - * @param isQuiet flag to not throw exception if delete fails + * @param quiet flag to not throw exception if delete fails * @throws IOException */ - Map> deleteKeysQuiet(String volumeName, String bucketName, - List keyNameList, Boolean isQuiet) + Map> deleteKeys(String volumeName, String bucketName, + List keyNameList, boolean quiet) throws IOException; /** diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index cb623a741a8c..7e48d340ce31 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -1644,15 +1644,15 @@ public void deleteKeys( } @Override - public Map> deleteKeysQuiet( - String volumeName, String bucketName, List keyNameList, Boolean isQuiet) + public Map> deleteKeys( + String volumeName, String bucketName, List keyNameList, boolean quiet) throws IOException { verifyVolumeName(volumeName); verifyBucketName(bucketName); Preconditions.checkNotNull(keyNameList); OmDeleteKeys omDeleteKeys = new OmDeleteKeys(volumeName, bucketName, keyNameList); - return ozoneManagerClient.deleteKeysQuiet(omDeleteKeys, isQuiet); + return ozoneManagerClient.deleteKeys(omDeleteKeys, quiet); } @Override diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index dfee94ec739b..37e63ef806ec 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -367,10 +367,10 @@ default void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { * through OzoneFileSystem. * * @param deleteKeys - * @param isQuiet - flag to not throw exception if delete fails + * @param quiet - flag to not throw exception if delete fails * @throws IOException */ - default Map> deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) + default Map> deleteKeys(OmDeleteKeys deleteKeys, boolean quiet) throws IOException { throw new UnsupportedOperationException("OzoneManager does not require " + "this to be implemented, as write requests use a new approach."); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 106b8cc13e98..8449f906cf3d 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -955,11 +955,11 @@ public void deleteKey(OmKeyArgs args) throws IOException { */ @Override public void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { - deleteKeysQuiet(deleteKeys, false); + deleteKeys(deleteKeys, false); } @Override - public Map> deleteKeysQuiet(OmDeleteKeys deleteKeys, Boolean isQuiet) + public Map> deleteKeys(OmDeleteKeys deleteKeys, boolean quiet) throws IOException { DeleteKeysRequest.Builder req = DeleteKeysRequest.newBuilder(); DeleteKeyArgs deletedKeys = DeleteKeyArgs.newBuilder() @@ -972,13 +972,13 @@ public Map> deleteKeysQuiet(OmDeleteKeys deleteKeys .build(); OMResponse omResponse = submitRequest(omRequest); - if (!isQuiet) { + if (!quiet) { handleError(omResponse); } - List deleteKeyErrors = - omResponse.getDeleteKeysResponse().getDeleteKeyErrorsList(); + List errors = + omResponse.getDeleteKeysResponse().getErrorsList(); Map> keyToErrors = new HashMap<>(); - for (OzoneManagerProtocolProtos.DeleteKeyErrors deleteKeyError : deleteKeyErrors) { + for (OzoneManagerProtocolProtos.DeleteKeyError deleteKeyError : errors) { keyToErrors.put(deleteKeyError.getKey(), Pair.of(deleteKeyError.getErrorCode(), deleteKeyError.getErrorMsg())); } return keyToErrors; diff --git a/hadoop-ozone/dist/src/main/compose/common/s3a-test.sh b/hadoop-ozone/dist/src/main/compose/common/s3a-test.sh index 1a784b116101..554b22b5a394 100644 --- a/hadoop-ozone/dist/src/main/compose/common/s3a-test.sh +++ b/hadoop-ozone/dist/src/main/compose/common/s3a-test.sh @@ -95,10 +95,9 @@ EOF # Some tests are skipped due to known issues. # - ITestS3AContractDistCp: HDDS-10616 # - ITestS3AContractGetFileStatusV1List: HDDS-10617 - # - ITestS3AContractMkdir: HDDS-10572 # - ITestS3AContractRename: HDDS-10665 mvn -B -V --fail-never --no-transfer-progress \ - -Dtest='ITestS3AContract*, ITestS3ACommitterMRJob, !ITestS3AContractDistCp, !ITestS3AContractGetFileStatusV1List, !ITestS3AContractMkdir, !ITestS3AContractRename' \ + -Dtest='ITestS3AContract*, ITestS3ACommitterMRJob, !ITestS3AContractDistCp, !ITestS3AContractGetFileStatusV1List, !ITestS3AContractRename' \ clean test local target="${RESULT_DIR}/junit/${bucket}/target" diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 85e205d08593..a4fa2bee866a 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1301,16 +1301,16 @@ message DeleteKeyArgs { repeated string keys = 3; } -message DeleteKeyErrors { - required string key = 1; - required string errorCode = 2; - required string errorMsg = 3; +message DeleteKeyError { + optional string key = 1; + optional string errorCode = 2; + optional string errorMsg = 3; } message DeleteKeysResponse { optional DeleteKeyArgs unDeletedKeys = 1; optional bool status = 2; - repeated DeleteKeyErrors deleteKeyErrors = 3; + repeated DeleteKeyError errors = 3; } message DeleteKeyResponse { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java index f62d1f62ed45..de61fdb7fc26 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java @@ -269,15 +269,15 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, Map> keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; - List deleteKeyErrors = new ArrayList<>(); + List deleteKeyErrors = new ArrayList<>(); for (Map.Entry> key : keyToErrors.entrySet()) { - deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyErrors.newBuilder() + deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyError.newBuilder() .setKey(key.getKey()).setErrorCode(key.getValue().getLeft()).setErrorMsg(key.getValue().getRight()).build()); } omClientResponse = new OMKeysDeleteResponse(omResponse .setDeleteKeysResponse( DeleteKeysResponse.newBuilder().setStatus(deleteStatus) - .setUnDeletedKeys(unDeletedKeys).addAllDeleteKeyErrors(deleteKeyErrors)) + .setUnDeletedKeys(unDeletedKeys).addAllErrors(deleteKeyErrors)) .setStatus(deleteStatus ? OK : PARTIAL_DELETE).setSuccess(deleteStatus) .build(), omKeyInfoList, ozoneManager.isRatisEnabled(), omBucketInfo.copyObject(), dbOpenKeys); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java index 789aae79ca50..fb3fcd4ac42a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java @@ -152,15 +152,15 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, Map> keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; - List deleteKeyErrors = new ArrayList<>(); + List deleteKeyErrors = new ArrayList<>(); for (Map.Entry> key : keyToErrors.entrySet()) { - deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyErrors.newBuilder() + deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyError.newBuilder() .setKey(key.getKey()).setErrorCode(key.getValue().getLeft()).setErrorMsg(key.getValue().getRight()).build()); } omClientResponse = new OMKeysDeleteResponseWithFSO(omResponse .setDeleteKeysResponse( OzoneManagerProtocolProtos.DeleteKeysResponse.newBuilder() - .setStatus(deleteStatus).setUnDeletedKeys(unDeletedKeys).addAllDeleteKeyErrors(deleteKeyErrors)) + .setStatus(deleteStatus).setUnDeletedKeys(unDeletedKeys).addAllErrors(deleteKeyErrors)) .setStatus(deleteStatus ? OK : PARTIAL_DELETE).setSuccess(deleteStatus) .build(), omKeyInfoList, dirList, ozoneManager.isRatisEnabled(), omBucketInfo.copyObject(), volumeId, dbOpenKeys); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java index 85c12a9ffccc..3f90f1531760 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java @@ -23,7 +23,7 @@ import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyArgs; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyErrors; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyError; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeysRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.junit.jupiter.api.Test; @@ -74,8 +74,8 @@ protected void checkDeleteKeysResponse( omClientResponse.getOMResponse().getDeleteKeysResponse() .getUnDeletedKeys(); assertEquals(0, unDeletedKeys.getKeysCount()); - List keyErrors = omClientResponse.getOMResponse().getDeleteKeysResponse() - .getDeleteKeyErrorsList(); + List keyErrors = omClientResponse.getOMResponse().getDeleteKeysResponse() + .getErrorsList(); assertEquals(0, keyErrors.size()); // Check all keys are deleted. @@ -127,8 +127,8 @@ protected void checkDeleteKeysResponseForFailure( .getDeleteKeysResponse().getUnDeletedKeys(); assertEquals(1, unDeletedKeys.getKeysCount()); - List keyErrors = omClientResponse.getOMResponse().getDeleteKeysResponse() - .getDeleteKeyErrorsList(); + List keyErrors = omClientResponse.getOMResponse().getDeleteKeysResponse() + .getErrorsList(); assertEquals(1, keyErrors.size()); assertEquals("dummy", unDeletedKeys.getKeys(0)); } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index 2988c17871af..3d09c7d2986a 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -456,7 +456,7 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, } long startNanos = Time.monotonicNowNanos(); try { - undeletedKeyResultMap = bucket.deleteKeysQuiet(deleteKeys, true); + undeletedKeyResultMap = bucket.deleteKeys(deleteKeys, true); for (DeleteObject d : request.getObjects()) { if (!request.isQuiet() && (!(undeletedKeyResultMap.containsKey(d.getKey())) || undeletedKeyResultMap.get(d.getKey()).getLeft().equals(ResultCodes.KEY_NOT_FOUND.name()))) { diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java index b95fc4ddff5f..c94fb5244d76 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java @@ -263,8 +263,8 @@ public void deleteKeys(String volumeName, String bucketName, } @Override - public Map> deleteKeysQuiet(String volumeName, String bucketName, - List keyNameList, Boolean isQuiet) + public Map> deleteKeys(String volumeName, String bucketName, + List keyNameList, boolean quiet) throws IOException { return new HashMap<>(); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java index 60ddc9c573ad..34929ba5dc6d 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java @@ -360,13 +360,12 @@ public void deleteKey(String key) throws IOException { } @Override - public Map> deleteKeysQuiet(List keyList, Boolean isQuiet) throws IOException { + public Map> deleteKeys(List keyList, boolean quiet) throws IOException { Map> keyErrorMap = new HashMap<>(); for (String key : keyList) { - if (!keyDetails.containsKey(key)) { + if (keyDetails.remove(key) == null) { keyErrorMap.put(key, Pair.of("KEY_NOT_FOUND", "Key does not exist")); } - keyDetails.remove(key); } return keyErrorMap; } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java index 22fb90ff1618..7263b67ddf7b 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java @@ -171,7 +171,7 @@ public void testDeleteKeys() throws IOException, OS3Exception { when(objectStore.getS3Bucket(anyString())).thenReturn(bucket); Map> deleteErrors = new HashMap<>(); deleteErrors.put("deleteKeyName", Pair.of("ACCESS_DENIED", "ACL check failed")); - when(bucket.deleteKeysQuiet(any(), anyBoolean())).thenReturn(deleteErrors); + when(bucket.deleteKeys(any(), anyBoolean())).thenReturn(deleteErrors); BucketEndpoint bucketEndpoint = new BucketEndpoint(); bucketEndpoint.setClient(client); From 5cf0f4baea31df3894a6d2cb12347d559a0eb6d6 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Mon, 3 Jun 2024 13:58:10 +0530 Subject: [PATCH 09/12] Create ErrorInfo wrapper class --- .../hadoop/ozone/client/OzoneBucket.java | 3 +- .../ozone/client/protocol/ClientProtocol.java | 6 +-- .../hadoop/ozone/client/rpc/RpcClient.java | 4 +- .../hadoop/ozone/om/helpers/ErrorInfo.java | 49 +++++++++++++++++++ .../om/protocol/OzoneManagerProtocol.java | 4 +- ...ManagerProtocolClientSideTranslatorPB.java | 9 ++-- .../om/request/key/OMKeysDeleteRequest.java | 17 ++++--- .../key/OmKeysDeleteRequestWithFSO.java | 9 ++-- .../ozone/s3/endpoint/BucketEndpoint.java | 12 ++--- .../ozone/client/ClientProtocolStub.java | 6 +-- .../hadoop/ozone/client/OzoneBucketStub.java | 8 +-- .../s3/endpoint/TestPermissionCheck.java | 6 +-- 12 files changed, 93 insertions(+), 40 deletions(-) create mode 100644 hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/ErrorInfo.java diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index a39682cf1421..575701be8001 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -39,6 +39,7 @@ import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BasicOmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; @@ -681,7 +682,7 @@ public void deleteKeys(List keyList) throws IOException { * @param quiet flag to not throw exception if delete fails * @throws IOException */ - public Map> deleteKeys(List keyList, boolean quiet) throws IOException { + public Map deleteKeys(List keyList, boolean quiet) throws IOException { return proxy.deleteKeys(volumeName, name, keyList, quiet); } diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 26a65e561482..8599b6a8b793 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -24,7 +24,6 @@ import java.util.Map; import jakarta.annotation.Nonnull; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationFactor; @@ -50,6 +49,7 @@ import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.DeleteTenantState; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteInfo; @@ -445,8 +445,8 @@ void deleteKeys(String volumeName, String bucketName, * @param quiet flag to not throw exception if delete fails * @throws IOException */ - Map> deleteKeys(String volumeName, String bucketName, - List keyNameList, boolean quiet) + Map deleteKeys(String volumeName, String bucketName, + List keyNameList, boolean quiet) throws IOException; /** diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 7e48d340ce31..6ef0591b969b 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -29,7 +29,6 @@ import javax.crypto.Cipher; import javax.crypto.CipherInputStream; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.crypto.CryptoInputStream; import org.apache.hadoop.crypto.CryptoOutputStream; import org.apache.hadoop.crypto.key.KeyProvider; @@ -98,6 +97,7 @@ import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.DeleteTenantState; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.KeyInfoWithVolumeContext; import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -1644,7 +1644,7 @@ public void deleteKeys( } @Override - public Map> deleteKeys( + public Map deleteKeys( String volumeName, String bucketName, List keyNameList, boolean quiet) throws IOException { verifyVolumeName(volumeName); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/ErrorInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/ErrorInfo.java new file mode 100644 index 000000000000..7889a568be84 --- /dev/null +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/ErrorInfo.java @@ -0,0 +1,49 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.helpers; + +/** + * Represent class which has info of error thrown for any operation. + */ +public class ErrorInfo { + private String code; + private String message; + + public ErrorInfo(String errorCode, String errorMessage) { + this.code = errorCode; + this.message = errorMessage; + } + + public String getCode() { + return code; + } + + public void setCode(String errorCode) { + this.code = errorCode; + } + + public String getMessage() { + return message; + } + + public void setMessage(String errorMessage) { + this.message = errorMessage; + } + +} diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java index 37e63ef806ec..8d9200c35824 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OzoneManagerProtocol.java @@ -25,7 +25,6 @@ import java.util.UUID; import jakarta.annotation.Nonnull; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.fs.SafeModeAction; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.ozone.OzoneAcl; @@ -34,6 +33,7 @@ import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.DBUpdates; import org.apache.hadoop.ozone.om.helpers.DeleteTenantState; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.KeyInfoWithVolumeContext; import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -370,7 +370,7 @@ default void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { * @param quiet - flag to not throw exception if delete fails * @throws IOException */ - default Map> deleteKeys(OmDeleteKeys deleteKeys, boolean quiet) + default Map deleteKeys(OmDeleteKeys deleteKeys, boolean quiet) throws IOException { throw new UnsupportedOperationException("OzoneManager does not require " + "this to be implemented, as write requests use a new approach."); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index 8449f906cf3d..f594bf2f91dd 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -29,7 +29,6 @@ import com.google.protobuf.Proto2Utils; import jakarta.annotation.Nonnull; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.fs.SafeModeAction; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.client.ECReplicationConfig; @@ -44,6 +43,7 @@ import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BasicOmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.ListKeysLightResult; import org.apache.hadoop.ozone.om.helpers.ListKeysResult; import org.apache.hadoop.ozone.om.helpers.DBUpdates; @@ -959,7 +959,7 @@ public void deleteKeys(OmDeleteKeys deleteKeys) throws IOException { } @Override - public Map> deleteKeys(OmDeleteKeys deleteKeys, boolean quiet) + public Map deleteKeys(OmDeleteKeys deleteKeys, boolean quiet) throws IOException { DeleteKeysRequest.Builder req = DeleteKeysRequest.newBuilder(); DeleteKeyArgs deletedKeys = DeleteKeyArgs.newBuilder() @@ -977,9 +977,10 @@ public Map> deleteKeys(OmDeleteKeys deleteKeys, boo } List errors = omResponse.getDeleteKeysResponse().getErrorsList(); - Map> keyToErrors = new HashMap<>(); + Map keyToErrors = new HashMap<>(); for (OzoneManagerProtocolProtos.DeleteKeyError deleteKeyError : errors) { - keyToErrors.put(deleteKeyError.getKey(), Pair.of(deleteKeyError.getErrorCode(), deleteKeyError.getErrorMsg())); + keyToErrors.put(deleteKeyError.getKey(), + new ErrorInfo(deleteKeyError.getErrorCode(), deleteKeyError.getErrorMsg())); } return keyToErrors; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java index de61fdb7fc26..19e9ed716e4a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java @@ -31,6 +31,7 @@ import org.apache.hadoop.ozone.om.ResolvedBucket; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; @@ -96,7 +97,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn Exception exception = null; OMClientResponse omClientResponse = null; Result result = null; - Map> keyToError = new HashMap<>(); + Map keyToError = new HashMap<>(); OMMetrics omMetrics = ozoneManager.getMetrics(); omMetrics.incNumKeyDeletes(); @@ -152,7 +153,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn objectKey); deleteKeys.remove(keyName); unDeletedKeys.addKeys(keyName); - keyToError.put(keyName, Pair.of(OMException.ResultCodes.KEY_NOT_FOUND.name(), "Key does not exist")); + keyToError.put(keyName, new ErrorInfo(OMException.ResultCodes.KEY_NOT_FOUND.name(), "Key does not exist")); continue; } @@ -170,7 +171,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn LOG.error("Acl check failed for Key: {}", objectKey, ex); deleteKeys.remove(keyName); unDeletedKeys.addKeys(keyName); - keyToError.put(keyName, Pair.of(OMException.ResultCodes.ACCESS_DENIED.name(), "ACL check failed")); + keyToError.put(keyName, new ErrorInfo(OMException.ResultCodes.ACCESS_DENIED.name(), "ACL check failed")); } } @@ -203,7 +204,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn // Add all keys which are failed due to any other exception . for (int i = indexFailed; i < length; i++) { unDeletedKeys.addKeys(deleteKeyArgs.getKeys(i)); - keyToError.put(deleteKeyArgs.getKeys(i), Pair.of(OMException.ResultCodes.INTERNAL_ERROR.name(), + keyToError.put(deleteKeyArgs.getKeys(i), new ErrorInfo(OMException.ResultCodes.INTERNAL_ERROR.name(), ex.getMessage())); } @@ -266,13 +267,13 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, - Map> keyToErrors, + Map keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; List deleteKeyErrors = new ArrayList<>(); - for (Map.Entry> key : keyToErrors.entrySet()) { - deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyError.newBuilder() - .setKey(key.getKey()).setErrorCode(key.getValue().getLeft()).setErrorMsg(key.getValue().getRight()).build()); + for (Map.Entry key : keyToErrors.entrySet()) { + deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyError.newBuilder().setKey(key.getKey()) + .setErrorCode(key.getValue().getCode()).setErrorMsg(key.getValue().getMessage()).build()); } omClientResponse = new OMKeysDeleteResponse(omResponse .setDeleteKeysResponse( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java index fb3fcd4ac42a..421d7d1a9bfc 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java @@ -17,7 +17,6 @@ */ package org.apache.hadoop.ozone.om.request.key; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; @@ -25,6 +24,7 @@ import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; @@ -149,13 +149,14 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OzoneManagerProtocolProtos.OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, - Map> keyToErrors, + Map keyToErrors, boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, List dbOpenKeys) { OMClientResponse omClientResponse; List deleteKeyErrors = new ArrayList<>(); - for (Map.Entry> key : keyToErrors.entrySet()) { + for (Map.Entry key : keyToErrors.entrySet()) { deleteKeyErrors.add(OzoneManagerProtocolProtos.DeleteKeyError.newBuilder() - .setKey(key.getKey()).setErrorCode(key.getValue().getLeft()).setErrorMsg(key.getValue().getRight()).build()); + .setKey(key.getKey()).setErrorCode(key.getValue().getCode()) + .setErrorMsg(key.getValue().getMessage()).build()); } omClientResponse = new OMKeysDeleteResponseWithFSO(omResponse .setDeleteKeysResponse( diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index 3d09c7d2986a..cf9caeccde83 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone.s3.endpoint; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.OzoneAcl; @@ -29,6 +28,7 @@ import org.apache.hadoop.ozone.client.OzoneVolume; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil; import org.apache.hadoop.ozone.s3.commontypes.EncodingTypeObject; import org.apache.hadoop.ozone.s3.commontypes.KeyMetadata; @@ -447,7 +447,7 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, OzoneBucket bucket = getBucket(bucketName); MultiDeleteResponse result = new MultiDeleteResponse(); - Map> undeletedKeyResultMap; + Map undeletedKeyResultMap; if (request.getObjects() != null) { List deleteKeys = new ArrayList<>(); @@ -459,12 +459,12 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, undeletedKeyResultMap = bucket.deleteKeys(deleteKeys, true); for (DeleteObject d : request.getObjects()) { if (!request.isQuiet() && (!(undeletedKeyResultMap.containsKey(d.getKey())) || - undeletedKeyResultMap.get(d.getKey()).getLeft().equals(ResultCodes.KEY_NOT_FOUND.name()))) { + undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name()))) { result.addDeleted(new DeletedObject(d.getKey())); } else if (undeletedKeyResultMap.containsKey(d.getKey()) && - !undeletedKeyResultMap.get(d.getKey()).getLeft().equals(ResultCodes.KEY_NOT_FOUND.name())) { - Pair error = undeletedKeyResultMap.get(d.getKey()); - result.addError(new Error(d.getKey(), error.getLeft(), error.getRight())); + !undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name())) { + ErrorInfo error = undeletedKeyResultMap.get(d.getKey()); + result.addError(new Error(d.getKey(), error.getCode(), error.getMessage())); } } getMetrics().updateDeleteKeySuccessStats(startNanos); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java index c94fb5244d76..daae84213ac6 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ClientProtocolStub.java @@ -20,7 +20,6 @@ package org.apache.hadoop.ozone.client; import jakarta.annotation.Nonnull; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationFactor; @@ -34,6 +33,7 @@ import org.apache.hadoop.ozone.client.io.OzoneOutputStream; import org.apache.hadoop.ozone.client.protocol.ClientProtocol; import org.apache.hadoop.ozone.om.helpers.DeleteTenantState; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteInfo; @@ -263,8 +263,8 @@ public void deleteKeys(String volumeName, String bucketName, } @Override - public Map> deleteKeys(String volumeName, String bucketName, - List keyNameList, boolean quiet) + public Map deleteKeys(String volumeName, String bucketName, + List keyNameList, boolean quiet) throws IOException { return new HashMap<>(); } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java index 34929ba5dc6d..3320801874d6 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java @@ -38,7 +38,6 @@ import javax.xml.bind.DatatypeConverter; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationFactor; @@ -53,6 +52,7 @@ import org.apache.hadoop.ozone.client.OzoneMultipartUploadPartListParts.PartInfo; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteInfo; import org.apache.hadoop.security.UserGroupInformation; @@ -360,11 +360,11 @@ public void deleteKey(String key) throws IOException { } @Override - public Map> deleteKeys(List keyList, boolean quiet) throws IOException { - Map> keyErrorMap = new HashMap<>(); + public Map deleteKeys(List keyList, boolean quiet) throws IOException { + Map keyErrorMap = new HashMap<>(); for (String key : keyList) { if (keyDetails.remove(key) == null) { - keyErrorMap.put(key, Pair.of("KEY_NOT_FOUND", "Key does not exist")); + keyErrorMap.put(key, new ErrorInfo("KEY_NOT_FOUND", "Key does not exist")); } } return keyErrorMap; diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java index 7263b67ddf7b..b74808de953f 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java @@ -19,7 +19,6 @@ */ package org.apache.hadoop.ozone.s3.endpoint; -import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.client.ObjectStore; @@ -28,6 +27,7 @@ import org.apache.hadoop.ozone.client.OzoneVolume; import org.apache.hadoop.ozone.client.protocol.ClientProtocol; import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.ErrorInfo; import org.apache.hadoop.ozone.s3.exception.OS3Exception; import org.apache.hadoop.ozone.s3.metrics.S3GatewayMetrics; import org.junit.jupiter.api.BeforeEach; @@ -169,8 +169,8 @@ public void testListKey() throws IOException { public void testDeleteKeys() throws IOException, OS3Exception { when(objectStore.getVolume(anyString())).thenReturn(volume); when(objectStore.getS3Bucket(anyString())).thenReturn(bucket); - Map> deleteErrors = new HashMap<>(); - deleteErrors.put("deleteKeyName", Pair.of("ACCESS_DENIED", "ACL check failed")); + Map deleteErrors = new HashMap<>(); + deleteErrors.put("deleteKeyName", new ErrorInfo("ACCESS_DENIED", "ACL check failed")); when(bucket.deleteKeys(any(), anyBoolean())).thenReturn(deleteErrors); BucketEndpoint bucketEndpoint = new BucketEndpoint(); From bd1e1e402db1b3cbe67b677c01a42c16b25ac4f5 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Mon, 3 Jun 2024 13:59:45 +0530 Subject: [PATCH 10/12] add comments --- .../org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index cf9caeccde83..ac11ea46a962 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -460,9 +460,11 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, for (DeleteObject d : request.getObjects()) { if (!request.isQuiet() && (!(undeletedKeyResultMap.containsKey(d.getKey())) || undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name()))) { + // if the key is not found, it is assumed to be successfully deleted result.addDeleted(new DeletedObject(d.getKey())); } else if (undeletedKeyResultMap.containsKey(d.getKey()) && !undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name())) { + // All errors other than KEY_NOT_FOUND are returned ErrorInfo error = undeletedKeyResultMap.get(d.getKey()); result.addError(new Error(d.getKey(), error.getCode(), error.getMessage())); } From 0bb06fce97f22378576303d3b259d927ba5df6e1 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Tue, 4 Jun 2024 14:18:30 +0530 Subject: [PATCH 11/12] Audit only failed deletes, create error map only for quiet delete --- ...neManagerProtocolClientSideTranslatorPB.java | 17 +++++++++-------- .../ozone/s3/endpoint/BucketEndpoint.java | 17 ++++++++--------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index f594bf2f91dd..0e4cab40edde 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -972,15 +972,16 @@ public Map deleteKeys(OmDeleteKeys deleteKeys, boolean quiet) .build(); OMResponse omResponse = submitRequest(omRequest); - if (!quiet) { - handleError(omResponse); - } - List errors = - omResponse.getDeleteKeysResponse().getErrorsList(); Map keyToErrors = new HashMap<>(); - for (OzoneManagerProtocolProtos.DeleteKeyError deleteKeyError : errors) { - keyToErrors.put(deleteKeyError.getKey(), - new ErrorInfo(deleteKeyError.getErrorCode(), deleteKeyError.getErrorMsg())); + if (quiet) { + List errors = + omResponse.getDeleteKeysResponse().getErrorsList(); + for (OzoneManagerProtocolProtos.DeleteKeyError deleteKeyError : errors) { + keyToErrors.put(deleteKeyError.getKey(), + new ErrorInfo(deleteKeyError.getErrorCode(), deleteKeyError.getErrorMsg())); + } + } else { + handleError(omResponse); } return keyToErrors; } diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index ac11ea46a962..938ee6ec9089 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -448,9 +448,9 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, OzoneBucket bucket = getBucket(bucketName); MultiDeleteResponse result = new MultiDeleteResponse(); Map undeletedKeyResultMap; + List deleteKeys = new ArrayList<>(); if (request.getObjects() != null) { - List deleteKeys = new ArrayList<>(); for (DeleteObject keyToDelete : request.getObjects()) { deleteKeys.add(keyToDelete.getKey()); } @@ -458,10 +458,13 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, try { undeletedKeyResultMap = bucket.deleteKeys(deleteKeys, true); for (DeleteObject d : request.getObjects()) { - if (!request.isQuiet() && (!(undeletedKeyResultMap.containsKey(d.getKey())) || - undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name()))) { + if (!(undeletedKeyResultMap.containsKey(d.getKey())) || + undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name())) { // if the key is not found, it is assumed to be successfully deleted - result.addDeleted(new DeletedObject(d.getKey())); + deleteKeys.remove(d.getKey()); + if (!request.isQuiet()) { + result.addDeleted(new DeletedObject(d.getKey())); + } } else if (undeletedKeyResultMap.containsKey(d.getKey()) && !undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name())) { // All errors other than KEY_NOT_FOUND are returned @@ -480,11 +483,7 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, } Map auditMap = getAuditParameters(); - List keys = new ArrayList<>(); - for (DeleteObject d : request.getObjects()) { - keys.add(d.getKey()); - } - auditMap.put("Keys", keys.toString()); + auditMap.put("failedDeletes", deleteKeys.toString()); if (result.getErrors().size() != 0) { AUDIT.logWriteFailure(buildAuditMessageForFailure(s3GAction, auditMap, new Exception("MultiDelete Exception"))); From 499809d6056a6fb5a076a4d2adfd7a8e9b670e02 Mon Sep 17 00:00:00 2001 From: tejaskriya Date: Fri, 7 Jun 2024 00:19:36 +0530 Subject: [PATCH 12/12] Simplify check in BucketEndpoint --- .../hadoop/ozone/s3/endpoint/BucketEndpoint.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java index 938ee6ec9089..1b845c79aebe 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java @@ -447,10 +447,10 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, OzoneBucket bucket = getBucket(bucketName); MultiDeleteResponse result = new MultiDeleteResponse(); - Map undeletedKeyResultMap; List deleteKeys = new ArrayList<>(); if (request.getObjects() != null) { + Map undeletedKeyResultMap; for (DeleteObject keyToDelete : request.getObjects()) { deleteKeys.add(keyToDelete.getKey()); } @@ -458,17 +458,16 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName, try { undeletedKeyResultMap = bucket.deleteKeys(deleteKeys, true); for (DeleteObject d : request.getObjects()) { - if (!(undeletedKeyResultMap.containsKey(d.getKey())) || - undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name())) { - // if the key is not found, it is assumed to be successfully deleted + ErrorInfo error = undeletedKeyResultMap.get(d.getKey()); + boolean deleted = error == null || + // if the key is not found, it is assumed to be successfully deleted + ResultCodes.KEY_NOT_FOUND.name().equals(error.getCode()); + if (deleted) { deleteKeys.remove(d.getKey()); if (!request.isQuiet()) { result.addDeleted(new DeletedObject(d.getKey())); } - } else if (undeletedKeyResultMap.containsKey(d.getKey()) && - !undeletedKeyResultMap.get(d.getKey()).getCode().equals(ResultCodes.KEY_NOT_FOUND.name())) { - // All errors other than KEY_NOT_FOUND are returned - ErrorInfo error = undeletedKeyResultMap.get(d.getKey()); + } else { result.addError(new Error(d.getKey(), error.getCode(), error.getMessage())); } }