This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new ad059968a55 HDDS-16604. S3 DeleteObjects returns 200 when OM rejects
the whole batch (#11327)
ad059968a55 is described below
commit ad059968a55717eef00607ca2da9fe95926edac7
Author: Mark Tsai <[email protected]>
AuthorDate: Sat Sep 26 14:55:24 2026 +0800
HDDS-16604. S3 DeleteObjects returns 200 when OM rejects the whole batch
(#11327)
---
.../apache/hadoop/ozone/client/OzoneBucket.java | 5 +-
.../ozone/client/protocol/ClientProtocol.java | 5 +-
.../ozone/om/protocol/OzoneManagerProtocol.java | 5 +-
...OzoneManagerProtocolClientSideTranslatorPB.java | 5 +-
...OzoneManagerProtocolClientSideTranslatorPB.java | 63 ++++++++++++++++++++++
5 files changed, 76 insertions(+), 7 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 e3cbcce76f3..29fa6de0ff6 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
@@ -862,8 +862,9 @@ public void deleteKeys(List<String> keyList) throws
IOException {
/**
* Deletes the given list of keys from the bucket.
* @param keyList List of the key name to be deleted.
- * @param quiet flag to not throw exception if delete fails
- * @throws IOException
+ * @param quiet if true, per-key failures are returned in the result map
instead of being thrown
+ * @return key name to error for each key that could not be deleted, empty
if all keys were deleted
+ * @throws IOException if the request fails as a whole (e.g. bucket not
found), even when quiet is true
*/
public Map<String, ErrorInfo> deleteKeys(List<String> 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 c4355581db6..cb553d33a9d 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
@@ -680,8 +680,9 @@ 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 quiet flag to not throw exception if delete fails
- * @throws IOException
+ * @param quiet if true, per-key failures are returned in the result map
instead of being thrown
+ * @return key name to error for each key that could not be deleted, empty
if all keys were deleted
+ * @throws IOException if the request fails as a whole (e.g. bucket not
found), even when quiet is true
*/
Map<String, ErrorInfo> deleteKeys(String volumeName, String bucketName,
List<String> keyNameList, boolean quiet)
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 3eea12d53f8..71c2d6b2294 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
@@ -379,8 +379,9 @@ default void deleteKeys(OmDeleteKeys deleteKeys) throws
IOException {
* through OzoneFileSystem.
*
* @param deleteKeys
- * @param quiet - flag to not throw exception if delete fails
- * @throws IOException
+ * @param quiet - if true, per-key failures are returned in the result map
instead of being thrown
+ * @return key name to error for each key that could not be deleted, empty
if all keys were deleted
+ * @throws IOException if the request fails as a whole (e.g. bucket not
found), even when quiet is true
*/
default Map<String, ErrorInfo> deleteKeys(OmDeleteKeys deleteKeys, boolean
quiet)
throws IOException {
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 79f2d3900c2..a6f8c7053c1 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
@@ -24,6 +24,7 @@
import static
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.ACCESS_DENIED;
import static
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.DIRECTORY_ALREADY_EXISTS;
import static
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK;
+import static
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.PARTIAL_DELETE;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
@@ -1041,7 +1042,9 @@ public Map<String, ErrorInfo> deleteKeys(OmDeleteKeys
deleteKeys, boolean quiet)
OMResponse omResponse = submitRequest(omRequest);
Map<String, ErrorInfo> keyToErrors = new HashMap<>();
- if (quiet) {
+ if (quiet && omResponse.getStatus() == PARTIAL_DELETE) {
+ // PARTIAL_DELETE means the batch was processed and only some keys
failed; those are reported per key
+ // in the returned map. Any other non-OK status means the whole request
failed.
List<OzoneManagerProtocolProtos.DeleteKeyError> errors =
omResponse.getDeleteKeysResponse().getErrorsList();
for (OzoneManagerProtocolProtos.DeleteKeyError deleteKeyError : errors) {
diff --git
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/protocolPB/TestOzoneManagerProtocolClientSideTranslatorPB.java
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/protocolPB/TestOzoneManagerProtocolClientSideTranslatorPB.java
index e08d1e35df7..e09e196f6f3 100644
---
a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/protocolPB/TestOzoneManagerProtocolClientSideTranslatorPB.java
+++
b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/protocolPB/TestOzoneManagerProtocolClientSideTranslatorPB.java
@@ -19,6 +19,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -26,8 +27,16 @@
import static org.mockito.Mockito.when;
import java.io.IOException;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
+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.OmDeleteKeys;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyError;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeysResponse;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.StartQuotaRepairRequest;
@@ -42,6 +51,7 @@ class TestOzoneManagerProtocolClientSideTranslatorPB {
private final OmTransport omTransport = mock(OmTransport.class);
private final OzoneManagerProtocolClientSideTranslatorPB pb = new
OzoneManagerProtocolClientSideTranslatorPB(
omTransport, "test-client-id");
+ private static final OmDeleteKeys DELETE_KEYS = new OmDeleteKeys("vol",
"bucket", Arrays.asList("key1", "key2"));
@Test
void testStartQuotaRepair() throws IOException {
@@ -99,4 +109,57 @@ void testStartQuotaRepairWithNullBuckets() {
verifyNoInteractions(omTransport);
}
+ @Test
+ void testQuietDeleteKeysThrowsWhenWholeBatchFails() throws IOException {
+ // Whole-batch failure at OM: non-OK status, DeleteKeysResponse has
status=false and no per-key errors.
+ when(omTransport.submitRequest(any(OMRequest.class))).thenReturn(
+ OMResponse.newBuilder()
+ .setCmdType(Type.DeleteKeys)
+ .setStatus(Status.BUCKET_NOT_FOUND)
+ .setSuccess(false)
+ .setMessage("Bucket not found")
+
.setDeleteKeysResponse(DeleteKeysResponse.newBuilder().setStatus(false))
+ .build());
+
+ OMException e = assertThrows(OMException.class, () ->
pb.deleteKeys(DELETE_KEYS, true));
+
+ assertThat(e.getResult()).isEqualTo(ResultCodes.BUCKET_NOT_FOUND);
+ assertThat(e).hasMessage("Bucket not found");
+ }
+
+ @Test
+ void testQuietDeleteKeysReturnsPerKeyErrorsOnPartialDelete() throws
IOException {
+ DeleteKeyError keyError = DeleteKeyError.newBuilder()
+ .setKey("key2")
+ .setErrorCode(ResultCodes.ACCESS_DENIED.name())
+ .setErrorMsg("ACL check failed")
+ .build();
+ when(omTransport.submitRequest(any(OMRequest.class))).thenReturn(
+ OMResponse.newBuilder()
+ .setCmdType(Type.DeleteKeys)
+ .setStatus(Status.PARTIAL_DELETE)
+ .setSuccess(false)
+
.setDeleteKeysResponse(DeleteKeysResponse.newBuilder().setStatus(false).addErrors(keyError))
+ .build());
+
+ Map<String, ErrorInfo> errors = pb.deleteKeys(DELETE_KEYS, true);
+
+ assertThat(errors).hasSize(1).containsKey("key2");
+
assertThat(errors.get("key2").getCode()).isEqualTo(ResultCodes.ACCESS_DENIED.name());
+ assertThat(errors.get("key2").getMessage()).isEqualTo("ACL check failed");
+ }
+
+ @Test
+ void testQuietDeleteKeysReturnsEmptyMapOnSuccess() throws IOException {
+ when(omTransport.submitRequest(any(OMRequest.class))).thenReturn(
+ OMResponse.newBuilder()
+ .setCmdType(Type.DeleteKeys)
+ .setStatus(Status.OK)
+ .setSuccess(true)
+
.setDeleteKeysResponse(DeleteKeysResponse.newBuilder().setStatus(true))
+ .build());
+
+ assertThat(pb.deleteKeys(DELETE_KEYS, true)).isEmpty();
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
