From bbec4b9d742dd1885e3b552f192c5ff09a3b6934 Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 16 Jul 2026 11:00:54 +0800 Subject: [PATCH 01/23] T1.1. Add bucket versioning three-state enum to proto and bucket helpers Co-Authored-By: Claude Fable 5 --- .../om/helpers/BucketVersioningStatus.java | 73 ++++++++++++++++ .../hadoop/ozone/om/helpers/OmBucketArgs.java | 27 ++++++ .../hadoop/ozone/om/helpers/OmBucketInfo.java | 42 ++++++++- .../ozone/om/helpers/TestOmBucketArgs.java | 26 ++++++ .../ozone/om/helpers/TestOmBucketInfo.java | 86 +++++++++++++++++++ .../src/main/proto/OmClientProtocol.proto | 17 ++++ 6 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java new file mode 100644 index 000000000000..424166bb473f --- /dev/null +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java @@ -0,0 +1,73 @@ +/* + * 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; + +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.BucketVersioningStatusProto; + +/** + * S3-compatible bucket versioning state machine: + * UNVERSIONED -> ENABLED <-> SUSPENDED. + * Once versioning has been enabled, a bucket can never return to UNVERSIONED. + */ +public enum BucketVersioningStatus { + UNVERSIONED, + ENABLED, + SUSPENDED; + + public static BucketVersioningStatus fromProto(BucketVersioningStatusProto proto) { + switch (proto) { + case VERSIONING_ENABLED: + return ENABLED; + case VERSIONING_SUSPENDED: + return SUSPENDED; + case UNVERSIONED: + default: + return UNVERSIONED; + } + } + + public BucketVersioningStatusProto toProto() { + switch (this) { + case ENABLED: + return BucketVersioningStatusProto.VERSIONING_ENABLED; + case SUSPENDED: + return BucketVersioningStatusProto.VERSIONING_SUSPENDED; + default: + return BucketVersioningStatusProto.UNVERSIONED; + } + } + + /** Maps the legacy isVersionEnabled flag of buckets without an explicit status. */ + public static BucketVersioningStatus fromVersionEnabledFlag(boolean isVersionEnabled) { + return isVersionEnabled ? ENABLED : UNVERSIONED; + } + + /** The legacy isVersionEnabled flag value kept in sync with this status. */ + public boolean toVersionEnabledFlag() { + return this == ENABLED; + } + + /** + * Whether a bucket may transition from this status to {@code target}. + * The only forbidden transition is back to UNVERSIONED after versioning + * has been enabled or suspended (matches the S3 state machine). + */ + public boolean canTransitionTo(BucketVersioningStatus target) { + return this == UNVERSIONED || target != UNVERSIONED; + } +} diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java index 8eed2630ead6..6b898a27e47f 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java @@ -44,6 +44,11 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { * Bucket Version flag. */ private final Boolean isVersionEnabled; + /** + * S3-compatible versioning status; null when not being changed. + * Takes precedence over isVersionEnabled when both are set. + */ + private final BucketVersioningStatus versioningStatus; /** * Type of storage to be used for this bucket. * [RAM_DISK, SSD, DISK, ARCHIVE] @@ -73,6 +78,7 @@ private OmBucketArgs(Builder b) { this.volumeName = b.volumeName; this.bucketName = b.bucketName; this.isVersionEnabled = b.isVersionEnabled; + this.versioningStatus = b.versioningStatus; this.storageType = b.storageType; this.ownerName = b.ownerName; this.defaultReplicationConfig = b.defaultReplicationConfig; @@ -108,6 +114,14 @@ public Boolean getIsVersionEnabled() { return isVersionEnabled; } + /** + * Returns the requested versioning status, or null if not being changed. + * @return BucketVersioningStatus + */ + public BucketVersioningStatus getVersioningStatus() { + return versioningStatus; + } + /** * Returns the type of storage to be used. * @return StorageType @@ -227,6 +241,7 @@ public static class Builder extends WithMetadata.Builder { private String volumeName; private String bucketName; private Boolean isVersionEnabled; + private BucketVersioningStatus versioningStatus; private StorageType storageType; private boolean quotaInBytesSet = false; private long quotaInBytes; @@ -261,6 +276,11 @@ public Builder setIsVersionEnabled(Boolean versionFlag) { return this; } + public Builder setVersioningStatus(BucketVersioningStatus status) { + this.versioningStatus = status; + return this; + } + @Deprecated public Builder setBucketEncryptionKey(BucketEncryptionKeyInfo info) { if (info == null || info.getKeyName() != null) { @@ -339,6 +359,9 @@ public BucketArgs getProtobuf() { if (isVersionEnabled != null) { builder.setIsVersionEnabled(isVersionEnabled); } + if (versioningStatus != null) { + builder.setVersioningStatus(versioningStatus.toProto()); + } if (storageType != null) { builder.setStorageType(storageType.toProto()); } @@ -381,6 +404,10 @@ public static Builder builderFromProtobuf(BucketArgs bucketArgs) { if (bucketArgs.hasIsVersionEnabled()) { builder.setIsVersionEnabled(bucketArgs.getIsVersionEnabled()); } + if (bucketArgs.hasVersioningStatus()) { + builder.setVersioningStatus( + BucketVersioningStatus.fromProto(bucketArgs.getVersioningStatus())); + } if (bucketArgs.hasStorageType()) { builder.setStorageType(StorageType.valueOf(bucketArgs.getStorageType())); } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java index 8da2be2755a3..0431b1eb67a7 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java @@ -59,9 +59,13 @@ public final class OmBucketInfo extends WithObjectID implements Auditable, CopyO */ private final ImmutableList acls; /** - * Bucket Version flag. + * Bucket Version flag, kept in sync with versioningStatus (ENABLED -> true). */ private final boolean isVersionEnabled; + /** + * S3-compatible versioning status; authoritative over isVersionEnabled. + */ + private final BucketVersioningStatus versioningStatus; /** * Type of storage to be used for this bucket. * [RAM_DISK, SSD, DISK, ARCHIVE] @@ -118,7 +122,9 @@ private OmBucketInfo(Builder b) { this.volumeName = b.volumeName; this.bucketName = b.bucketName; this.acls = b.acls.build(); - this.isVersionEnabled = b.isVersionEnabled; + this.versioningStatus = b.versioningStatus != null ? b.versioningStatus + : BucketVersioningStatus.fromVersionEnabledFlag(b.isVersionEnabled); + this.isVersionEnabled = this.versioningStatus.toVersionEnabledFlag(); this.storageType = b.storageType; this.creationTime = b.creationTime; this.modificationTime = b.modificationTime; @@ -173,6 +179,14 @@ public boolean getIsVersionEnabled() { return isVersionEnabled; } + /** + * Returns the S3-compatible versioning status; never null. + * @return BucketVersioningStatus + */ + public BucketVersioningStatus getVersioningStatus() { + return versioningStatus; + } + /** * Returns the type of storage to be used. * @return StorageType @@ -379,6 +393,7 @@ public Builder toBuilder() { .setBucketName(bucketName) .setStorageType(storageType) .setIsVersionEnabled(isVersionEnabled) + .setVersioningStatus(versioningStatus) .setCreationTime(creationTime) .setModificationTime(modificationTime) .setBucketEncryptionKey(bekInfo) @@ -404,6 +419,7 @@ public static class Builder extends WithObjectID.Builder { private String bucketName; private final AclListBuilder acls; private boolean isVersionEnabled; + private BucketVersioningStatus versioningStatus; private StorageType storageType = StorageType.DISK; private long creationTime; private long modificationTime; @@ -462,6 +478,22 @@ public Builder addAcl(OzoneAcl ozoneAcl) { public Builder setIsVersionEnabled(boolean versionFlag) { this.isVersionEnabled = versionFlag; + // Keep versioningStatus in sync for callers that only know the legacy + // flag; an explicitly SUSPENDED status is preserved on disable. + if (versionFlag) { + this.versioningStatus = BucketVersioningStatus.ENABLED; + } else if (versioningStatus != BucketVersioningStatus.SUSPENDED) { + this.versioningStatus = BucketVersioningStatus.UNVERSIONED; + } + return this; + } + + /** No-op when status is null (e.g. records without the new field). */ + public Builder setVersioningStatus(BucketVersioningStatus status) { + if (status != null) { + this.versioningStatus = status; + this.isVersionEnabled = status.toVersionEnabledFlag(); + } return this; } @@ -600,6 +632,7 @@ public BucketInfo getProtobuf() { .setBucketName(bucketName) .addAllAcls(OzoneAclUtil.toProtobuf(acls)) .setIsVersionEnabled(isVersionEnabled) + .setVersioningStatus(versioningStatus.toProto()) .setStorageType(storageType.toProto()) .setCreationTime(creationTime) .setModificationTime(modificationTime) @@ -657,6 +690,9 @@ public static Builder builderFromProtobuf(BucketInfo bucketInfo, .setAcls(bucketInfo.getAclsList().stream().map( OzoneAcl::fromProtobuf).collect(Collectors.toList())) .setIsVersionEnabled(bucketInfo.getIsVersionEnabled()) + .setVersioningStatus(bucketInfo.hasVersioningStatus() + ? BucketVersioningStatus.fromProto(bucketInfo.getVersioningStatus()) + : null) .setStorageType(StorageType.valueOf(bucketInfo.getStorageType())) .setCreationTime(bucketInfo.getCreationTime()) .setUsedBytes(bucketInfo.getUsedBytes()) @@ -763,6 +799,7 @@ public boolean equals(Object o) { bucketName.equals(that.bucketName) && Objects.equals(acls, that.acls) && Objects.equals(isVersionEnabled, that.isVersionEnabled) && + versioningStatus == that.versioningStatus && storageType == that.storageType && getObjectID() == that.getObjectID() && getUpdateID() == that.getUpdateID() && @@ -791,6 +828,7 @@ public String toString() { ", bucketName='" + bucketName + "'" + ", acls=" + acls + ", isVersionEnabled=" + isVersionEnabled + + ", versioningStatus=" + versioningStatus + ", storageType=" + storageType + ", creationTime=" + creationTime + ", bekInfo=" + bekInfo + diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketArgs.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketArgs.java index 147255b3b573..cb24e5cbb571 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketArgs.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketArgs.java @@ -65,6 +65,32 @@ public void testQuotaIsSetFlagsAreCorrectlySet() { assertTrue(argsFromProto.hasQuotaInNamespace()); } + @Test + public void testVersioningStatusIsSetCorrectly() { + OmBucketArgs bucketArgs = OmBucketArgs.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .build(); + + OmBucketArgs argsFromProto = OmBucketArgs.getFromProtobuf( + bucketArgs.getProtobuf()); + + // absent means "not being changed" + assertNull(argsFromProto.getVersioningStatus()); + + bucketArgs = OmBucketArgs.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .setVersioningStatus(BucketVersioningStatus.SUSPENDED) + .build(); + + argsFromProto = OmBucketArgs.getFromProtobuf( + bucketArgs.getProtobuf()); + + assertEquals(BucketVersioningStatus.SUSPENDED, + argsFromProto.getVersioningStatus()); + } + @Test public void testDefaultReplicationConfigIsSetCorrectly() { OmBucketArgs bucketArgs = OmBucketArgs.newBuilder() diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java index 857103a20c0d..a089227c636a 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java @@ -29,6 +29,7 @@ import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.hdds.protocol.StorageType; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; @@ -54,6 +55,91 @@ public void protobufConversion() { OmBucketInfo.getFromProtobuf(bucket.getProtobuf())); } + @Test + public void versioningStatusDerivedFromLegacyFlag() { + // Records written before the versioningStatus field existed deserialize + // unchanged: the status is derived from the legacy isVersionEnabled flag. + OzoneManagerProtocolProtos.BucketInfo oldRecord = + OzoneManagerProtocolProtos.BucketInfo.newBuilder() + .setVolumeName("vol1") + .setBucketName("bucket") + .setIsVersionEnabled(false) + .setStorageType(HddsProtos.StorageTypeProto.DISK) + .build(); + OmBucketInfo bucket = OmBucketInfo.getFromProtobuf(oldRecord); + assertEquals(BucketVersioningStatus.UNVERSIONED, + bucket.getVersioningStatus()); + assertFalse(bucket.getIsVersionEnabled()); + assertEquals(bucket, OmBucketInfo.getFromProtobuf(bucket.getProtobuf())); + + oldRecord = oldRecord.toBuilder().setIsVersionEnabled(true).build(); + bucket = OmBucketInfo.getFromProtobuf(oldRecord); + assertEquals(BucketVersioningStatus.ENABLED, + bucket.getVersioningStatus()); + assertTrue(bucket.getIsVersionEnabled()); + assertEquals(bucket, OmBucketInfo.getFromProtobuf(bucket.getProtobuf())); + } + + @Test + public void versioningStatusProtobufConversion() { + // SUSPENDED is not representable by the legacy flag alone, so it must + // survive a proto round trip via the new field. + OmBucketInfo bucket = OmBucketInfo.newBuilder() + .setBucketName("bucket") + .setVolumeName("vol1") + .setVersioningStatus(BucketVersioningStatus.SUSPENDED) + .build(); + assertFalse(bucket.getIsVersionEnabled()); + + OmBucketInfo recovered = OmBucketInfo.getFromProtobuf(bucket.getProtobuf()); + assertEquals(BucketVersioningStatus.SUSPENDED, + recovered.getVersioningStatus()); + assertFalse(recovered.getIsVersionEnabled()); + assertEquals(bucket, recovered); + } + + @Test + public void builderKeepsVersioningStatusAndLegacyFlagInSync() { + OmBucketInfo.Builder builder = OmBucketInfo.newBuilder() + .setBucketName("bucket") + .setVolumeName("vol1"); + + // default is UNVERSIONED + assertEquals(BucketVersioningStatus.UNVERSIONED, + builder.build().getVersioningStatus()); + + // legacy true -> ENABLED + builder.setIsVersionEnabled(true); + assertEquals(BucketVersioningStatus.ENABLED, + builder.build().getVersioningStatus()); + assertTrue(builder.build().getIsVersionEnabled()); + + // explicit SUSPENDED forces the legacy flag to false + builder.setVersioningStatus(BucketVersioningStatus.SUSPENDED); + assertEquals(BucketVersioningStatus.SUSPENDED, + builder.build().getVersioningStatus()); + assertFalse(builder.build().getIsVersionEnabled()); + + // legacy false does not clobber an explicitly SUSPENDED status + builder.setIsVersionEnabled(false); + assertEquals(BucketVersioningStatus.SUSPENDED, + builder.build().getVersioningStatus()); + + // a null status is a no-op (records without the new field) + builder.setVersioningStatus(null); + assertEquals(BucketVersioningStatus.SUSPENDED, + builder.build().getVersioningStatus()); + + // legacy false on a never-enabled bucket stays UNVERSIONED + OmBucketInfo unversioned = OmBucketInfo.newBuilder() + .setBucketName("bucket") + .setVolumeName("vol1") + .setIsVersionEnabled(false) + .build(); + assertEquals(BucketVersioningStatus.UNVERSIONED, + unversioned.getVersioningStatus()); + } + @Test public void protobufConversionOfBucketLink() { OmBucketInfo bucket = OmBucketInfo.newBuilder() diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index bb8f54c79c56..bb9d3ebd433c 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -808,6 +808,9 @@ message BucketInfo { optional uint64 snapshotUsedNamespace = 22; // TODO: S3 bucket tags persisted in OM DB; set by PutBucketTagging, read by GetBucketTagging. repeated hadoop.hdds.KeyValue tags = 23; + // S3-compatible object versioning status. When absent, derived from + // isVersionEnabled (true -> VERSIONING_ENABLED, false -> UNVERSIONED). + optional BucketVersioningStatusProto versioningStatus = 24; } enum BucketLayoutProto { @@ -816,6 +819,17 @@ enum BucketLayoutProto { OBJECT_STORE = 3; } +/** + * S3-compatible bucket versioning state machine: + * UNVERSIONED -> VERSIONING_ENABLED <-> VERSIONING_SUSPENDED. + * Once enabled, a bucket can never return to UNVERSIONED. + */ +enum BucketVersioningStatusProto { + UNVERSIONED = 1; + VERSIONING_ENABLED = 2; + VERSIONING_SUSPENDED = 3; +} + /** * Cipher suite. */ @@ -883,6 +897,9 @@ message BucketArgs { optional BucketEncryptionInfoProto bekInfo = 12; // TODO: Tag payload for PutBucketTagging only. repeated hadoop.hdds.KeyValue tags = 13; + // S3-compatible object versioning status. Takes precedence over + // isVersionEnabled when both are set. + optional BucketVersioningStatusProto versioningStatus = 14; } message PrefixInfo { From d944440828a2e305b6e4b5ee85389e20ea5aa5f6 Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 16 Jul 2026 11:02:02 +0800 Subject: [PATCH 02/23] T1.2. Enforce bucket versioning state machine in OMBucketSetPropertyRequest Co-Authored-By: Claude Fable 5 --- .../bucket/OMBucketSetPropertyRequest.java | 28 ++++- .../TestOMBucketSetPropertyRequest.java | 107 ++++++++++++++++++ 2 files changed, 131 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index a88e5fb73334..b5d4f1fa9f21 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -38,6 +38,7 @@ import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext; import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo; +import org.apache.hadoop.ozone.om.helpers.BucketVersioningStatus; import org.apache.hadoop.ozone.om.helpers.KeyValueUtil; import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -174,10 +175,29 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut //Check Versioning to update Boolean versioning = omBucketArgs.getIsVersionEnabled(); - if (versioning != null) { - bucketInfoBuilder.setIsVersionEnabled(versioning); - LOG.debug("Updating bucket versioning for bucket: {} in volume: {}", - bucketName, volumeName); + BucketVersioningStatus newVersioningStatus = omBucketArgs.getVersioningStatus(); + if (newVersioningStatus == null && versioning != null) { + // Legacy flag from older clients: enabling always maps to ENABLED; + // disabling maps to SUSPENDED once versioning has ever been enabled + // (the S3 state machine forbids returning to UNVERSIONED). + if (versioning) { + newVersioningStatus = BucketVersioningStatus.ENABLED; + } else { + newVersioningStatus = + dbBucketInfo.getVersioningStatus() == BucketVersioningStatus.UNVERSIONED + ? BucketVersioningStatus.UNVERSIONED : BucketVersioningStatus.SUSPENDED; + } + } + if (newVersioningStatus != null) { + if (!dbBucketInfo.getVersioningStatus().canTransitionTo(newVersioningStatus)) { + throw new OMException("Bucket versioning cannot be changed from " + + dbBucketInfo.getVersioningStatus() + " to " + newVersioningStatus + + "; once enabled, versioning can only be suspended.", + OMException.ResultCodes.INVALID_REQUEST); + } + bucketInfoBuilder.setVersioningStatus(newVersioningStatus); + LOG.debug("Updating bucket versioning to {} for bucket: {} in volume: {}", + newVersioningStatus, bucketName, volumeName); } //Check quotaInBytes and quotaInNamespace to update diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java index 2e41d4c8b173..cab8f72e2845 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java @@ -33,6 +33,7 @@ import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo; import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.BucketVersioningStatus; import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; @@ -422,6 +423,112 @@ public void testValidateAndUpdateCacheWithQuotaNamespaceUsed() "is less than used namespaceQuota"); } + @Test + public void testVersioningStatusTransitions() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager); + String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName); + + assertEquals(BucketVersioningStatus.UNVERSIONED, + omMetadataManager.getBucketTable().get(bucketKey).getVersioningStatus()); + + // UNVERSIONED -> ENABLED + OMClientResponse response = new OMBucketSetPropertyRequest( + createSetVersioningStatusRequest(volumeName, bucketName, + BucketVersioningStatus.ENABLED)).validateAndUpdateCache(ozoneManager, 1); + assertTrue(response.getOMResponse().getSuccess()); + OmBucketInfo dbBucketInfo = omMetadataManager.getBucketTable().get(bucketKey); + assertEquals(BucketVersioningStatus.ENABLED, dbBucketInfo.getVersioningStatus()); + assertTrue(dbBucketInfo.getIsVersionEnabled()); + + // ENABLED -> SUSPENDED + response = new OMBucketSetPropertyRequest( + createSetVersioningStatusRequest(volumeName, bucketName, + BucketVersioningStatus.SUSPENDED)).validateAndUpdateCache(ozoneManager, 2); + assertTrue(response.getOMResponse().getSuccess()); + dbBucketInfo = omMetadataManager.getBucketTable().get(bucketKey); + assertEquals(BucketVersioningStatus.SUSPENDED, dbBucketInfo.getVersioningStatus()); + assertFalse(dbBucketInfo.getIsVersionEnabled()); + + // SUSPENDED -> UNVERSIONED is rejected + response = new OMBucketSetPropertyRequest( + createSetVersioningStatusRequest(volumeName, bucketName, + BucketVersioningStatus.UNVERSIONED)).validateAndUpdateCache(ozoneManager, 3); + assertFalse(response.getOMResponse().getSuccess()); + assertEquals(OzoneManagerProtocolProtos.Status.INVALID_REQUEST, + response.getOMResponse().getStatus()); + assertThat(response.getOMResponse().getMessage()) + .contains("once enabled, versioning can only be suspended"); + assertEquals(BucketVersioningStatus.SUSPENDED, + omMetadataManager.getBucketTable().get(bucketKey).getVersioningStatus()); + + // SUSPENDED -> ENABLED + response = new OMBucketSetPropertyRequest( + createSetVersioningStatusRequest(volumeName, bucketName, + BucketVersioningStatus.ENABLED)).validateAndUpdateCache(ozoneManager, 4); + assertTrue(response.getOMResponse().getSuccess()); + assertEquals(BucketVersioningStatus.ENABLED, + omMetadataManager.getBucketTable().get(bucketKey).getVersioningStatus()); + } + + @Test + public void testLegacyVersioningFlagMapsToStateMachine() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager); + String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName); + + // legacy false on a never-enabled bucket stays UNVERSIONED + OMClientResponse response = new OMBucketSetPropertyRequest( + createSetVersioningFlagRequest(volumeName, bucketName, false)) + .validateAndUpdateCache(ozoneManager, 1); + assertTrue(response.getOMResponse().getSuccess()); + assertEquals(BucketVersioningStatus.UNVERSIONED, + omMetadataManager.getBucketTable().get(bucketKey).getVersioningStatus()); + + // legacy true -> ENABLED + response = new OMBucketSetPropertyRequest( + createSetVersioningFlagRequest(volumeName, bucketName, true)) + .validateAndUpdateCache(ozoneManager, 2); + assertTrue(response.getOMResponse().getSuccess()); + assertEquals(BucketVersioningStatus.ENABLED, + omMetadataManager.getBucketTable().get(bucketKey).getVersioningStatus()); + + // legacy false after enabling -> SUSPENDED, not UNVERSIONED + response = new OMBucketSetPropertyRequest( + createSetVersioningFlagRequest(volumeName, bucketName, false)) + .validateAndUpdateCache(ozoneManager, 3); + assertTrue(response.getOMResponse().getSuccess()); + OmBucketInfo dbBucketInfo = omMetadataManager.getBucketTable().get(bucketKey); + assertEquals(BucketVersioningStatus.SUSPENDED, dbBucketInfo.getVersioningStatus()); + assertFalse(dbBucketInfo.getIsVersionEnabled()); + } + + private OMRequest createSetVersioningStatusRequest(String volumeName, + String bucketName, BucketVersioningStatus status) { + return OMRequest.newBuilder().setSetBucketPropertyRequest( + SetBucketPropertyRequest.newBuilder().setBucketArgs( + BucketArgs.newBuilder().setBucketName(bucketName) + .setVolumeName(volumeName) + .setVersioningStatus(status.toProto()).build())) + .setCmdType(OzoneManagerProtocolProtos.Type.SetBucketProperty) + .setClientId(UUID.randomUUID().toString()).build(); + } + + private OMRequest createSetVersioningFlagRequest(String volumeName, + String bucketName, boolean isVersionEnabled) { + return OMRequest.newBuilder().setSetBucketPropertyRequest( + SetBucketPropertyRequest.newBuilder().setBucketArgs( + BucketArgs.newBuilder().setBucketName(bucketName) + .setVolumeName(volumeName) + .setIsVersionEnabled(isVersionEnabled).build())) + .setCmdType(OzoneManagerProtocolProtos.Type.SetBucketProperty) + .setClientId(UUID.randomUUID().toString()).build(); + } + @Test public void testSettingQuotaRetainsReplication() throws Exception { String volumeName1 = UUID.randomUUID().toString(); From 7c484c24292e638dde11e764ee0e20001d16f8ba Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 16 Jul 2026 11:02:17 +0800 Subject: [PATCH 03/23] T1.3. Add versionId, isDeleteMarker and isNullVersion to OmKeyInfo Co-Authored-By: Claude Fable 5 --- .../hadoop/ozone/om/helpers/OmKeyInfo.java | 68 +++++++++++++++++++ .../ozone/om/helpers/TestOmKeyInfo.java | 28 ++++++++ .../src/main/proto/OmClientProtocol.proto | 9 +++ 3 files changed, 105 insertions(+) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java index ab4da4badd90..da92da49d5db 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java @@ -112,6 +112,15 @@ public final class OmKeyInfo extends WithParentObjectId // been modified. private final Long expectedDataGeneration; + // S3-compatible object versioning. versionId is assigned once from the + // committing transaction's index when a version is created, then frozen; + // absent on records that predate versioning support (treated as the null + // version). A delete marker has isDeleteMarker set and no data blocks. + // isNullVersion marks the single overwritable "null version" slot per key. + private final Long versionId; + private final boolean isDeleteMarker; + private final boolean isNullVersion; + private OmKeyInfo(Builder b) { super(b); this.volumeName = b.volumeName; @@ -130,6 +139,9 @@ private OmKeyInfo(Builder b) { this.ownerName = b.ownerName; this.tags = b.tags.build(); this.expectedDataGeneration = b.expectedDataGeneration; + this.versionId = b.versionId; + this.isDeleteMarker = b.isDeleteMarker; + this.isNullVersion = b.isNullVersion; } /** @@ -469,6 +481,22 @@ public FileChecksum getFileChecksum() { return fileChecksum; } + /** + * @return the object version identity, or null for records that predate + * versioning support (treated as the null version). + */ + public Long getVersionId() { + return versionId; + } + + public boolean isDeleteMarker() { + return isDeleteMarker; + } + + public boolean isNullVersion() { + return isNullVersion; + } + @Override public String toString() { return "OmKeyInfo{" + @@ -511,6 +539,9 @@ public static class Builder extends WithParentObjectId.Builder { private boolean isFile; private final MapBuilder tags; private Long expectedDataGeneration = null; + private Long versionId = null; + private boolean isDeleteMarker; + private boolean isNullVersion; public Builder() { this.acls = AclListBuilder.empty(); @@ -533,6 +564,9 @@ public Builder(OmKeyInfo obj) { this.fileChecksum = obj.fileChecksum; this.isFile = obj.isFile; this.expectedDataGeneration = obj.expectedDataGeneration; + this.versionId = obj.versionId; + this.isDeleteMarker = obj.isDeleteMarker; + this.isNullVersion = obj.isNullVersion; this.tags = MapBuilder.of(obj.tags); obj.keyLocationVersions.forEach(keyLocationVersion -> this.omKeyLocationInfoGroups.add( @@ -704,6 +738,21 @@ public Builder setExpectedDataGeneration(Long existingGeneration) { return this; } + public Builder setVersionId(Long versionId) { + this.versionId = versionId; + return this; + } + + public Builder setDeleteMarker(boolean deleteMarker) { + this.isDeleteMarker = deleteMarker; + return this; + } + + public Builder setNullVersion(boolean nullVersion) { + this.isNullVersion = nullVersion; + return this; + } + @Override protected void validate() { super.validate(); @@ -855,6 +904,16 @@ private KeyInfo getProtobuf(boolean ignorePipeline, String fullKeyName, if (ownerName != null) { kb.setOwnerName(ownerName); } + if (versionId != null) { + kb.setVersionId(versionId); + } + // only persisted when set, to keep records without versioning unchanged + if (isDeleteMarker) { + kb.setIsDeleteMarker(true); + } + if (isNullVersion) { + kb.setIsNullVersion(true); + } return kb.build(); } @@ -909,6 +968,15 @@ public static Builder builderFromProtobuf(KeyInfo keyInfo) { if (keyInfo.hasOwnerName()) { builder.setOwnerName(keyInfo.getOwnerName()); } + if (keyInfo.hasVersionId()) { + builder.setVersionId(keyInfo.getVersionId()); + } + if (keyInfo.hasIsDeleteMarker()) { + builder.setDeleteMarker(keyInfo.getIsDeleteMarker()); + } + if (keyInfo.hasIsNullVersion()) { + builder.setNullVersion(keyInfo.getIsNullVersion()); + } return builder; } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java index 285853a3a766..50c3764e5e80 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java @@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; @@ -116,6 +117,33 @@ public void getProtobufMessageEC() throws IOException { assertEquals(2, config.getParity()); } + @Test + public void protobufConversionWithVersioningFields() { + // records without versioning fields keep them unset after a round trip + OmKeyInfo key = createOmKeyInfo( + RatisReplicationConfig.getInstance(ReplicationFactor.THREE)); + OzoneManagerProtocolProtos.KeyInfo proto = key.getProtobuf(ClientVersion.CURRENT_VERSION); + assertFalse(proto.hasVersionId()); + assertFalse(proto.hasIsDeleteMarker()); + assertFalse(proto.hasIsNullVersion()); + OmKeyInfo recovered = OmKeyInfo.getFromProtobuf(proto); + assertNull(recovered.getVersionId()); + assertFalse(recovered.isDeleteMarker()); + assertFalse(recovered.isNullVersion()); + + // versioning fields survive a round trip and the copy constructor + key = createOmKeyInfo(RatisReplicationConfig.getInstance(ReplicationFactor.THREE)) + .toBuilder() + .setVersionId(4242L) + .setDeleteMarker(true) + .setNullVersion(true) + .build(); + recovered = OmKeyInfo.getFromProtobuf(key.getProtobuf(ClientVersion.CURRENT_VERSION)); + assertEquals(4242L, recovered.getVersionId()); + assertTrue(recovered.isDeleteMarker()); + assertTrue(recovered.isNullVersion()); + } + private OmKeyInfo createOmKeyInfo(ReplicationConfig replicationConfig) { return new Builder() .setKeyName("key1") diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index bb9d3ebd433c..9e9a658c622a 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1234,6 +1234,15 @@ message KeyInfo { // This allows a key to be created an committed atomically if the original has not // been modified. optional uint64 expectedDataGeneration = 22; + // S3-compatible object versioning fields. versionId identifies an object + // version: assigned once from the committing transaction's index when the + // version is created, then frozen. A delete marker is a record with + // isDeleteMarker set and no data blocks. isNullVersion marks the single + // overwritable "null version" slot per key (writes while versioning is + // suspended, or objects that predate enabling versioning). + optional uint64 versionId = 23; + optional bool isDeleteMarker = 24; + optional bool isNullVersion = 25; } // KeyInfoProtoLight is a lightweight subset of KeyInfo message containing From 89c02bb9f271f62e3447d1aa82c3cd7db2100b0c Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 16 Jul 2026 11:02:26 +0800 Subject: [PATCH 04/23] T1.4. Add versionedKeyTable column family for noncurrent object versions Co-Authored-By: Claude Fable 5 --- .../hadoop/ozone/om/OMMetadataManager.java | 24 +++++++++++++++++++ .../ozone/om/OmMetadataManagerImpl.java | 18 ++++++++++++++ .../hadoop/ozone/om/codec/OMDBDefinition.java | 15 ++++++++++++ .../ozone/om/TestOmMetadataManager.java | 19 +++++++++++++++ 4 files changed, 76 insertions(+) diff --git a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java index be66ffc195b5..f21f94f5fc7e 100644 --- a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java +++ b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java @@ -172,6 +172,22 @@ public interface OMMetadataManager extends DBStoreHAManager, AutoCloseable { */ String getOzoneKey(String volume, String bucket, String key); + /** + * Given a volume, bucket, key and versionId, return the corresponding + * versionedKeyTable DB key: the versionId is appended as fixed-width hex of + * (Long.MAX_VALUE - versionId), so all versions of a key are adjacent and + * ordered newest first. + */ + String getVersionedOzoneKey(String volume, String bucket, String key, long versionId); + + /** + * Prefix under which all noncurrent versions of the given key are stored in + * the versionedKeyTable. Since key names may themselves contain the + * separator, iterating consumers must check that the remainder after this + * prefix is exactly one fixed-width versionId suffix. + */ + String getVersionedOzoneKeyPrefix(String volume, String bucket, String key); + /** * Get DB key for a key or prefix in an FSO bucket given existing * volume and bucket names. @@ -405,6 +421,14 @@ List getExpiredMultipartUploads( Table getKeyTable(BucketLayout bucketLayout); + /** + * Returns the versionedKeyTable holding noncurrent object versions + * (including noncurrent delete markers) of versioning-enabled buckets. + * + * @return versionedKeyTable. + */ + Table getVersionedKeyTable(); + /** * Returns the FileTable. * diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index 283bb4933580..bd1ff7c48aec 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -159,6 +159,7 @@ public class OmMetadataManagerImpl implements OMMetadataManager, private Table volumeTable; private Table bucketTable; private Table keyTable; + private Table versionedKeyTable; private Table openKeyTable; private Table multipartInfoTable; @@ -376,6 +377,11 @@ public Table getKeyTable(BucketLayout bucketLayout) { return keyTable; } + @Override + public Table getVersionedKeyTable() { + return versionedKeyTable; + } + @Override public Table getFileTable() { return fileTable; @@ -494,6 +500,7 @@ protected void initializeOmTables(CacheType cacheType, volumeTable = initializer.get(OMDBDefinition.VOLUME_TABLE_DEF, cacheType); bucketTable = initializer.get(OMDBDefinition.BUCKET_TABLE_DEF, cacheType); keyTable = initializer.get(OMDBDefinition.KEY_TABLE_DEF); + versionedKeyTable = initializer.get(OMDBDefinition.VERSIONED_KEY_TABLE_DEF); openKeyTable = initializer.get(OMDBDefinition.OPEN_KEY_TABLE_DEF); multipartInfoTable = initializer.get(OMDBDefinition.MULTIPART_INFO_TABLE_DEF); @@ -649,6 +656,17 @@ public String getOzoneKey(String volume, String bucket, String key) { return builder.toString(); } + @Override + public String getVersionedOzoneKey(String volume, String bucket, String key, long versionId) { + return getVersionedOzoneKeyPrefix(volume, bucket, key) + + String.format("%016x", Long.MAX_VALUE - versionId); + } + + @Override + public String getVersionedOzoneKeyPrefix(String volume, String bucket, String key) { + return getOzoneKey(volume, bucket, key) + OM_KEY_PREFIX; + } + @Override public String getOzoneKeyFSO(String volumeName, String bucketName, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java index 02e32edec464..933c86d2f9f7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java @@ -86,6 +86,7 @@ * | Column Family | Mapping | * |----------------------------------------------------------------------------------| * | keyTable | /volume/bucket/key :- KeyInfo | + * | versionedKeyTable | /volume/bucket/key/revVersionId :- KeyInfo | * | deletedTable | /volume/bucket/key :- RepeatedKeyInfo | * | openKeyTable | /volume/bucket/key/id :- KeyInfo | * | multipartInfoTable | /volume/bucket/key/uploadId :- parts | @@ -210,6 +211,19 @@ public final class OMDBDefinition extends DBDefinition.WithMap { StringCodec.get(), OmKeyInfo.getKeyTableCodec()); + public static final String VERSIONED_KEY_TABLE = "versionedKeyTable"; + /** + * versionedKeyTable: /volume/bucket/key/revVersionId :- KeyInfo. + * Noncurrent object versions (including noncurrent delete markers) of + * versioning-enabled buckets; the current version stays in keyTable. + * revVersionId is the fixed-width hex of (Long.MAX_VALUE - versionId), so + * versions of a key are adjacent and ordered newest first. + */ + public static final DBColumnFamilyDefinition VERSIONED_KEY_TABLE_DEF + = new DBColumnFamilyDefinition<>(VERSIONED_KEY_TABLE, + StringCodec.get(), + OmKeyInfo.getKeyTableCodec()); + public static final String DELETED_TABLE = "deletedTable"; /** deletedTable: /volume/bucket/key :- RepeatedKeyInfo (excludes fields only used in openKeyTable). */ public static final DBColumnFamilyDefinition DELETED_TABLE_DEF @@ -353,6 +367,7 @@ public final class OMDBDefinition extends DBDefinition.WithMap { TENANT_STATE_TABLE_DEF, TRANSACTION_INFO_TABLE_DEF, USER_TABLE_DEF, + VERSIONED_KEY_TABLE_DEF, VOLUME_TABLE_DEF); private static final OMDBDefinition INSTANCE = new OMDBDefinition(); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index ec241f9dcb3e..f506c5a41f3d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -46,6 +46,7 @@ import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.TENANT_STATE_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.TRANSACTION_INFO_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.USER_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VERSIONED_KEY_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND; @@ -120,6 +121,7 @@ public class TestOmMetadataManager { VOLUME_TABLE, BUCKET_TABLE, KEY_TABLE, + VERSIONED_KEY_TABLE, DELETED_TABLE, OPEN_KEY_TABLE, MULTIPART_INFO_TABLE, @@ -172,6 +174,23 @@ public void testTransactionTable() throws Exception { assertEquals(250, transactionInfo.getTransactionIndex()); } + @Test + public void testVersionedOzoneKeyOrdering() { + String prefix = omMetadataManager.getVersionedOzoneKeyPrefix("vol", "buck", "key"); + assertEquals("/vol/buck/key/", prefix); + + // newer versions (larger versionId) must sort before older ones, and all + // versioned keys must sort under the key's prefix + String v1 = omMetadataManager.getVersionedOzoneKey("vol", "buck", "key", 1L); + String v2 = omMetadataManager.getVersionedOzoneKey("vol", "buck", "key", 42L); + String v3 = omMetadataManager.getVersionedOzoneKey("vol", "buck", "key", Long.MAX_VALUE - 1); + assertThat(v3).startsWith(prefix).isLessThan(v2); + assertThat(v2).startsWith(prefix).isLessThan(v1); + + // fixed-width suffix: identical length regardless of versionId magnitude + assertEquals(v1.length(), v3.length()); + } + @Test public void testListVolumes() throws Exception { String ownerName = "owner"; From f74c3318223dd79ef69b708eb000a38defee22f1 Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 16 Jul 2026 11:04:20 +0800 Subject: [PATCH 05/23] T1.5. Gate object versioning behind OMLayoutFeature.OBJECT_VERSIONING Co-Authored-By: Claude Fable 5 --- .../bucket/OMBucketSetPropertyRequest.java | 23 +++++++++ .../ozone/om/upgrade/OMLayoutFeature.java | 5 +- .../TestOMBucketSetPropertyRequest.java | 49 +++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index b5d4f1fa9f21..2a8f8da52071 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -396,4 +396,27 @@ public static OMRequest disallowSetBucketPropertyWithECReplicationConfig( } return req; } + + @RequestFeatureValidator( + conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION, + processingPhase = RequestProcessingPhase.PRE_PROCESS, + requestType = Type.SetBucketProperty + ) + public static OMRequest disallowSetBucketPropertyWithVersioningStatus( + OMRequest req, ValidationContext ctx) throws OMException { + if (!ctx.versionManager() + .isAllowed(OMLayoutFeature.OBJECT_VERSIONING)) { + SetBucketPropertyRequest propReq = + req.getSetBucketPropertyRequest(); + if (propReq.hasBucketArgs() + && propReq.getBucketArgs().hasVersioningStatus()) { + throw new OMException("Cluster does not have the object versioning" + + " feature finalized yet, but the request contains a bucket" + + " versioning status. Rejecting the request, please finalize the" + + " cluster upgrade and then try again.", + OMException.ResultCodes.NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION); + } + } + return req; + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java index ef99b453b7f0..b832d6efc9ec 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java @@ -44,7 +44,10 @@ public enum OMLayoutFeature implements LayoutFeature { QUOTA(6, "Ozone quota re-calculate"), HBASE_SUPPORT(7, "Full support of hsync, lease recovery and listOpenFiles APIs for HBase"), DELEGATION_TOKEN_SYMMETRIC_SIGN(8, "Delegation token signed by symmetric key"), - SNAPSHOT_DEFRAG(9, "Supporting defragmentation of snapshot"); + SNAPSHOT_DEFRAG(9, "Supporting defragmentation of snapshot"), + + OBJECT_VERSIONING(10, "S3-compatible object versioning: bucket versioning" + + " state machine and the versionedKeyTable for noncurrent versions"); /////////////////////////////// ///////////////////////////// diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java index cab8f72e2845..be5bf3375fab 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java @@ -24,20 +24,28 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.util.UUID; import org.apache.hadoop.hdds.client.DefaultReplicationConfig; import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.BucketVersioningStatus; import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; +import org.apache.hadoop.ozone.om.request.validation.ValidationContext; import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature; +import org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.BucketArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; @@ -507,6 +515,47 @@ public void testLegacyVersioningFlagMapsToStateMachine() throws Exception { assertFalse(dbBucketInfo.getIsVersionEnabled()); } + @Test + public void testVersioningStatusRejectedBeforeFinalization() + throws Exception { + OMRequest request = createSetVersioningStatusRequest( + UUID.randomUUID().toString(), UUID.randomUUID().toString(), + BucketVersioningStatus.ENABLED); + + OMLayoutVersionManager preFinalizedVersionManager = + mock(OMLayoutVersionManager.class); + when(preFinalizedVersionManager + .isAllowed(OMLayoutFeature.OBJECT_VERSIONING)).thenReturn(false); + ValidationContext preFinalizedContext = ValidationContext.of( + preFinalizedVersionManager, omMetadataManager); + + OMException omException = assertThrows(OMException.class, + () -> OMBucketSetPropertyRequest + .disallowSetBucketPropertyWithVersioningStatus( + request, preFinalizedContext)); + assertEquals(OMException.ResultCodes + .NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION, + omException.getResult()); + + // requests without a versioningStatus pass through untouched + OMRequest legacyRequest = createSetVersioningFlagRequest( + UUID.randomUUID().toString(), UUID.randomUUID().toString(), true); + assertSame(legacyRequest, OMBucketSetPropertyRequest + .disallowSetBucketPropertyWithVersioningStatus( + legacyRequest, preFinalizedContext)); + + // after finalization the request passes through untouched + OMLayoutVersionManager finalizedVersionManager = + mock(OMLayoutVersionManager.class); + when(finalizedVersionManager + .isAllowed(OMLayoutFeature.OBJECT_VERSIONING)).thenReturn(true); + ValidationContext finalizedContext = ValidationContext.of( + finalizedVersionManager, omMetadataManager); + assertSame(request, OMBucketSetPropertyRequest + .disallowSetBucketPropertyWithVersioningStatus( + request, finalizedContext)); + } + private OMRequest createSetVersioningStatusRequest(String volumeName, String bucketName, BucketVersioningStatus status) { return OMRequest.newBuilder().setSetBucketPropertyRequest( From 37c33966f562a6b4804992edf224bc851c148e11 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 17 Jul 2026 10:16:00 +0800 Subject: [PATCH 06/23] T1. Address comments Co-Authored-By: Claude Fable 5 --- .../main/java/org/apache/hadoop/ozone/OzoneConsts.java | 1 + .../org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java | 3 +++ .../org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java | 1 + .../org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java | 8 +++++++- .../org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java | 7 +++++++ .../java/org/apache/hadoop/ozone/om/OzoneManager.java | 1 + 6 files changed, 20 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 3b63ecf19747..ce00fa4c9304 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -299,6 +299,7 @@ public final class OzoneConsts { public static final String STORAGE_TYPE = "storageType"; public static final String RESOURCE_TYPE = "resourceType"; public static final String IS_VERSION_ENABLED = "isVersionEnabled"; + public static final String VERSIONING_STATUS = "versioningStatus"; public static final String CREATION_TIME = "creationTime"; public static final String MODIFICATION_TIME = "modificationTime"; public static final String DATA_SIZE = "dataSize"; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java index 6b898a27e47f..d6cfa9b6f2c0 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java @@ -204,6 +204,9 @@ public Map toAuditMap() { getMetadata().get(OzoneConsts.GDPR_FLAG)); auditMap.put(OzoneConsts.IS_VERSION_ENABLED, String.valueOf(this.isVersionEnabled)); + if (this.versioningStatus != null) { + auditMap.put(OzoneConsts.VERSIONING_STATUS, this.versioningStatus.name()); + } if (this.storageType != null) { auditMap.put(OzoneConsts.STORAGE_TYPE, this.storageType.name()); } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java index 0431b1eb67a7..de320d3410ca 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java @@ -352,6 +352,7 @@ public Map toAuditMap() { (this.acls != null) ? this.acls.toString() : null); auditMap.put(OzoneConsts.IS_VERSION_ENABLED, String.valueOf(this.isVersionEnabled)); + auditMap.put(OzoneConsts.VERSIONING_STATUS, this.versioningStatus.name()); auditMap.put(OzoneConsts.STORAGE_TYPE, (this.storageType != null) ? this.storageType.name() : null); auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime)); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java index da92da49d5db..65540af1284a 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java @@ -513,6 +513,9 @@ public String toString() { ", isFile=" + isFile + ", fileName='" + fileName + '\'' + ", acls=" + acls + + ", versionId=" + versionId + + ", isDeleteMarker=" + isDeleteMarker + + ", isNullVersion=" + isNullVersion + '}'; } @@ -1014,7 +1017,10 @@ public boolean isKeyInfoSame(OmKeyInfo omKeyInfo, boolean checkPath, Objects.equals(getMetadata(), omKeyInfo.getMetadata()) && Objects.equals(acls, omKeyInfo.acls) && Objects.equals(getTags(), omKeyInfo.getTags()) && - getObjectID() == omKeyInfo.getObjectID(); + getObjectID() == omKeyInfo.getObjectID() && + Objects.equals(versionId, omKeyInfo.versionId) && + isDeleteMarker == omKeyInfo.isDeleteMarker && + isNullVersion == omKeyInfo.isNullVersion; if (isEqual && checkUpdateID) { isEqual = getUpdateID() == omKeyInfo.getUpdateID(); diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java index 50c3764e5e80..12846bece1c4 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java @@ -142,6 +142,13 @@ public void protobufConversionWithVersioningFields() { assertEquals(4242L, recovered.getVersionId()); assertTrue(recovered.isDeleteMarker()); assertTrue(recovered.isNullVersion()); + + // records differing only in a versioning field must not compare equal + OmKeyInfo plain = createOmKeyInfo( + RatisReplicationConfig.getInstance(ReplicationFactor.THREE)); + assertNotEquals(plain, plain.toBuilder().setVersionId(1L).build()); + assertNotEquals(plain, plain.toBuilder().setDeleteMarker(true).build()); + assertNotEquals(plain, plain.toBuilder().setNullVersion(true).build()); } private OmKeyInfo createOmKeyInfo(ReplicationConfig replicationConfig) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 5d1e33f7cd84..4d9777893146 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -3060,6 +3060,7 @@ public OmBucketInfo getBucketInfo(String volume, String bucket) .setDefaultReplicationConfig( realBucket.getDefaultReplicationConfig()) .setIsVersionEnabled(realBucket.getIsVersionEnabled()) + .setVersioningStatus(realBucket.getVersioningStatus()) .setStorageType(realBucket.getStorageType()) .setQuotaInBytes(realBucket.getQuotaInBytes()) .setQuotaInNamespace(realBucket.getQuotaInNamespace()) From e89e8beeeb5ee92c1b4903ef38c4f829017054cb Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 31 Jul 2026 10:27:58 +0800 Subject: [PATCH 07/23] T1. Use a 0x00 separator in versionedKeyTable dbKeys Key names in OBJECT_STORE buckets contain '/' verbatim, so a '/' separator interleaves a key's versions with those of keys nested under it, breaking the single-seek promotion and the merged ListObjectVersions order. Co-Authored-By: Claude Opus 5 --- .../org/apache/hadoop/ozone/OzoneConsts.java | 9 ++++++++ .../hadoop/ozone/om/OMMetadataManager.java | 8 ++++--- .../ozone/om/OmMetadataManagerImpl.java | 3 ++- .../hadoop/ozone/om/codec/OMDBDefinition.java | 8 ++++--- .../ozone/om/TestOmMetadataManager.java | 21 ++++++++++++++++++- 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index ce00fa4c9304..9c6a6f00e1d8 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -180,6 +180,15 @@ public final class OzoneConsts { public static final String OM_KEY_PREFIX = "/"; public static final String DOUBLE_SLASH_OM_KEY_PREFIX = "//"; + /** + * Separates a key name from the versionId suffix in versionedKeyTable DB keys. + * OM_KEY_PREFIX cannot be used: OBJECT_STORE key names contain '/' verbatim, so + * a key's versions would interleave with the versions of keys nested under it. + * 0x00 is the minimum byte value, so {keyName} + this separator can never be a + * prefix of {keyName} + "/". Written as an octal escape because a literal + * unicode escape for NUL is expanded by the Java lexer before parsing. + */ + public static final String OM_VERSIONED_KEY_SEPARATOR = "\0"; public static final String OM_USER_PREFIX = "$"; public static final String OM_S3_PREFIX = "S3:"; public static final String OM_S3_CALLER_CONTEXT_PREFIX = "S3Auth:S3G|"; diff --git a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java index f21f94f5fc7e..ad2450e8e75f 100644 --- a/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java +++ b/hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java @@ -182,9 +182,11 @@ public interface OMMetadataManager extends DBStoreHAManager, AutoCloseable { /** * Prefix under which all noncurrent versions of the given key are stored in - * the versionedKeyTable. Since key names may themselves contain the - * separator, iterating consumers must check that the remainder after this - * prefix is exactly one fixed-width versionId suffix. + * the versionedKeyTable. The key name is separated from the versionId suffix + * by OM_VERSIONED_KEY_SEPARATOR rather than OM_KEY_PREFIX, so that a key's + * versions stay contiguous under this prefix and sort before any key nested + * under it: seeking this prefix yields exactly that key's versions, newest + * first. */ String getVersionedOzoneKeyPrefix(String volume, String bucket, String key); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index bd1ff7c48aec..18f213df0884 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.OM_DB_NAME; import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_CHECKPOINT_DIR; +import static org.apache.hadoop.ozone.OzoneConsts.OM_VERSIONED_KEY_SEPARATOR; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_DB_MAX_OPEN_FILES; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_DB_MAX_OPEN_FILES_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DB_MAX_OPEN_FILES; @@ -664,7 +665,7 @@ public String getVersionedOzoneKey(String volume, String bucket, String key, lon @Override public String getVersionedOzoneKeyPrefix(String volume, String bucket, String key) { - return getOzoneKey(volume, bucket, key) + OM_KEY_PREFIX; + return getOzoneKey(volume, bucket, key) + OM_VERSIONED_KEY_SEPARATOR; } @Override diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java index 933c86d2f9f7..49d24f9d6138 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java @@ -86,7 +86,7 @@ * | Column Family | Mapping | * |----------------------------------------------------------------------------------| * | keyTable | /volume/bucket/key :- KeyInfo | - * | versionedKeyTable | /volume/bucket/key/revVersionId :- KeyInfo | + * | versionedKeyTable | /volume/bucket/key\0revVersionId :- KeyInfo | * | deletedTable | /volume/bucket/key :- RepeatedKeyInfo | * | openKeyTable | /volume/bucket/key/id :- KeyInfo | * | multipartInfoTable | /volume/bucket/key/uploadId :- parts | @@ -213,11 +213,13 @@ public final class OMDBDefinition extends DBDefinition.WithMap { public static final String VERSIONED_KEY_TABLE = "versionedKeyTable"; /** - * versionedKeyTable: /volume/bucket/key/revVersionId :- KeyInfo. + * versionedKeyTable: /volume/bucket/key\0revVersionId :- KeyInfo. * Noncurrent object versions (including noncurrent delete markers) of * versioning-enabled buckets; the current version stays in keyTable. * revVersionId is the fixed-width hex of (Long.MAX_VALUE - versionId), so - * versions of a key are adjacent and ordered newest first. + * versions of a key are adjacent and ordered newest first. The separator is + * OM_VERSIONED_KEY_SEPARATOR (0x00), not '/', because OBJECT_STORE key names + * contain '/' verbatim. */ public static final DBColumnFamilyDefinition VERSIONED_KEY_TABLE_DEF = new DBColumnFamilyDefinition<>(VERSIONED_KEY_TABLE, diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java index f506c5a41f3d..d321dff0a81d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmMetadataManager.java @@ -177,7 +177,7 @@ public void testTransactionTable() throws Exception { @Test public void testVersionedOzoneKeyOrdering() { String prefix = omMetadataManager.getVersionedOzoneKeyPrefix("vol", "buck", "key"); - assertEquals("/vol/buck/key/", prefix); + assertEquals("/vol/buck/key\0", prefix); // newer versions (larger versionId) must sort before older ones, and all // versioned keys must sort under the key's prefix @@ -191,6 +191,25 @@ public void testVersionedOzoneKeyOrdering() { assertEquals(v1.length(), v3.length()); } + @Test + public void testVersionedOzoneKeyIsolatedFromNestedKeys() { + // OBJECT_STORE key names contain '/' verbatim, so "key" and "key/001" are two + // unrelated keys. Every version of "key" must sort under "key"'s prefix and + // ahead of anything belonging to "key/001", otherwise a prefix seek for the + // newest noncurrent version of "key" would land on a version of "key/001". + String prefix = omMetadataManager.getVersionedOzoneKeyPrefix("vol", "buck", "key"); + String nestedPrefix = omMetadataManager.getVersionedOzoneKeyPrefix("vol", "buck", "key/001"); + assertThat(nestedPrefix).doesNotStartWith(prefix); + + String oldest = omMetadataManager.getVersionedOzoneKey("vol", "buck", "key", 1L); + String nestedNewest = + omMetadataManager.getVersionedOzoneKey("vol", "buck", "key/001", Long.MAX_VALUE - 1); + assertThat(oldest).isLessThan(nestedNewest); + + // the nested key's own current entry in keyTable also sorts after all of them + assertThat(oldest).isLessThan(omMetadataManager.getOzoneKey("vol", "buck", "key/001")); + } + @Test public void testListVolumes() throws Exception { String ownerName = "owner"; From 3be795609f6d2918fa9ec3287f476071691e8f43 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 31 Jul 2026 15:07:02 +0800 Subject: [PATCH 08/23] T1. Do not derive a versioning status from the legacy flag --- .../om/helpers/BucketVersioningStatus.java | 5 -- .../hadoop/ozone/om/helpers/OmBucketInfo.java | 51 ++++++++++++++----- .../ozone/om/helpers/TestOmBucketInfo.java | 42 +++++++-------- .../apache/hadoop/ozone/om/OzoneManager.java | 6 ++- .../bucket/OMBucketSetPropertyRequest.java | 28 ++++++---- .../TestOMBucketSetPropertyRequest.java | 23 ++++++--- 6 files changed, 97 insertions(+), 58 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java index 424166bb473f..f99a9bb3fbca 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BucketVersioningStatus.java @@ -52,11 +52,6 @@ public BucketVersioningStatusProto toProto() { } } - /** Maps the legacy isVersionEnabled flag of buckets without an explicit status. */ - public static BucketVersioningStatus fromVersionEnabledFlag(boolean isVersionEnabled) { - return isVersionEnabled ? ENABLED : UNVERSIONED; - } - /** The legacy isVersionEnabled flag value kept in sync with this status. */ public boolean toVersionEnabledFlag() { return this == ENABLED; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java index de320d3410ca..5968bff7a9d3 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java @@ -122,9 +122,14 @@ private OmBucketInfo(Builder b) { this.volumeName = b.volumeName; this.bucketName = b.bucketName; this.acls = b.acls.build(); - this.versioningStatus = b.versioningStatus != null ? b.versioningStatus - : BucketVersioningStatus.fromVersionEnabledFlag(b.isVersionEnabled); - this.isVersionEnabled = this.versioningStatus.toVersionEnabledFlag(); + // A null versioningStatus means the bucket carries no S3 versioning status + // at all, mirroring the optional proto field: such a bucket is driven by the + // legacy isVersionEnabled flag alone. The flag is derived from the status + // only when a status was actually set, so that a legacy client enabling + // versioning does not silently opt an existing bucket into S3 versioning. + this.versioningStatus = b.versioningStatus; + this.isVersionEnabled = b.versioningStatus != null + ? b.versioningStatus.toVersionEnabledFlag() : b.isVersionEnabled; this.storageType = b.storageType; this.creationTime = b.creationTime; this.modificationTime = b.modificationTime; @@ -180,11 +185,24 @@ public boolean getIsVersionEnabled() { } /** - * Returns the S3-compatible versioning status; never null. + * Returns the S3-compatible versioning status; never null. A bucket that only + * carries the legacy isVersionEnabled flag is UNVERSIONED as far as S3 + * versioning is concerned: the legacy flag selects the in-record block version + * list, which is a different feature. * @return BucketVersioningStatus */ public BucketVersioningStatus getVersioningStatus() { - return versioningStatus; + return versioningStatus != null + ? versioningStatus : BucketVersioningStatus.UNVERSIONED; + } + + /** + * Whether an S3 versioning status was explicitly set on this bucket, as + * opposed to the bucket carrying only the legacy isVersionEnabled flag. + * @return whether the optional status field is present + */ + public boolean hasVersioningStatus() { + return versioningStatus != null; } /** @@ -352,7 +370,8 @@ public Map toAuditMap() { (this.acls != null) ? this.acls.toString() : null); auditMap.put(OzoneConsts.IS_VERSION_ENABLED, String.valueOf(this.isVersionEnabled)); - auditMap.put(OzoneConsts.VERSIONING_STATUS, this.versioningStatus.name()); + auditMap.put(OzoneConsts.VERSIONING_STATUS, + this.versioningStatus != null ? this.versioningStatus.name() : null); auditMap.put(OzoneConsts.STORAGE_TYPE, (this.storageType != null) ? this.storageType.name() : null); auditMap.put(OzoneConsts.CREATION_TIME, String.valueOf(this.creationTime)); @@ -477,15 +496,14 @@ public Builder addAcl(OzoneAcl ozoneAcl) { return this; } + /** + * Sets the legacy flag only. It deliberately does not derive a + * versioningStatus: a legacy client enabling versioning must not opt the + * bucket into S3 versioning semantics. Deriving the status is the job of + * OMBucketSetPropertyRequest, where an actual state transition is requested. + */ public Builder setIsVersionEnabled(boolean versionFlag) { this.isVersionEnabled = versionFlag; - // Keep versioningStatus in sync for callers that only know the legacy - // flag; an explicitly SUSPENDED status is preserved on disable. - if (versionFlag) { - this.versioningStatus = BucketVersioningStatus.ENABLED; - } else if (versioningStatus != BucketVersioningStatus.SUSPENDED) { - this.versioningStatus = BucketVersioningStatus.UNVERSIONED; - } return this; } @@ -633,7 +651,6 @@ public BucketInfo getProtobuf() { .setBucketName(bucketName) .addAllAcls(OzoneAclUtil.toProtobuf(acls)) .setIsVersionEnabled(isVersionEnabled) - .setVersioningStatus(versioningStatus.toProto()) .setStorageType(storageType.toProto()) .setCreationTime(creationTime) .setModificationTime(modificationTime) @@ -647,6 +664,12 @@ public BucketInfo getProtobuf() { .setQuotaInNamespace(quotaInNamespace) .setSnapshotUsedBytes(snapshotUsedBytes) .setSnapshotUsedNamespace(snapshotUsedNamespace); + // Written only when actually set, so that hasVersioningStatus() keeps + // telling a legacy-flag bucket apart from an S3-versioned one after a + // round trip through RocksDB. + if (versioningStatus != null) { + bib.setVersioningStatus(versioningStatus.toProto()); + } if (bucketLayout != null) { bib.setBucketLayout(bucketLayout.toProto()); } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java index a089227c636a..935cfb05fd55 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java @@ -56,9 +56,11 @@ public void protobufConversion() { } @Test - public void versioningStatusDerivedFromLegacyFlag() { + public void legacyFlagDoesNotImplyAnS3VersioningStatus() { // Records written before the versioningStatus field existed deserialize - // unchanged: the status is derived from the legacy isVersionEnabled flag. + // unchanged and keep carrying the legacy flag alone. The legacy flag + // selects the in-record block version list, which is a different feature + // from S3 versioning, so it must not be promoted to a status. OzoneManagerProtocolProtos.BucketInfo oldRecord = OzoneManagerProtocolProtos.BucketInfo.newBuilder() .setVolumeName("vol1") @@ -67,6 +69,7 @@ public void versioningStatusDerivedFromLegacyFlag() { .setStorageType(HddsProtos.StorageTypeProto.DISK) .build(); OmBucketInfo bucket = OmBucketInfo.getFromProtobuf(oldRecord); + assertFalse(bucket.hasVersioningStatus()); assertEquals(BucketVersioningStatus.UNVERSIONED, bucket.getVersioningStatus()); assertFalse(bucket.getIsVersionEnabled()); @@ -74,9 +77,13 @@ public void versioningStatusDerivedFromLegacyFlag() { oldRecord = oldRecord.toBuilder().setIsVersionEnabled(true).build(); bucket = OmBucketInfo.getFromProtobuf(oldRecord); - assertEquals(BucketVersioningStatus.ENABLED, + assertFalse(bucket.hasVersioningStatus()); + assertEquals(BucketVersioningStatus.UNVERSIONED, bucket.getVersioningStatus()); assertTrue(bucket.getIsVersionEnabled()); + // the absent status survives the round trip, so a re-serialized legacy + // record is still distinguishable from an S3-versioned one + assertFalse(bucket.getProtobuf().hasVersioningStatus()); assertEquals(bucket, OmBucketInfo.getFromProtobuf(bucket.getProtobuf())); } @@ -104,40 +111,33 @@ public void builderKeepsVersioningStatusAndLegacyFlagInSync() { .setBucketName("bucket") .setVolumeName("vol1"); - // default is UNVERSIONED + // no status set at all + assertFalse(builder.build().hasVersioningStatus()); assertEquals(BucketVersioningStatus.UNVERSIONED, builder.build().getVersioningStatus()); - // legacy true -> ENABLED + // the legacy flag sets only itself: no status is derived from it builder.setIsVersionEnabled(true); - assertEquals(BucketVersioningStatus.ENABLED, + assertFalse(builder.build().hasVersioningStatus()); + assertEquals(BucketVersioningStatus.UNVERSIONED, builder.build().getVersioningStatus()); assertTrue(builder.build().getIsVersionEnabled()); - // explicit SUSPENDED forces the legacy flag to false + // an explicit status is authoritative and drives the legacy flag builder.setVersioningStatus(BucketVersioningStatus.SUSPENDED); + assertTrue(builder.build().hasVersioningStatus()); assertEquals(BucketVersioningStatus.SUSPENDED, builder.build().getVersioningStatus()); assertFalse(builder.build().getIsVersionEnabled()); - // legacy false does not clobber an explicitly SUSPENDED status - builder.setIsVersionEnabled(false); - assertEquals(BucketVersioningStatus.SUSPENDED, - builder.build().getVersioningStatus()); + // ENABLED shows up as true to clients that only know the legacy flag + builder.setVersioningStatus(BucketVersioningStatus.ENABLED); + assertTrue(builder.build().getIsVersionEnabled()); // a null status is a no-op (records without the new field) builder.setVersioningStatus(null); - assertEquals(BucketVersioningStatus.SUSPENDED, + assertEquals(BucketVersioningStatus.ENABLED, builder.build().getVersioningStatus()); - - // legacy false on a never-enabled bucket stays UNVERSIONED - OmBucketInfo unversioned = OmBucketInfo.newBuilder() - .setBucketName("bucket") - .setVolumeName("vol1") - .setIsVersionEnabled(false) - .build(); - assertEquals(BucketVersioningStatus.UNVERSIONED, - unversioned.getVersioningStatus()); } @Test diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 4d9777893146..f8b1be0e959a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -3060,7 +3060,11 @@ public OmBucketInfo getBucketInfo(String volume, String bucket) .setDefaultReplicationConfig( realBucket.getDefaultReplicationConfig()) .setIsVersionEnabled(realBucket.getIsVersionEnabled()) - .setVersioningStatus(realBucket.getVersioningStatus()) + // Only when the real bucket actually carries a status: copying the + // value getVersioningStatus() derives for a legacy bucket would give + // the link an explicit status the real bucket does not have. + .setVersioningStatus(realBucket.hasVersioningStatus() + ? realBucket.getVersioningStatus() : null) .setStorageType(realBucket.getStorageType()) .setQuotaInBytes(realBucket.getQuotaInBytes()) .setQuotaInNamespace(realBucket.getQuotaInNamespace()) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index 2a8f8da52071..f897c5423ca7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -176,17 +176,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut //Check Versioning to update Boolean versioning = omBucketArgs.getIsVersionEnabled(); BucketVersioningStatus newVersioningStatus = omBucketArgs.getVersioningStatus(); - if (newVersioningStatus == null && versioning != null) { - // Legacy flag from older clients: enabling always maps to ENABLED; - // disabling maps to SUSPENDED once versioning has ever been enabled - // (the S3 state machine forbids returning to UNVERSIONED). - if (versioning) { - newVersioningStatus = BucketVersioningStatus.ENABLED; - } else { - newVersioningStatus = - dbBucketInfo.getVersioningStatus() == BucketVersioningStatus.UNVERSIONED - ? BucketVersioningStatus.UNVERSIONED : BucketVersioningStatus.SUSPENDED; - } + if (versioning != null) { + // Apply the legacy flag on its own; setVersioningStatus below overrides + // it when a status is also being set. + bucketInfoBuilder.setIsVersionEnabled(versioning); + } + if (newVersioningStatus == null && versioning != null + && dbBucketInfo.hasVersioningStatus()) { + // Legacy flag from an older client against a bucket that already has an + // S3 versioning status: keep the two consistent. Disabling maps to + // SUSPENDED, since the S3 state machine forbids returning to + // UNVERSIONED once versioning has been enabled. + // + // On a bucket without a status the flag is left alone: it selects the + // legacy in-record block version list, and an old client must not be + // able to opt a bucket into S3 versioning semantics. + newVersioningStatus = versioning + ? BucketVersioningStatus.ENABLED : BucketVersioningStatus.SUSPENDED; } if (newVersioningStatus != null) { if (!dbBucketInfo.getVersioningStatus().canTransitionTo(newVersioningStatus)) { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java index be5bf3375fab..0d2815cbc0af 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java @@ -497,20 +497,31 @@ public void testLegacyVersioningFlagMapsToStateMachine() throws Exception { assertEquals(BucketVersioningStatus.UNVERSIONED, omMetadataManager.getBucketTable().get(bucketKey).getVersioningStatus()); - // legacy true -> ENABLED + // legacy true does NOT opt the bucket into S3 versioning: it only sets the + // legacy flag, which selects the in-record block version list response = new OMBucketSetPropertyRequest( createSetVersioningFlagRequest(volumeName, bucketName, true)) .validateAndUpdateCache(ozoneManager, 2); assertTrue(response.getOMResponse().getSuccess()); - assertEquals(BucketVersioningStatus.ENABLED, - omMetadataManager.getBucketTable().get(bucketKey).getVersioningStatus()); + OmBucketInfo dbBucketInfo = omMetadataManager.getBucketTable().get(bucketKey); + assertFalse(dbBucketInfo.hasVersioningStatus()); + assertEquals(BucketVersioningStatus.UNVERSIONED, + dbBucketInfo.getVersioningStatus()); + assertTrue(dbBucketInfo.getIsVersionEnabled()); - // legacy false after enabling -> SUSPENDED, not UNVERSIONED + // once a status exists, an old client's flag is kept consistent with it: + // disabling maps to SUSPENDED rather than back to UNVERSIONED response = new OMBucketSetPropertyRequest( - createSetVersioningFlagRequest(volumeName, bucketName, false)) + createSetVersioningStatusRequest(volumeName, bucketName, + BucketVersioningStatus.ENABLED)) .validateAndUpdateCache(ozoneManager, 3); assertTrue(response.getOMResponse().getSuccess()); - OmBucketInfo dbBucketInfo = omMetadataManager.getBucketTable().get(bucketKey); + + response = new OMBucketSetPropertyRequest( + createSetVersioningFlagRequest(volumeName, bucketName, false)) + .validateAndUpdateCache(ozoneManager, 4); + assertTrue(response.getOMResponse().getSuccess()); + dbBucketInfo = omMetadataManager.getBucketTable().get(bucketKey); assertEquals(BucketVersioningStatus.SUSPENDED, dbBucketInfo.getVersioningStatus()); assertFalse(dbBucketInfo.getIsVersionEnabled()); } From f13c90ce5396ef0a70c3f2a87e1e391ecf0378fe Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 23 Jul 2026 13:50:08 +0800 Subject: [PATCH 09/23] T2.1. Add pluggable VersionIdGenerator with the transaction index default --- .../src/main/resources/ozone-default.xml | 12 ++ .../apache/hadoop/ozone/om/OMConfigKeys.java | 7 + .../TransactionIndexVersionIdGenerator.java | 36 +++++ .../ozone/om/helpers/VersionIdGenerator.java | 86 ++++++++++++ .../om/helpers/TestVersionIdGenerator.java | 131 ++++++++++++++++++ 5 files changed, 272 insertions(+) create mode 100644 hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/TransactionIndexVersionIdGenerator.java create mode 100644 hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java create mode 100644 hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index ad8ea45b3d7f..a2f70b1305c9 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -5163,4 +5163,16 @@ OZONE, RATIS, OM The maximum number of events that can be pending in OM Ratis. + + ozone.om.versioning.version-id-generator + org.apache.hadoop.ozone.om.helpers.TransactionIndexVersionIdGenerator + OZONE, OM + + Implementation of org.apache.hadoop.ozone.om.helpers.VersionIdGenerator used to assign the + versionId of an object version. The default uses the index of the committing transaction. + The setting is cluster-wide and may be changed on a running cluster; a write whose generated + versionId already exists on the key is rejected, and the existing version has to be deleted + before that id can be written again. + + diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java index 02b270070ed4..b1ae6de8c167 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java @@ -21,6 +21,8 @@ import org.apache.hadoop.hdds.client.ReplicationFactor; import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.TransactionIndexVersionIdGenerator; +import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; import org.apache.ratis.util.TimeDuration; /** @@ -715,6 +717,11 @@ public final class OMConfigKeys { "ozone.om.ratis.events.max.limit"; public static final int OZONE_OM_RATIS_EVENTS_MAX_LIMIT_DEFAULT = 100; + public static final String OZONE_OM_VERSIONING_VERSION_ID_GENERATOR = + "ozone.om.versioning.version-id-generator"; + public static final Class + OZONE_OM_VERSIONING_VERSION_ID_GENERATOR_DEFAULT = TransactionIndexVersionIdGenerator.class; + /** * Never constructed. */ diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/TransactionIndexVersionIdGenerator.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/TransactionIndexVersionIdGenerator.java new file mode 100644 index 000000000000..c94a7679e042 --- /dev/null +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/TransactionIndexVersionIdGenerator.java @@ -0,0 +1,36 @@ +/* + * 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; + +import com.google.common.base.Preconditions; + +/** + * Uses the index of the committing transaction as the versionId, the default + * generator. Holds no allocator state, so a version costs no read or write + * beyond the commit itself. + */ +public class TransactionIndexVersionIdGenerator implements VersionIdGenerator { + + @Override + public long generateVersionId(long transactionLogIndex, boolean hasCurrentVersion) { + Preconditions.checkArgument(transactionLogIndex > FIRST_VERSION_ID, + "Transaction index " + transactionLogIndex + + " is a reserved versionId, expected greater than " + FIRST_VERSION_ID); + return transactionLogIndex; + } +} diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java new file mode 100644 index 000000000000..05fcbfe72c06 --- /dev/null +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java @@ -0,0 +1,86 @@ +/* + * 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; + +import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.ozone.om.OMConfigKeys; +import org.apache.hadoop.util.ReflectionUtils; + +/** + * Assigns the versionId of an object version when the version is committed. + * + *

Deployments differ in whether they need a version identity that can be + * constructed without listing, so the implementation is chosen per cluster + * through {@link OMConfigKeys#OZONE_OM_VERSIONING_VERSION_ID_GENERATOR}. + * Implementations must be public, have a public no-argument constructor, and + * satisfy the constraints that the versionedKeyTable layout and version + * promotion rely on: + * + *

    + *
  • ids increase within a key: a version created later has a larger id;
    • + *
  • an id is assigned once when the version is created and never changes + * afterwards, so that external references stay valid;
    • + *
  • {@link #NULL_VERSION_ID} is reserved and is never generated.
    • + *
+ * + *

The generator is cluster-wide and may be changed on a running cluster, so + * these constraints hold per generator but not necessarily across a change of + * generator. Colliding ids are rejected at commit time rather than prevented + * here; see {@code VersionIdAllocator}. + */ +public interface VersionIdGenerator { + + /** + * Reserved id of the null version slot, rendered as the literal "null" by + * the S3 layer. Never returned by a generator. + */ + long NULL_VERSION_ID = 0L; + + /** + * Reserved id of the pinned first version of a key, a separate slot from + * {@link #NULL_VERSION_ID}. Only assigned by generators that pin the first + * version of a key; it is smaller than any transaction index, so such a + * version sorts at the old end of the key's version sequence. + */ + long FIRST_VERSION_ID = 1L; + + /** + * Generates the versionId to freeze on a version being committed. + * + * @param transactionLogIndex index of the committing OM Ratis transaction + * @param hasCurrentVersion whether keyTable already holds a current version + * of the key being committed. The write path looks the current version up + * anyway, so generators that treat the first version of a key specially + * need no read of their own. + * @return the versionId of the new version + */ + long generateVersionId(long transactionLogIndex, boolean hasCurrentVersion); + + /** + * Instantiates the generator configured for this cluster. + * + * @throws RuntimeException if the configured class cannot be instantiated + */ + static VersionIdGenerator fromConfiguration(ConfigurationSource conf) { + Class generatorClass = conf.getClass( + OMConfigKeys.OZONE_OM_VERSIONING_VERSION_ID_GENERATOR, + OMConfigKeys.OZONE_OM_VERSIONING_VERSION_ID_GENERATOR_DEFAULT, + VersionIdGenerator.class); + return ReflectionUtils.newInstance(generatorClass, null); + } +} diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java new file mode 100644 index 000000000000..65b047507fc2 --- /dev/null +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java @@ -0,0 +1,131 @@ +/* + * 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; + +import static org.apache.hadoop.ozone.om.helpers.VersionIdGenerator.FIRST_VERSION_ID; +import static org.apache.hadoop.ozone.om.helpers.VersionIdGenerator.NULL_VERSION_ID; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.stream.Stream; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.om.OMConfigKeys; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Tests the constraints every {@link VersionIdGenerator} has to satisfy, and + * how the cluster-wide generator is selected. + */ +public class TestVersionIdGenerator { + + /** The first transaction index that is not a reserved versionId. */ + private static final long FIRST_USABLE_INDEX = FIRST_VERSION_ID + 1; + + /** Every generator shipped with Ozone; extended as generators are added. */ + static Stream generators() { + return Stream.of(new TransactionIndexVersionIdGenerator()); + } + + @ParameterizedTest + @MethodSource("generators") + void generatedIdsIncreaseWithTheTransactionIndex(VersionIdGenerator generator) { + long previous = generator.generateVersionId(FIRST_USABLE_INDEX, false); + for (long index = FIRST_USABLE_INDEX + 1; index < 100; index++) { + long current = generator.generateVersionId(index, true); + assertTrue(previous < current, + "versionId " + current + " generated for transaction " + index + + " does not exceed " + previous); + previous = current; + } + } + + @ParameterizedTest + @MethodSource("generators") + void generatedIdsNeverCollideWithReservedIds(VersionIdGenerator generator) { + for (long index = FIRST_USABLE_INDEX; index < 100; index++) { + assertNotEquals(NULL_VERSION_ID, generator.generateVersionId(index, true)); + } + // A transaction index that lands on a reserved id is a misconfiguration of + // the Ratis log rather than something to silently work around. + assertThrows(IllegalArgumentException.class, + () -> generator.generateVersionId(NULL_VERSION_ID, true)); + assertThrows(IllegalArgumentException.class, + () -> generator.generateVersionId(FIRST_VERSION_ID, true)); + } + + @ParameterizedTest + @MethodSource("generators") + void generationIsDeterministic(VersionIdGenerator generator) { + assertEquals(generator.generateVersionId(4242, true), + generator.generateVersionId(4242, true)); + assertEquals(generator.generateVersionId(4242, false), + generator.generateVersionId(4242, false)); + } + + @Test + void reservedIdsDoNotCollide() { + assertNotEquals(NULL_VERSION_ID, FIRST_VERSION_ID); + } + + @Test + void transactionIndexGeneratorIsTheDefault() { + assertInstanceOf(TransactionIndexVersionIdGenerator.class, + VersionIdGenerator.fromConfiguration(new OzoneConfiguration())); + } + + @Test + void transactionIndexIgnoresWhetherTheKeyHasACurrentVersion() { + VersionIdGenerator generator = new TransactionIndexVersionIdGenerator(); + + assertEquals(7, generator.generateVersionId(7, false)); + assertEquals(7, generator.generateVersionId(7, true)); + } + + @Test + void generatorClassIsReadFromConfiguration() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OMConfigKeys.OZONE_OM_VERSIONING_VERSION_ID_GENERATOR, + TransactionIndexVersionIdGenerator.class.getName()); + + assertInstanceOf(TransactionIndexVersionIdGenerator.class, + VersionIdGenerator.fromConfiguration(conf)); + } + + @Test + void unknownGeneratorClassIsRejected() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OMConfigKeys.OZONE_OM_VERSIONING_VERSION_ID_GENERATOR, + "org.apache.hadoop.ozone.om.helpers.NoSuchVersionIdGenerator"); + + assertThrows(RuntimeException.class, () -> VersionIdGenerator.fromConfiguration(conf)); + } + + @Test + void generatorClassNotImplementingTheInterfaceIsRejected() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OMConfigKeys.OZONE_OM_VERSIONING_VERSION_ID_GENERATOR, + String.class.getName()); + + assertThrows(RuntimeException.class, () -> VersionIdGenerator.fromConfiguration(conf)); + } +} From 71b8f92f025896dd9ad7d0c288f90569fcb071fa Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 23 Jul 2026 13:52:39 +0800 Subject: [PATCH 10/23] T2.2. Require versionIds to increase within a key and enforce it at commit --- .../ozone/om/helpers/VersionIdGenerator.java | 16 +- .../om/helpers/TestVersionIdGenerator.java | 4 +- .../hadoop/ozone/om/VersionIdAllocator.java | 118 ++++++++++++ .../ozone/om/TestVersionIdAllocator.java | 178 ++++++++++++++++++ 4 files changed, 310 insertions(+), 6 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VersionIdAllocator.java create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestVersionIdAllocator.java diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java index 05fcbfe72c06..1b293f37d136 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java @@ -32,16 +32,22 @@ * promotion rely on: * *

    - *
  • ids increase within a key: a version created later has a larger id;
    • + *
  • ids strictly increase within a key: for one generator, the id of a + * version created later is always greater than the id of every version of + * that key created before it, never equal and never smaller. The + * versionedKeyTable ordering and version promotion depend on this;
    • *
  • an id is assigned once when the version is created and never changes * afterwards, so that external references stay valid;
    • *
  • {@link #NULL_VERSION_ID} is reserved and is never generated.
    • *
* - *

The generator is cluster-wide and may be changed on a running cluster, so - * these constraints hold per generator but not necessarily across a change of - * generator. Colliding ids are rejected at commit time rather than prevented - * here; see {@code VersionIdAllocator}. + *

The first constraint binds one generator, not a sequence of them: the + * generator is cluster-wide and may be changed on a running cluster, and the + * new one knows nothing of the ids the old one handed out. + * {@code VersionIdAllocator} enforces the constraint at commit time and refuses + * a write whose id does not come after the key's current version, so a change + * of generator fails loudly on affected keys instead of corrupting their + * version order. */ public interface VersionIdGenerator { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java index 65b047507fc2..8a91d2b8fbac 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java @@ -48,7 +48,9 @@ static Stream generators() { @ParameterizedTest @MethodSource("generators") - void generatedIdsIncreaseWithTheTransactionIndex(VersionIdGenerator generator) { + void generatedIdsStrictlyIncreaseWithinAKey(VersionIdGenerator generator) { + // The contract every generator owes VersionIdAllocator: over the life of a + // key, ids only ever go up, starting from the key's first version. long previous = generator.generateVersionId(FIRST_USABLE_INDEX, false); for (long index = FIRST_USABLE_INDEX + 1; index < 100; index++) { long current = generator.generateVersionId(index, true); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VersionIdAllocator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VersionIdAllocator.java new file mode 100644 index 000000000000..bc2f776254a2 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VersionIdAllocator.java @@ -0,0 +1,118 @@ +/* + * 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; + +import java.io.IOException; +import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; + +/** + * Assigns the versionId of a version being committed, using the generator + * configured for this cluster. + * + *

The generator is cluster-wide and can be changed by reconfiguring the OMs, + * so ids generated before and after a change are not guaranteed to be distinct. + * Rather than constrain the change, a commit whose versionId already exists on + * the key is rejected: the operator or the client deletes the existing version + * first, and the write can then be retried. + */ +public class VersionIdAllocator { + + private final VersionIdGenerator generator; + + public VersionIdAllocator(ConfigurationSource conf) { + this(VersionIdGenerator.fromConfiguration(conf)); + } + + public VersionIdAllocator(VersionIdGenerator generator) { + this.generator = generator; + } + + public VersionIdGenerator getGenerator() { + return generator; + } + + /** + * Returns the versionId to freeze on the version being committed. + * + *

The id must be greater than the one on the key's current version: a + * generator has to hand out increasing ids for a key, and the versionedKeyTable + * ordering and version promotion depend on it. An id that is not greater is + * refused rather than written, because it would either overwrite an existing + * version or sort into the wrong place. In practice this only happens after the + * cluster's generator is changed, or if the Ratis log index went backwards. + * + * @param currentVersion the key's current version, or null if the key has + * none. The write path holds it already, so no extra read is needed here. + * @throws OMException INVALID_REQUEST if the generated id does not exceed the + * current version's id, KEY_ALREADY_EXISTS if it is already taken + */ + public long allocate(OMMetadataManager metadataManager, String volumeName, + String bucketName, String keyName, long transactionLogIndex, + OmKeyInfo currentVersion) throws IOException { + + long versionId = + generator.generateVersionId(transactionLogIndex, currentVersion != null); + + if (currentVersion == null) { + // No current version means the key has no versions at all, so nothing can + // be taken and nothing constrains the id. + return versionId; + } + + Long currentVersionId = currentVersion.getVersionId(); + if (currentVersionId == null) { + // A current version written before versioning was enabled carries no id, + // so there is nothing to order against; fall back to looking the id up. + if (isTaken(metadataManager, volumeName, bucketName, keyName, versionId)) { + throw alreadyExists(volumeName, bucketName, keyName, versionId); + } + return versionId; + } + + if (versionId <= currentVersionId) { + throw new OMException("Version " + versionId + " of key /" + volumeName + "/" + + bucketName + "/" + keyName + " does not come after the current version " + + currentVersionId + ". " + generator.getClass().getName() + + " must generate increasing versionIds for a key; an id that goes backwards can " + + "happen after the cluster's " + OMConfigKeys.OZONE_OM_VERSIONING_VERSION_ID_GENERATOR + + " is changed. Delete the key's versions before writing with the new generator.", + OMException.ResultCodes.INVALID_REQUEST); + } + + // Strictly greater than the largest id on the key, so no lookup is needed: + // every noncurrent version of the key has a smaller id than the current one. + return versionId; + } + + private static OMException alreadyExists(String volumeName, String bucketName, + String keyName, long versionId) { + return new OMException("Version " + versionId + " of key /" + volumeName + "/" + + bucketName + "/" + keyName + " already exists. Delete that version before " + + "writing this one.", OMException.ResultCodes.KEY_ALREADY_EXISTS); + } + + private boolean isTaken(OMMetadataManager metadataManager, String volumeName, + String bucketName, String keyName, long versionId) throws IOException { + return metadataManager.getVersionedKeyTable().isExist( + metadataManager.getVersionedOzoneKey(volumeName, bucketName, keyName, versionId)); + } + +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestVersionIdAllocator.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestVersionIdAllocator.java new file mode 100644 index 000000000000..17f3b6e41b0e --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestVersionIdAllocator.java @@ -0,0 +1,178 @@ +/* + * 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; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.HashSet; +import java.util.Set; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.TransactionIndexVersionIdGenerator; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Tests {@link VersionIdAllocator}: which versionId a commit gets, and the + * rejection of ids that are already taken on the key. + */ +public class TestVersionIdAllocator { + + private static final String VOLUME = "vol1"; + private static final String BUCKET = "bucket1"; + private static final String KEY = "key1"; + + private OMMetadataManager metadataManager; + private Set versionedKeys; + private int lookups; + + @BeforeEach + void setUp() throws Exception { + versionedKeys = new HashSet<>(); + lookups = 0; + + Table versionedKeyTable = mock(Table.class); + when(versionedKeyTable.isExist(anyString())).thenAnswer(invocation -> { + lookups++; + return versionedKeys.contains(invocation.getArgument(0)); + }); + + metadataManager = mock(OMMetadataManager.class); + when(metadataManager.getVersionedKeyTable()).thenReturn(versionedKeyTable); + when(metadataManager.getVersionedOzoneKey(eq(VOLUME), eq(BUCKET), eq(KEY), anyLong())) + .thenAnswer(invocation -> dbKey(invocation.getArgument(3))); + } + + private static String dbKey(long versionId) { + return "/" + VOLUME + "/" + BUCKET + "/" + KEY + "/" + versionId; + } + + private VersionIdAllocator allocator() { + return new VersionIdAllocator(new TransactionIndexVersionIdGenerator()); + } + + private static OmKeyInfo keyWithVersionId(Long versionId) { + return new OmKeyInfo.Builder() + .setVolumeName(VOLUME) + .setBucketName(BUCKET) + .setKeyName(KEY) + .setVersionId(versionId) + .build(); + } + + @Test + void allocatesTheTransactionIndexForTheFirstVersion() throws Exception { + assertEquals(7, allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 7, null)); + } + + @Test + void allocatesTheTransactionIndexForALaterVersion() throws Exception { + assertEquals(9, allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 9, + keyWithVersionId(7L))); + } + + @Test + void rejectsAnIdEqualToTheCurrentVersion() { + OMException e = assertThrows(OMException.class, + () -> allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 7, + keyWithVersionId(7L))); + + assertEquals(OMException.ResultCodes.INVALID_REQUEST, e.getResult()); + } + + @Test + void rejectsAnIdOlderThanTheCurrentVersion() { + // Refused even though no version holds this id: writing it would sort the + // new version before versions that predate it. + OMException e = assertThrows(OMException.class, + () -> allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 5, + keyWithVersionId(9L))); + + assertEquals(OMException.ResultCodes.INVALID_REQUEST, e.getResult()); + } + + @Test + void skipsTheLookupWhenTheKeyHasNoCurrentVersion() throws Exception { + assertEquals(7, allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 7, null)); + assertEquals(0, lookups); + } + + @Test + void skipsTheLookupWhenTheGeneratedIdIsNewerThanTheCurrentVersion() throws Exception { + // The steady-state path: the current version holds the key's largest id, so + // an id above it cannot be taken and costs no read. + assertEquals(9, allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 9, + keyWithVersionId(7L))); + assertEquals(0, lookups); + } + + @Test + void skipsTheLookupWhenTheIdIsRefusedForGoingBackwards() { + assertThrows(OMException.class, + () -> allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 5, + keyWithVersionId(9L))); + + assertEquals(0, lookups); + } + + @Test + void looksUpTheTableForACurrentVersionPredatingVersioning() throws Exception { + // Keys written before versioning was enabled carry no versionId, so there + // is nothing to order against and the id has to be looked up. + assertEquals(7, allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 7, + keyWithVersionId(null))); + + assertEquals(1, lookups); + } + + @Test + void rejectsATakenIdForACurrentVersionPredatingVersioning() { + versionedKeys.add(dbKey(7)); + + OMException e = assertThrows(OMException.class, + () -> allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 7, + keyWithVersionId(null))); + + assertEquals(OMException.ResultCodes.KEY_ALREADY_EXISTS, e.getResult()); + } + + @Test + void allowsAnIdHeldByAnotherKeysVersion() throws Exception { + // Ids are only unique within a key, so another key holding it is fine. + versionedKeys.add("/" + VOLUME + "/" + BUCKET + "/otherKey/7"); + + assertEquals(7, allocator().allocate(metadataManager, VOLUME, BUCKET, KEY, 7, + keyWithVersionId(null))); + } + + @Test + void usesTheGeneratorConfiguredForTheCluster() { + assertInstanceOf(TransactionIndexVersionIdGenerator.class, + new VersionIdAllocator(new OzoneConfiguration()).getGenerator()); + } + +} From cb00be9dc79dc8226c71b97f07d23111bb453f4d Mon Sep 17 00:00:00 2001 From: Symious Date: Thu, 23 Jul 2026 13:57:03 +0800 Subject: [PATCH 11/23] T2.3. Add PinnedFirstVersionIdGenerator --- .../src/main/resources/ozone-default.xml | 4 +- .../PinnedFirstVersionIdGenerator.java | 53 +++++++++++++++++++ .../om/helpers/TestVersionIdGenerator.java | 39 +++++++++++++- 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/PinnedFirstVersionIdGenerator.java diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index a2f70b1305c9..0777bb4b9c39 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -5169,7 +5169,9 @@ OZONE, OM Implementation of org.apache.hadoop.ozone.om.helpers.VersionIdGenerator used to assign the - versionId of an object version. The default uses the index of the committing transaction. + versionId of an object version. The default uses the index of the committing transaction; + PinnedFirstVersionIdGenerator additionally pins the first version of every key to a reserved + sentinel, so that it can be referenced without listing the key's versions first. The setting is cluster-wide and may be changed on a running cluster; a write whose generated versionId already exists on the key is rejected, and the existing version has to be deleted before that id can be written again. diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/PinnedFirstVersionIdGenerator.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/PinnedFirstVersionIdGenerator.java new file mode 100644 index 000000000000..061c467651c9 --- /dev/null +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/PinnedFirstVersionIdGenerator.java @@ -0,0 +1,53 @@ +/* + * 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; + +import com.google.common.base.Preconditions; + +/** + * Pins the first version of every key to {@link #FIRST_VERSION_ID} and uses the + * committing transaction's index for every later version, exactly like + * {@link TransactionIndexVersionIdGenerator}. Clusters configured with this + * generator can reference the first version of a key without listing it first. + * + *

Holds no allocator state either: a key is on its first version exactly + * when keyTable holds no current version for it, which the write path looks up + * anyway. + * + *

The sentinel is smaller than any transaction index, so the first version + * sorts at the old end of the key's version sequence, as the versionedKeyTable + * layout requires. + * + *

Known trade-off: once every version of a key is permanently deleted, a + * recreated key takes the sentinel again, so an external reference to the first + * version resolves to the new content. Later versions are transaction indices + * and are never reused. + */ +public class PinnedFirstVersionIdGenerator implements VersionIdGenerator { + + @Override + public long generateVersionId(long transactionLogIndex, boolean hasCurrentVersion) { + if (!hasCurrentVersion) { + return FIRST_VERSION_ID; + } + Preconditions.checkArgument(transactionLogIndex > FIRST_VERSION_ID, + "Transaction index " + transactionLogIndex + + " is a reserved versionId, expected greater than " + FIRST_VERSION_ID); + return transactionLogIndex; + } +} diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java index 8a91d2b8fbac..534dbb5dd2af 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java @@ -43,7 +43,8 @@ public class TestVersionIdGenerator { /** Every generator shipped with Ozone; extended as generators are added. */ static Stream generators() { - return Stream.of(new TransactionIndexVersionIdGenerator()); + return Stream.of(new TransactionIndexVersionIdGenerator(), + new PinnedFirstVersionIdGenerator()); } @ParameterizedTest @@ -66,6 +67,7 @@ void generatedIdsStrictlyIncreaseWithinAKey(VersionIdGenerator generator) { void generatedIdsNeverCollideWithReservedIds(VersionIdGenerator generator) { for (long index = FIRST_USABLE_INDEX; index < 100; index++) { assertNotEquals(NULL_VERSION_ID, generator.generateVersionId(index, true)); + assertNotEquals(NULL_VERSION_ID, generator.generateVersionId(index, false)); } // A transaction index that lands on a reserved id is a misconfiguration of // the Ratis log rather than something to silently work around. @@ -103,6 +105,41 @@ void transactionIndexIgnoresWhetherTheKeyHasACurrentVersion() { assertEquals(7, generator.generateVersionId(7, true)); } + @Test + void pinnedFirstPinsOnlyTheFirstVersionOfAKey() { + VersionIdGenerator generator = new PinnedFirstVersionIdGenerator(); + + assertEquals(FIRST_VERSION_ID, generator.generateVersionId(7, false)); + assertEquals(7, generator.generateVersionId(7, true)); + } + + @Test + void pinnedFirstSentinelIsOlderThanEveryTransactionIndex() { + VersionIdGenerator generator = new PinnedFirstVersionIdGenerator(); + long first = generator.generateVersionId(FIRST_USABLE_INDEX, false); + + for (long index = FIRST_USABLE_INDEX; index < 100; index++) { + assertTrue(first < generator.generateVersionId(index, true), + "sentinel " + first + " is not older than the version at transaction " + index); + } + } + + @Test + void pinnedFirstSentinelIsNotTheNullVersion() { + assertNotEquals(NULL_VERSION_ID, + new PinnedFirstVersionIdGenerator().generateVersionId(7, false)); + } + + @Test + void pinnedFirstGeneratorIsSelectableByConfiguration() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(OMConfigKeys.OZONE_OM_VERSIONING_VERSION_ID_GENERATOR, + PinnedFirstVersionIdGenerator.class.getName()); + + assertInstanceOf(PinnedFirstVersionIdGenerator.class, + VersionIdGenerator.fromConfiguration(conf)); + } + @Test void generatorClassIsReadFromConfiguration() { OzoneConfiguration conf = new OzoneConfiguration(); From 5c654932210a2e3417c942fe1d029de6f87b1bb6 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 31 Jul 2026 10:27:06 +0800 Subject: [PATCH 12/23] T2. Address comments Rename NULL_VERSION_ID to UNSET_VERSION_ID: 0 is the unset value of the optional proto field, not the id of the null version. A null version carries a normally generated id and is identified by isNullVersion, so that a null created between two versioned writes orders as the middle version rather than the oldest. Co-Authored-By: Claude Opus 5 --- .../ozone/om/helpers/VersionIdGenerator.java | 23 ++++++++++++------- .../om/helpers/TestVersionIdGenerator.java | 14 +++++------ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java index 1b293f37d136..083b423a6677 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/VersionIdGenerator.java @@ -38,7 +38,8 @@ * versionedKeyTable ordering and version promotion depend on this; *

  • an id is assigned once when the version is created and never changes * afterwards, so that external references stay valid;
    • - *
  • {@link #NULL_VERSION_ID} is reserved and is never generated.
    • + *
  • {@link #UNSET_VERSION_ID} and {@link #FIRST_VERSION_ID} are reserved + * and are never generated.
    • * * *

    The first constraint binds one generator, not a sequence of them: the @@ -52,16 +53,22 @@ public interface VersionIdGenerator { /** - * Reserved id of the null version slot, rendered as the literal "null" by - * the S3 layer. Never returned by a generator. + * Unset value of the optional versionId field, carried by records written + * before versioning existed. Reserved, and never returned by a generator. + * + *

    This is not the id of the null version: a null version carries a + * normally generated id like any other version and is identified by the + * {@code isNullVersion} attribute instead. Pinning it to a fixed low value + * would misorder a null created between two versioned writes, which is the + * middle version of the key rather than its oldest. */ - long NULL_VERSION_ID = 0L; + long UNSET_VERSION_ID = 0L; /** - * Reserved id of the pinned first version of a key, a separate slot from - * {@link #NULL_VERSION_ID}. Only assigned by generators that pin the first - * version of a key; it is smaller than any transaction index, so such a - * version sorts at the old end of the key's version sequence. + * Reserved id of the pinned first version of a key. Only assigned by + * generators that pin the first version of a key; it is smaller than any + * transaction index, so such a version sorts at the old end of the key's + * version sequence. */ long FIRST_VERSION_ID = 1L; diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java index 534dbb5dd2af..612b42872927 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestVersionIdGenerator.java @@ -18,7 +18,7 @@ package org.apache.hadoop.ozone.om.helpers; import static org.apache.hadoop.ozone.om.helpers.VersionIdGenerator.FIRST_VERSION_ID; -import static org.apache.hadoop.ozone.om.helpers.VersionIdGenerator.NULL_VERSION_ID; +import static org.apache.hadoop.ozone.om.helpers.VersionIdGenerator.UNSET_VERSION_ID; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -66,13 +66,13 @@ void generatedIdsStrictlyIncreaseWithinAKey(VersionIdGenerator generator) { @MethodSource("generators") void generatedIdsNeverCollideWithReservedIds(VersionIdGenerator generator) { for (long index = FIRST_USABLE_INDEX; index < 100; index++) { - assertNotEquals(NULL_VERSION_ID, generator.generateVersionId(index, true)); - assertNotEquals(NULL_VERSION_ID, generator.generateVersionId(index, false)); + assertNotEquals(UNSET_VERSION_ID, generator.generateVersionId(index, true)); + assertNotEquals(UNSET_VERSION_ID, generator.generateVersionId(index, false)); } // A transaction index that lands on a reserved id is a misconfiguration of // the Ratis log rather than something to silently work around. assertThrows(IllegalArgumentException.class, - () -> generator.generateVersionId(NULL_VERSION_ID, true)); + () -> generator.generateVersionId(UNSET_VERSION_ID, true)); assertThrows(IllegalArgumentException.class, () -> generator.generateVersionId(FIRST_VERSION_ID, true)); } @@ -88,7 +88,7 @@ void generationIsDeterministic(VersionIdGenerator generator) { @Test void reservedIdsDoNotCollide() { - assertNotEquals(NULL_VERSION_ID, FIRST_VERSION_ID); + assertNotEquals(UNSET_VERSION_ID, FIRST_VERSION_ID); } @Test @@ -125,8 +125,8 @@ void pinnedFirstSentinelIsOlderThanEveryTransactionIndex() { } @Test - void pinnedFirstSentinelIsNotTheNullVersion() { - assertNotEquals(NULL_VERSION_ID, + void pinnedFirstSentinelIsNotTheUnsetId() { + assertNotEquals(UNSET_VERSION_ID, new PinnedFirstVersionIdGenerator().generateVersionId(7, false)); } From e6cc33ddb2af2273e88f81dcc2806d049c55227a Mon Sep 17 00:00:00 2001 From: Symious Date: Wed, 29 Jul 2026 14:03:04 +0800 Subject: [PATCH 13/23] T3.1. Keep the overwritten current version in the versionedKeyTable Co-Authored-By: Claude Opus 5 --- .../hadoop/ozone/om/helpers/OmBucketInfo.java | 13 ++ .../apache/hadoop/ozone/om/OzoneManager.java | 11 + .../om/request/key/OMKeyCommitRequest.java | 37 ++- .../ozone/om/request/key/OMKeyRequest.java | 12 +- .../om/response/key/OMKeyCommitResponse.java | 21 +- .../om/request/key/OMKeyRequestTests.java | 3 + .../key/TestOMKeyVersioningRequests.java | 213 ++++++++++++++++++ 7 files changed, 301 insertions(+), 9 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java index 5968bff7a9d3..b48d2d04d664 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java @@ -205,6 +205,19 @@ public boolean hasVersioningStatus() { return versioningStatus != null; } + /** + * Whether S3-compatible versioning is in effect, i.e. whether a write keeps + * the previous current version as a separate record in the versionedKeyTable. + * True for status ENABLED on an OBJECT_STORE bucket; buckets of other layouts + * carrying the legacy isVersionEnabled flag keep the legacy in-record block + * version behaviour. + * @return whether writes create versionedKeyTable records + */ + public boolean isS3VersioningEnabled() { + return versioningStatus == BucketVersioningStatus.ENABLED + && bucketLayout == BucketLayout.OBJECT_STORE; + } + /** * Returns the type of storage to be used. * @return StorageType diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index f8b1be0e959a..b85802f26e88 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -416,6 +416,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl private BucketManager bucketManager; private KeyManager keyManager; private PrefixManagerImpl prefixManager; + private final VersionIdAllocator versionIdAllocator; private final UpgradeFinalizer upgradeFinalizer; private ExecutorService edekCacheLoader = null; @@ -565,6 +566,7 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption) versionManager = new OMLayoutVersionManager(omStorage.getLayoutVersion()); upgradeFinalizer = new OMUpgradeFinalizer(versionManager); + versionIdAllocator = new VersionIdAllocator(conf); replicationConfigValidator = conf.getObject(ReplicationConfigValidator.class); @@ -2387,6 +2389,15 @@ public long getObjectIdFromTxId(long trxnId) { trxnId); } + /** + * Assigns the versionId of a version being committed, using the + * {@link org.apache.hadoop.ozone.om.helpers.VersionIdGenerator} configured + * for this cluster. + */ + public VersionIdAllocator getVersionIdAllocator() { + return versionIdAllocator; + } + /** * * @return Gets the stored layout version from the DB meta table. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index 6c34443058b5..c495db1fde57 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -52,6 +52,7 @@ import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.helpers.QuotaUtil; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; import org.apache.hadoop.ozone.om.helpers.WithMetadata; import org.apache.hadoop.ozone.om.request.util.OmKeyHSyncUtil; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; @@ -303,14 +304,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut } validateAtomicRewrite(keyToDelete, omKeyInfo, auditMap); + final boolean s3Versioning = omBucketInfo.isS3VersioningEnabled(); // Set the UpdateID to current transactionLogIndex - omKeyInfo = omKeyInfo.toBuilder() + OmKeyInfo.Builder committedKeyBuilder = omKeyInfo.toBuilder() .setExpectedDataGeneration(null) .addAllMetadata(KeyValueUtil.getFromProtobuf( commitKeyArgs.getMetadataList())) .setUpdateID(trxnLogIndex) - .setDataSize(commitKeyArgs.getDataSize()) - .build(); + .setDataSize(commitKeyArgs.getDataSize()); + if (s3Versioning) { + // The version identity is frozen when the version is created: an hsync + // re-commit keeps updating the same version, so it keeps its versionId. + committedKeyBuilder.setVersionId(isSameHsyncKey + ? keyToDelete.getVersionId() + : ozoneManager.getVersionIdAllocator().allocate(omMetadataManager, + volumeName, bucketName, keyName, trxnLogIndex, keyToDelete)); + } + omKeyInfo = committedKeyBuilder.build(); // Update the block length for each block, return the allocated but // uncommitted blocks @@ -401,6 +411,24 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut dbOpenKey, newOpenKeyInfo, trxnLogIndex); } + // With S3-compatible versioning the overwritten current version is kept + // as a noncurrent version instead of being reclaimed. A record written + // before versioning was enabled carries no versionId and becomes the + // key's null version. + String dbVersionedKey = null; + OmKeyInfo versionedKeyInfo = null; + if (s3Versioning && keyToDelete != null && !isSameHsyncKey) { + versionedKeyInfo = keyToDelete.getVersionId() != null ? keyToDelete + : keyToDelete.toBuilder() + .setVersionId(VersionIdGenerator.UNSET_VERSION_ID) + .setNullVersion(true) + .build(); + dbVersionedKey = omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, versionedKeyInfo.getVersionId()); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + dbVersionedKey, versionedKeyInfo, trxnLogIndex); + } + omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry( dbOzoneKey, omKeyInfo, trxnLogIndex); @@ -408,7 +436,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut omClientResponse = new OMKeyCommitResponse(omResponse.build(), omKeyInfo, dbOzoneKey, dbOpenKey, omBucketInfo.copyObject(), - oldKeyVersionsToDeleteMap, isHSync, newOpenKeyInfo, dbOpenKeyToDeleteKey, openKeyToDelete); + oldKeyVersionsToDeleteMap, isHSync, newOpenKeyInfo, dbOpenKeyToDeleteKey, openKeyToDelete) + .withVersionedKey(dbVersionedKey, versionedKeyInfo); result = Result.SUCCESS; } catch (IOException | InvalidPathException ex) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index d12b8fa05257..b0fef6cad794 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -959,11 +959,15 @@ protected OmKeyInfo prepareFileInfo( if (dbKeyInfo != null) { // The key already exist, the new blocks will replace old ones // as new versions unless the bucket does not have versioning - // turned on. - dbKeyInfo.addNewVersion(locations, false, - omBucketInfo.getIsVersionEnabled()); + // turned on. With S3-compatible versioning the previous current version + // is kept as its own record in the versionedKeyTable at commit time, so + // the in-record block version list is not used to accumulate object + // versions and always holds a single version. + boolean keepInRecordVersions = omBucketInfo.getIsVersionEnabled() + && !omBucketInfo.isS3VersioningEnabled(); + dbKeyInfo.addNewVersion(locations, false, keepInRecordVersions); long newSize = size; - if (omBucketInfo.getIsVersionEnabled()) { + if (keepInRecordVersions) { newSize += dbKeyInfo.getDataSize(); } // The modification time is set in preExecute. Use the same diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java index 425c4f63ac5e..f36aff6dfe46 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java @@ -21,6 +21,7 @@ import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DELETED_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VERSIONED_KEY_TABLE; import com.google.common.annotations.VisibleForTesting; import jakarta.annotation.Nonnull; @@ -39,7 +40,7 @@ * Response for CommitKey request. */ @CleanupTableInfo(cleanupTables = {OPEN_KEY_TABLE, KEY_TABLE, DELETED_TABLE, - BUCKET_TABLE}) + BUCKET_TABLE, VERSIONED_KEY_TABLE}) public class OMKeyCommitResponse extends OmKeyResponse { private OmKeyInfo omKeyInfo; @@ -51,6 +52,8 @@ public class OMKeyCommitResponse extends OmKeyResponse { private OmKeyInfo newOpenKeyInfo; private OmKeyInfo openKeyToUpdate; private String openKeyNameToUpdate; + private String versionedKeyName; + private OmKeyInfo versionedKeyInfo; @SuppressWarnings("checkstyle:ParameterNumber") public OMKeyCommitResponse( @@ -82,6 +85,17 @@ public OMKeyCommitResponse(@Nonnull OMResponse omResponse, @Nonnull checkStatusNotOK(); } + /** + * The version this commit overwrote, to be kept in the versionedKeyTable as + * a noncurrent version. Null for buckets without S3-compatible versioning. + */ + public OMKeyCommitResponse withVersionedKey(String dbVersionedKey, + OmKeyInfo keyInfo) { + this.versionedKeyName = dbVersionedKey; + this.versionedKeyInfo = keyInfo; + return this; + } + @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { @@ -98,6 +112,11 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, omMetadataManager.getKeyTable(getBucketLayout()) .putWithBatch(batchOperation, ozoneKeyName, omKeyInfo); + if (versionedKeyInfo != null) { + omMetadataManager.getVersionedKeyTable() + .putWithBatch(batchOperation, versionedKeyName, versionedKeyInfo); + } + updateDeletedTable(omMetadataManager, batchOperation); handleOpenKeyToUpdate(omMetadataManager, batchOperation); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequestTests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequestTests.java index 167fbc354a3c..f219f23f4077 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequestTests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequestTests.java @@ -74,6 +74,7 @@ import org.apache.hadoop.ozone.om.OzoneManagerPrepareState; import org.apache.hadoop.ozone.om.ResolvedBucket; import org.apache.hadoop.ozone.om.ScmClient; +import org.apache.hadoop.ozone.om.VersionIdAllocator; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; @@ -154,6 +155,8 @@ public void setup() throws Exception { when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager); when(ozoneManager.getConfiguration()).thenReturn(ozoneConfiguration); when(ozoneManager.getConfig()).thenReturn(ozoneConfiguration.getObject(OmConfig.class)); + when(ozoneManager.getVersionIdAllocator()) + .thenReturn(new VersionIdAllocator(ozoneConfiguration)); OMLayoutVersionManager lvm = mock(OMLayoutVersionManager.class); when(lvm.isAllowed(anyString())).thenReturn(true); when(ozoneManager.getVersionManager()).thenReturn(lvm); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java new file mode 100644 index 000000000000..c8211d432be8 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -0,0 +1,213 @@ +/* + * 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.request.key; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.UUID; +import org.apache.hadoop.hdds.utils.db.cache.CacheKey; +import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.BucketVersioningStatus; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; +import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; +import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CommitKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.util.Time; +import org.junit.jupiter.api.Test; + +/** + * Tests the S3-compatible versioning behaviour of key writes on a + * versioning-enabled OBJECT_STORE bucket: a commit freezes a versionId on the + * new current version and keeps the version it overwrote as a noncurrent + * version in the versionedKeyTable instead of reclaiming it. + */ +public class TestOMKeyVersioningRequests extends OMKeyRequestTests { + + @Override + public BucketLayout getBucketLayout() { + return BucketLayout.OBJECT_STORE; + } + + private void setupVersionedBucket() throws Exception { + OMRequestTestUtils.addVolumeToDB(volumeName, omMetadataManager); + OmBucketInfo bucketInfo = OmBucketInfo.newBuilder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setBucketLayout(BucketLayout.OBJECT_STORE) + .setVersioningStatus(BucketVersioningStatus.ENABLED) + .setCreationTime(Time.now()) + .build(); + omMetadataManager.getBucketTable().addCacheEntry( + new CacheKey<>(omMetadataManager.getBucketKey(volumeName, bucketName)), + CacheValue.get(1L, bucketInfo)); + } + + /** Puts a current version into keyTable, as an earlier write would have. */ + private String seedCurrentVersion(Long versionId) throws Exception { + OmKeyInfo keyInfo = OMRequestTestUtils.createOmKeyInfo( + volumeName, bucketName, keyName, replicationConfig) + .setVersionId(versionId) + .build(); + String ozoneKey = omMetadataManager.getOzoneKey( + volumeName, bucketName, keyName); + omMetadataManager.getKeyTable(getBucketLayout()).put(ozoneKey, keyInfo); + return ozoneKey; + } + + private OMRequest commitRequest(boolean isHsync) { + KeyArgs keyArgs = KeyArgs.newBuilder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(keyName) + .setModificationTime(Time.now()) + .setDataSize(0) + .build(); + return OMRequest.newBuilder() + .setCommitKeyRequest(CommitKeyRequest.newBuilder() + .setKeyArgs(keyArgs) + .setClientID(clientID) + .setHsync(isHsync)) + .setCmdType(OzoneManagerProtocolProtos.Type.CommitKey) + .setClientId(UUID.randomUUID().toString()).build(); + } + + private OMClientResponse commitAt(long trxnLogIndex) throws Exception { + return commitAt(trxnLogIndex, false); + } + + private OMClientResponse commitAt(long trxnLogIndex, boolean isHsync) + throws Exception { + OMRequestTestUtils.addKeyToTable(true, volumeName, bucketName, keyName, + clientID, replicationConfig, omMetadataManager); + OMClientResponse response = new OMKeyCommitRequest(commitRequest(isHsync), + getBucketLayout()).validateAndUpdateCache(ozoneManager, trxnLogIndex); + assertEquals(OzoneManagerProtocolProtos.Status.OK, + response.getOMResponse().getStatus()); + return response; + } + + private OmKeyInfo noncurrentVersion(long versionId) throws Exception { + return omMetadataManager.getVersionedKeyTable().get( + omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, versionId)); + } + + @Test + public void testCommitOfNewKeyAssignsVersionIdAndHasNoNoncurrentVersion() + throws Exception { + setupVersionedBucket(); + + commitAt(500L); + + OmKeyInfo current = omMetadataManager.getKeyTable(getBucketLayout()).get( + omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)); + assertNotNull(current); + assertEquals(500L, current.getVersionId()); + assertFalse(current.isDeleteMarker()); + assertFalse(current.isNullVersion()); + assertNull(noncurrentVersion(500L)); + } + + @Test + public void testOverwriteKeepsPreviousVersionAsNoncurrent() + throws Exception { + setupVersionedBucket(); + String ozoneKey = seedCurrentVersion(100L); + + commitAt(500L); + + OmKeyInfo current = + omMetadataManager.getKeyTable(getBucketLayout()).get(ozoneKey); + assertEquals(500L, current.getVersionId()); + + OmKeyInfo noncurrent = noncurrentVersion(100L); + assertNotNull(noncurrent); + assertEquals(100L, noncurrent.getVersionId()); + assertFalse(noncurrent.isNullVersion()); + } + + /** + * A record written before versioning was enabled carries no versionId, so on + * the first overwrite it becomes the key's single null version. + */ + @Test + public void testPreVersioningRecordBecomesNullVersion() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(null); + + commitAt(500L); + + OmKeyInfo noncurrent = + noncurrentVersion(VersionIdGenerator.UNSET_VERSION_ID); + assertNotNull(noncurrent); + assertTrue(noncurrent.isNullVersion()); + assertEquals(VersionIdGenerator.UNSET_VERSION_ID, + noncurrent.getVersionId()); + } + + /** + * The overwritten version keeps its blocks in the versionedKeyTable: they + * must not be queued for reclamation, and they must not be carried into the + * new current version's in-record block version list either. + */ + @Test + public void testOverwriteDoesNotReclaimOrInheritPreviousBlocks() + throws Exception { + setupVersionedBucket(); + String ozoneKey = seedCurrentVersion(100L); + + commitAt(500L); + + assertNull(omMetadataManager.getDeletedTable().get(ozoneKey)); + OmKeyInfo current = + omMetadataManager.getKeyTable(getBucketLayout()).get(ozoneKey); + assertEquals(1, current.getKeyLocationVersions().size()); + assertNotNull(noncurrentVersion(100L)); + } + + /** + * An hsync re-commit keeps updating the version it opened rather than + * creating a new one, so its versionId stays frozen and nothing moves to the + * versionedKeyTable. + */ + @Test + public void testHsyncRecommitKeepsVersionIdFrozen() throws Exception { + setupVersionedBucket(); + + commitAt(500L, true); + OmKeyInfo firstCommit = omMetadataManager.getKeyTable(getBucketLayout()) + .get(omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)); + assertEquals(500L, firstCommit.getVersionId()); + + commitAt(600L, true); + OmKeyInfo recommitted = omMetadataManager.getKeyTable(getBucketLayout()) + .get(omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)); + assertEquals(500L, recommitted.getVersionId()); + assertNull(noncurrentVersion(500L)); + } +} From 141476962edb9118f80f8aff94b8fdcf0006c2e5 Mon Sep 17 00:00:00 2001 From: Symious Date: Wed, 29 Jul 2026 14:10:48 +0800 Subject: [PATCH 14/23] T3.2. Insert a delete marker on DELETE without a versionId Co-Authored-By: Claude Opus 5 --- .../om/request/key/OMKeyDeleteRequest.java | 223 ++++++++++++++---- .../key/OMKeyDeleteMarkerResponse.java | 86 +++++++ .../key/TestOMKeyVersioningRequests.java | 120 ++++++++++ 3 files changed, 380 insertions(+), 49 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index 26287ca66d26..dc614fc02af6 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -24,8 +24,13 @@ import java.io.IOException; import java.nio.file.InvalidPathException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Objects; +import org.apache.hadoop.hdds.client.RatisReplicationConfig; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; 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; @@ -42,11 +47,14 @@ import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup; +import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; import org.apache.hadoop.ozone.om.request.validation.ValidationCondition; import org.apache.hadoop.ozone.om.request.validation.ValidationContext; import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteMarkerResponse; import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyRequest; @@ -128,6 +136,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut boolean acquiredLock = false; OMClientResponse omClientResponse = null; Result result = null; + // whether the request removed a key that was visible to plain reads; a + // delete marker supersedes the current version without removing anything + boolean visibleKeyRemoved = false; + // whether the request took the delete marker path, and so may have left + // versionedKeyTable entries in the table cache for this transaction + boolean insertingDeleteMarker = false; long startNanos = Time.monotonicNowNanos(); try { String objectKey = @@ -142,57 +156,73 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable(getBucketLayout()).get(objectKey); - if (omKeyInfo == null) { - throw new OMException("Key not found", KEY_NOT_FOUND); - } - - validateIfMatchETag(keyArgs, omKeyInfo); - - // Set the UpdateID to current transactionLogIndex - omKeyInfo = omKeyInfo.toBuilder() - .setUpdateID(trxnLogIndex) - .build(); - - // Update table cache. Put a tombstone entry - omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry( - new CacheKey<>( - omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)), - CacheValue.get(trxnLogIndex)); OmBucketInfo omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName); - long quotaReleased = sumBlockLengths(omKeyInfo); - // Empty entries won't be added to deleted table so this key shouldn't get added to snapshotUsed space. - boolean isKeyNonEmpty = !OmKeyInfo.isKeyEmpty(omKeyInfo); - omBucketInfo.decrUsedBytes(quotaReleased, isKeyNonEmpty); - omBucketInfo.decrUsedNamespace(1L, isKeyNonEmpty); - OmKeyInfo deletedOpenKeyInfo = null; - - // If omKeyInfo has hsync metadata, delete its corresponding open key as well - String dbOpenKey = null; - String hsyncClientId = omKeyInfo.getMetadata().get(OzoneConsts.HSYNC_CLIENT_ID); - if (hsyncClientId != null) { - Table openKeyTable = omMetadataManager.getOpenKeyTable(getBucketLayout()); - dbOpenKey = omMetadataManager.getOpenKey(volumeName, bucketName, keyName, hsyncClientId); - OmKeyInfo openKeyInfo = openKeyTable.get(dbOpenKey); - if (openKeyInfo != null) { - openKeyInfo = openKeyInfo.withMetadataMutations( - metadata -> metadata.put(DELETED_HSYNC_KEY, "true")); - openKeyTable.addCacheEntry(dbOpenKey, openKeyInfo, trxnLogIndex); - deletedOpenKeyInfo = openKeyInfo; - } else { - LOG.warn("Potentially inconsistent DB state: open key not found with dbOpenKey '{}'", dbOpenKey); + if (omBucketInfo.isS3VersioningEnabled()) { + // A delete without a versionId removes no data: a delete marker + // becomes the current version and the version it supersedes moves to + // the versionedKeyTable. Like S3, the marker is inserted even when the + // key does not exist. + insertingDeleteMarker = true; + omClientResponse = insertDeleteMarker(ozoneManager, omMetadataManager, + omBucketInfo, omKeyInfo, objectKey, keyArgs, trxnLogIndex, + omResponse); + // The key stays in the keyTable as a marker, so nothing is released; + // only superseding a visible object is one fewer visible key. + visibleKeyRemoved = omKeyInfo != null && !omKeyInfo.isDeleteMarker(); + } else { + if (omKeyInfo == null) { + throw new OMException("Key not found", KEY_NOT_FOUND); } - } - omClientResponse = new OMKeyDeleteResponse( - omResponse.setDeleteKeyResponse(DeleteKeyResponse.newBuilder()) - .build(), omKeyInfo, - omBucketInfo.copyObject(), deletedOpenKeyInfo); - if (omKeyInfo.isFile()) { - auditMap.put(OzoneConsts.DATA_SIZE, String.valueOf(omKeyInfo.getDataSize())); - auditMap.put(OzoneConsts.REPLICATION_CONFIG, omKeyInfo.getReplicationConfig().toString()); + validateIfMatchETag(keyArgs, omKeyInfo); + + // Set the UpdateID to current transactionLogIndex + omKeyInfo = omKeyInfo.toBuilder() + .setUpdateID(trxnLogIndex) + .build(); + + // Update table cache. Put a tombstone entry + omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry( + new CacheKey<>( + omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)), + CacheValue.get(trxnLogIndex)); + + long quotaReleased = sumBlockLengths(omKeyInfo); + // Empty entries won't be added to deleted table so this key shouldn't get added to snapshotUsed space. + boolean isKeyNonEmpty = !OmKeyInfo.isKeyEmpty(omKeyInfo); + omBucketInfo.decrUsedBytes(quotaReleased, isKeyNonEmpty); + omBucketInfo.decrUsedNamespace(1L, isKeyNonEmpty); + OmKeyInfo deletedOpenKeyInfo = null; + + // If omKeyInfo has hsync metadata, delete its corresponding open key as well + String dbOpenKey = null; + String hsyncClientId = omKeyInfo.getMetadata().get(OzoneConsts.HSYNC_CLIENT_ID); + if (hsyncClientId != null) { + Table openKeyTable = omMetadataManager.getOpenKeyTable(getBucketLayout()); + dbOpenKey = omMetadataManager.getOpenKey(volumeName, bucketName, keyName, hsyncClientId); + OmKeyInfo openKeyInfo = openKeyTable.get(dbOpenKey); + if (openKeyInfo != null) { + openKeyInfo = openKeyInfo.withMetadataMutations( + metadata -> metadata.put(DELETED_HSYNC_KEY, "true")); + openKeyTable.addCacheEntry(dbOpenKey, openKeyInfo, trxnLogIndex); + deletedOpenKeyInfo = openKeyInfo; + } else { + LOG.warn("Potentially inconsistent DB state: open key not found with dbOpenKey '{}'", dbOpenKey); + } + } + + omClientResponse = new OMKeyDeleteResponse( + omResponse.setDeleteKeyResponse(DeleteKeyResponse.newBuilder()) + .build(), omKeyInfo, + omBucketInfo.copyObject(), deletedOpenKeyInfo); + if (omKeyInfo.isFile()) { + auditMap.put(OzoneConsts.DATA_SIZE, String.valueOf(omKeyInfo.getDataSize())); + auditMap.put(OzoneConsts.REPLICATION_CONFIG, omKeyInfo.getReplicationConfig().toString()); + } + visibleKeyRemoved = true; } result = Result.SUCCESS; @@ -201,9 +231,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut } catch (IOException | InvalidPathException ex) { result = Result.FAILURE; exception = ex; - omClientResponse = - new OMKeyDeleteResponse(createErrorOMResponse(omResponse, exception), - getBucketLayout()); + // The failure response has to declare the same tables as the successful + // one: the double buffer cleans up the table cache from the response's + // CleanupTableInfo, so a delete marker request that failed after + // touching the versionedKeyTable cache would otherwise leave an entry + // behind that is in no DB and is never cleaned up. + OMResponse errorResponse = createErrorOMResponse(omResponse, exception); + omClientResponse = insertingDeleteMarker + ? new OMKeyDeleteMarkerResponse(errorResponse, getBucketLayout()) + : new OMKeyDeleteResponse(errorResponse, getBucketLayout()); long endNanosDeleteKeyFailureLatencyNs = Time.monotonicNowNanos(); perfMetrics.setDeleteKeyFailureLatencyNs(endNanosDeleteKeyFailureLatencyNs - startNanos); } finally { @@ -222,7 +258,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut switch (result) { case SUCCESS: - omMetrics.decNumKeys(); + if (visibleKeyRemoved) { + omMetrics.decNumKeys(); + } LOG.debug("Key deleted. Volume:{}, Bucket:{}, Key:{}", volumeName, bucketName, keyName); break; @@ -239,6 +277,93 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut return omClientResponse; } + /** + * Supersedes the key's current version with a delete marker: a record with + * no data blocks that makes plain reads of the key return KEY_NOT_FOUND + * while every existing version stays readable by versionId. The superseded + * current version becomes a noncurrent version; a record written before + * versioning was enabled carries no versionId and becomes the key's null + * version. When the key does not exist the marker is still inserted, as S3 + * does. + */ + @SuppressWarnings("checkstyle:ParameterNumber") + private OMClientResponse insertDeleteMarker(OzoneManager ozoneManager, + OMMetadataManager omMetadataManager, OmBucketInfo omBucketInfo, + OmKeyInfo currentVersion, String objectKey, + OzoneManagerProtocolProtos.KeyArgs keyArgs, long trxnLogIndex, + OMResponse.Builder omResponse) throws IOException { + + String volumeName = omBucketInfo.getVolumeName(); + String bucketName = omBucketInfo.getBucketName(); + String keyName = keyArgs.getKeyName(); + + // Everything that can fail runs before the first cache entry is added: a + // request that throws here is answered with an OMKeyDeleteResponse, whose + // cleanup does not cover the versionedKeyTable, so a cache entry left + // behind would never be removed and would outlive the failed request. + long markerVersionId = ozoneManager.getVersionIdAllocator().allocate( + omMetadataManager, volumeName, bucketName, keyName, trxnLogIndex, + currentVersion); + // the marker is a record of its own; it holds no blocks, so it consumes + // namespace but no space + checkBucketQuotaInNamespace(omBucketInfo, 1L); + + OmKeyInfo.Builder markerBuilder; + if (currentVersion != null) { + markerBuilder = currentVersion.toBuilder() + .setMetadata(new HashMap<>()) + .setTags(new HashMap<>()) + .setFileChecksum(null); + } else { + markerBuilder = new OmKeyInfo.Builder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(keyName) + .setReplicationConfig(RatisReplicationConfig.getInstance( + ReplicationFactor.ONE)) + .setObjectID(ozoneManager.getObjectIdFromTxId(trxnLogIndex)) + .setOwnerName(omBucketInfo.getOwner()) + .setFile(true); + } + OmKeyInfo deleteMarker = markerBuilder + .setOmKeyLocationInfos(Collections.singletonList( + new OmKeyLocationInfoGroup(0, new ArrayList<>()))) + .setDataSize(0L) + .setCreationTime(keyArgs.getModificationTime()) + .setModificationTime(keyArgs.getModificationTime()) + .setUpdateID(trxnLogIndex) + .setVersionId(markerVersionId) + .setDeleteMarker(true) + .setNullVersion(false) + .build(); + + String movedVersionedKeyName = null; + OmKeyInfo movedVersionedKeyInfo = null; + if (currentVersion != null) { + movedVersionedKeyInfo = currentVersion.getVersionId() != null + ? currentVersion + : currentVersion.toBuilder() + .setVersionId(VersionIdGenerator.UNSET_VERSION_ID) + .setNullVersion(true) + .build(); + movedVersionedKeyName = omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, + movedVersionedKeyInfo.getVersionId()); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + movedVersionedKeyName, movedVersionedKeyInfo, trxnLogIndex); + } + + omBucketInfo.incrUsedNamespace(1L); + + omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry( + objectKey, deleteMarker, trxnLogIndex); + + return new OMKeyDeleteMarkerResponse( + omResponse.setDeleteKeyResponse(DeleteKeyResponse.newBuilder()).build(), + deleteMarker, objectKey, movedVersionedKeyName, movedVersionedKeyInfo, + omBucketInfo.copyObject()); + } + /** * Validates key delete requests. * We do not want to allow older clients to delete keys in buckets which use diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java new file mode 100644 index 000000000000..822596d57c9f --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java @@ -0,0 +1,86 @@ +/* + * 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.response.key; + +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VERSIONED_KEY_TABLE; + +import jakarta.annotation.Nonnull; +import java.io.IOException; +import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.response.CleanupTableInfo; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; + +/** + * Response for a DeleteKey request on a bucket with S3-compatible versioning + * enabled: no data is removed. A delete marker becomes the current version in + * the keyTable, and the version it supersedes (if the key existed) moves to + * the versionedKeyTable. + */ +@CleanupTableInfo(cleanupTables = {KEY_TABLE, VERSIONED_KEY_TABLE, BUCKET_TABLE}) +public class OMKeyDeleteMarkerResponse extends OmKeyResponse { + + private OmKeyInfo deleteMarker; + private String ozoneKeyName; + private String movedVersionedKeyName; + private OmKeyInfo movedVersionedKeyInfo; + private OmBucketInfo omBucketInfo; + + public OMKeyDeleteMarkerResponse(@Nonnull OMResponse omResponse, + @Nonnull OmKeyInfo deleteMarker, @Nonnull String ozoneKeyName, + String movedVersionedKeyName, OmKeyInfo movedVersionedKeyInfo, + @Nonnull OmBucketInfo omBucketInfo) { + super(omResponse, omBucketInfo.getBucketLayout()); + this.deleteMarker = deleteMarker; + this.ozoneKeyName = ozoneKeyName; + this.movedVersionedKeyName = movedVersionedKeyName; + this.movedVersionedKeyInfo = movedVersionedKeyInfo; + this.omBucketInfo = omBucketInfo; + } + + /** + * For when the request is not successful. + * For a successful request, the other constructor should be used. + */ + public OMKeyDeleteMarkerResponse(@Nonnull OMResponse omResponse, + @Nonnull BucketLayout bucketLayout) { + super(omResponse, bucketLayout); + checkStatusNotOK(); + } + + @Override + public void addToDBBatch(OMMetadataManager omMetadataManager, + BatchOperation batchOperation) throws IOException { + omMetadataManager.getKeyTable(getBucketLayout()) + .putWithBatch(batchOperation, ozoneKeyName, deleteMarker); + + if (movedVersionedKeyInfo != null) { + omMetadataManager.getVersionedKeyTable().putWithBatch(batchOperation, + movedVersionedKeyName, movedVersionedKeyInfo); + } + + omMetadataManager.getBucketTable().putWithBatch(batchOperation, + omMetadataManager.getBucketKey(omBucketInfo.getVolumeName(), + omBucketInfo.getBucketName()), omBucketInfo); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java index c8211d432be8..db2b57c5bcb5 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -35,6 +35,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.CommitKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.util.Time; @@ -69,9 +70,15 @@ private void setupVersionedBucket() throws Exception { /** Puts a current version into keyTable, as an earlier write would have. */ private String seedCurrentVersion(Long versionId) throws Exception { + return seedCurrentVersion(versionId, false); + } + + private String seedCurrentVersion(Long versionId, boolean deleteMarker) + throws Exception { OmKeyInfo keyInfo = OMRequestTestUtils.createOmKeyInfo( volumeName, bucketName, keyName, replicationConfig) .setVersionId(versionId) + .setDeleteMarker(deleteMarker) .build(); String ozoneKey = omMetadataManager.getOzoneKey( volumeName, bucketName, keyName); @@ -79,6 +86,32 @@ private String seedCurrentVersion(Long versionId) throws Exception { return ozoneKey; } + private OMRequest deleteRequest() { + KeyArgs keyArgs = KeyArgs.newBuilder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(keyName) + .setModificationTime(Time.now()) + .build(); + return OMRequest.newBuilder() + .setDeleteKeyRequest(DeleteKeyRequest.newBuilder().setKeyArgs(keyArgs)) + .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKey) + .setClientId(UUID.randomUUID().toString()).build(); + } + + private OMClientResponse deleteAt(long trxnLogIndex) throws Exception { + OMClientResponse response = new OMKeyDeleteRequest(deleteRequest(), + getBucketLayout()).validateAndUpdateCache(ozoneManager, trxnLogIndex); + assertEquals(OzoneManagerProtocolProtos.Status.OK, + response.getOMResponse().getStatus()); + return response; + } + + private OmKeyInfo currentVersion() throws Exception { + return omMetadataManager.getKeyTable(getBucketLayout()).get( + omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)); + } + private OMRequest commitRequest(boolean isHsync) { KeyArgs keyArgs = KeyArgs.newBuilder() .setVolumeName(volumeName) @@ -210,4 +243,91 @@ public void testHsyncRecommitKeepsVersionIdFrozen() throws Exception { assertEquals(500L, recommitted.getVersionId()); assertNull(noncurrentVersion(500L)); } + + @Test + public void testDeleteInsertsMarkerAndKeepsPreviousVersion() + throws Exception { + setupVersionedBucket(); + seedCurrentVersion(100L); + + deleteAt(200L); + + OmKeyInfo current = currentVersion(); + assertNotNull(current); + assertTrue(current.isDeleteMarker()); + assertEquals(200L, current.getVersionId()); + assertEquals(0L, current.getDataSize()); + assertTrue(current.getLatestVersionLocations().getLocationList().isEmpty()); + + OmKeyInfo noncurrent = noncurrentVersion(100L); + assertNotNull(noncurrent); + assertFalse(noncurrent.isDeleteMarker()); + } + + /** S3 inserts a delete marker even for a key that does not exist. */ + @Test + public void testDeleteOfMissingKeyStillInsertsMarker() throws Exception { + setupVersionedBucket(); + + deleteAt(200L); + + OmKeyInfo current = currentVersion(); + assertNotNull(current); + assertTrue(current.isDeleteMarker()); + assertEquals(200L, current.getVersionId()); + assertNull(noncurrentVersion(VersionIdGenerator.UNSET_VERSION_ID)); + } + + /** Deleting a key whose current version is already a marker stacks another. */ + @Test + public void testDeleteStacksAnotherMarker() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(100L, true); + + deleteAt(200L); + + OmKeyInfo current = currentVersion(); + assertTrue(current.isDeleteMarker()); + assertEquals(200L, current.getVersionId()); + + OmKeyInfo stacked = noncurrentVersion(100L); + assertNotNull(stacked); + assertTrue(stacked.isDeleteMarker()); + } + + @Test + public void testDeleteOfPreVersioningRecordMovesItToNullVersion() + throws Exception { + setupVersionedBucket(); + seedCurrentVersion(null); + + deleteAt(200L); + + OmKeyInfo noncurrent = + noncurrentVersion(VersionIdGenerator.UNSET_VERSION_ID); + assertNotNull(noncurrent); + assertTrue(noncurrent.isNullVersion()); + assertFalse(noncurrent.isDeleteMarker()); + } + + /** + * A delete marker holds no blocks, so it consumes namespace but no space, + * and the superseded version keeps its own usage. + */ + @Test + public void testDeleteMarkerConsumesNamespaceButNoSpace() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(100L); + String bucketKey = + omMetadataManager.getBucketKey(volumeName, bucketName); + OmBucketInfo before = omMetadataManager.getBucketTable().get(bucketKey); + long usedBytes = before.getUsedBytes(); + long usedNamespace = before.getUsedNamespace(); + + deleteAt(200L); + + OmBucketInfo after = omMetadataManager.getBucketTable().get(bucketKey); + assertEquals(usedBytes, after.getUsedBytes()); + assertEquals(usedNamespace + 1, after.getUsedNamespace()); + } } From e75f103c7a02e82d8107587f6f18aea6c8f63440 Mon Sep 17 00:00:00 2001 From: Symious Date: Wed, 29 Jul 2026 14:17:58 +0800 Subject: [PATCH 15/23] T3.3. Count every version against the bucket quota Co-Authored-By: Claude Opus 5 --- .../key/TestOMKeyVersioningRequests.java | 121 +++++++++++++++++- 1 file changed, 114 insertions(+), 7 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java index db2b57c5bcb5..097f6b1abeb4 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -26,13 +27,16 @@ import java.util.UUID; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.BucketVersioningStatus; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.QuotaUtil; import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteMarkerResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CommitKeyRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyRequest; @@ -55,12 +59,25 @@ public BucketLayout getBucketLayout() { } private void setupVersionedBucket() throws Exception { + setupVersionedBucket(OzoneConsts.QUOTA_RESET, OzoneConsts.QUOTA_RESET); + } + + private void setupVersionedBucket(long quotaInBytes, long quotaInNamespace) + throws Exception { + setupVersionedBucket(quotaInBytes, quotaInNamespace, 0L); + } + + private void setupVersionedBucket(long quotaInBytes, long quotaInNamespace, + long usedNamespace) throws Exception { OMRequestTestUtils.addVolumeToDB(volumeName, omMetadataManager); OmBucketInfo bucketInfo = OmBucketInfo.newBuilder() .setVolumeName(volumeName) .setBucketName(bucketName) .setBucketLayout(BucketLayout.OBJECT_STORE) .setVersioningStatus(BucketVersioningStatus.ENABLED) + .setQuotaInBytes(quotaInBytes) + .setQuotaInNamespace(quotaInNamespace) + .setUsedNamespace(usedNamespace) .setCreationTime(Time.now()) .build(); omMetadataManager.getBucketTable().addCacheEntry( @@ -112,18 +129,19 @@ private OmKeyInfo currentVersion() throws Exception { omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)); } - private OMRequest commitRequest(boolean isHsync) { + private OMRequest commitRequest(boolean isHsync, long dataSize, + long writerClientId) { KeyArgs keyArgs = KeyArgs.newBuilder() .setVolumeName(volumeName) .setBucketName(bucketName) .setKeyName(keyName) .setModificationTime(Time.now()) - .setDataSize(0) + .setDataSize(dataSize) .build(); return OMRequest.newBuilder() .setCommitKeyRequest(CommitKeyRequest.newBuilder() .setKeyArgs(keyArgs) - .setClientID(clientID) + .setClientID(writerClientId) .setHsync(isHsync)) .setCmdType(OzoneManagerProtocolProtos.Type.CommitKey) .setClientId(UUID.randomUUID().toString()).build(); @@ -135,15 +153,27 @@ private OMClientResponse commitAt(long trxnLogIndex) throws Exception { private OMClientResponse commitAt(long trxnLogIndex, boolean isHsync) throws Exception { - OMRequestTestUtils.addKeyToTable(true, volumeName, bucketName, keyName, - clientID, replicationConfig, omMetadataManager); - OMClientResponse response = new OMKeyCommitRequest(commitRequest(isHsync), - getBucketLayout()).validateAndUpdateCache(ozoneManager, trxnLogIndex); + OMClientResponse response = + commitAt(trxnLogIndex, isHsync, 0L, clientID); assertEquals(OzoneManagerProtocolProtos.Status.OK, response.getOMResponse().getStatus()); return response; } + /** + * Commits the key as the writer identified by {@code writerClientId}. A + * non-hsync commit tombstones its own open key, so a second write of the + * same key comes from a different client, as it would in practice. + */ + private OMClientResponse commitAt(long trxnLogIndex, boolean isHsync, + long dataSize, long writerClientId) throws Exception { + OMRequestTestUtils.addKeyToTable(true, volumeName, bucketName, keyName, + writerClientId, replicationConfig, omMetadataManager); + return new OMKeyCommitRequest( + commitRequest(isHsync, dataSize, writerClientId), + getBucketLayout()).validateAndUpdateCache(ozoneManager, trxnLogIndex); + } + private OmKeyInfo noncurrentVersion(long versionId) throws Exception { return omMetadataManager.getVersionedKeyTable().get( omMetadataManager.getVersionedOzoneKey( @@ -310,6 +340,83 @@ public void testDeleteOfPreVersioningRecordMovesItToNullVersion() assertFalse(noncurrent.isDeleteMarker()); } + /** + * Every version counts against the bucket's space quota: an overwrite adds + * the new version's usage without releasing the version it supersedes. + */ + @Test + public void testEachVersionCountsAgainstUsedBytes() throws Exception { + setupVersionedBucket(); + String bucketKey = + omMetadataManager.getBucketKey(volumeName, bucketName); + + assertEquals(OzoneManagerProtocolProtos.Status.OK, + commitAt(500L, false, 100L, clientID).getOMResponse().getStatus()); + long afterFirst = omMetadataManager.getBucketTable().get(bucketKey) + .getUsedBytes(); + assertEquals(QuotaUtil.getReplicatedSize(100L, replicationConfig), + afterFirst); + + assertEquals(OzoneManagerProtocolProtos.Status.OK, + commitAt(600L, false, 300L, clientID + 1).getOMResponse().getStatus()); + long afterSecond = omMetadataManager.getBucketTable().get(bucketKey) + .getUsedBytes(); + assertEquals(afterFirst + + QuotaUtil.getReplicatedSize(300L, replicationConfig), + afterSecond); + assertEquals(2, omMetadataManager.getBucketTable().get(bucketKey) + .getUsedNamespace()); + } + + @Test + public void testVersionedWriteRejectedWhenSpaceQuotaExceeded() + throws Exception { + long quota = QuotaUtil.getReplicatedSize(150L, replicationConfig); + setupVersionedBucket(quota, OzoneConsts.QUOTA_RESET); + + assertEquals(OzoneManagerProtocolProtos.Status.OK, + commitAt(500L, false, 100L, clientID).getOMResponse().getStatus()); + // the first version is not released, so the second one no longer fits + assertEquals(OzoneManagerProtocolProtos.Status.QUOTA_EXCEEDED, + commitAt(600L, false, 100L, clientID + 1).getOMResponse().getStatus()); + + OmKeyInfo current = currentVersion(); + assertEquals(500L, current.getVersionId()); + assertNull(noncurrentVersion(500L)); + } + + @Test + public void testVersionedWriteRejectedWhenNamespaceQuotaExceeded() + throws Exception { + setupVersionedBucket(OzoneConsts.QUOTA_RESET, 1L); + + assertEquals(OzoneManagerProtocolProtos.Status.OK, + commitAt(500L, false, 0L, clientID).getOMResponse().getStatus()); + // each version is a record of its own, so the second one needs namespace + assertEquals(OzoneManagerProtocolProtos.Status.QUOTA_EXCEEDED, + commitAt(600L, false, 0L, clientID + 1).getOMResponse().getStatus()); + } + + /** A delete marker is a record too, so it needs namespace quota. */ + @Test + public void testDeleteMarkerRejectedWhenNamespaceQuotaExceeded() + throws Exception { + // the bucket already holds the one key its namespace quota allows + setupVersionedBucket(OzoneConsts.QUOTA_RESET, 1L, 1L); + seedCurrentVersion(100L); + + OMClientResponse response = new OMKeyDeleteRequest(deleteRequest(), + getBucketLayout()).validateAndUpdateCache(ozoneManager, 200L); + assertEquals(OzoneManagerProtocolProtos.Status.QUOTA_EXCEEDED, + response.getOMResponse().getStatus()); + assertFalse(currentVersion().isDeleteMarker()); + // a rejected request must not leave the superseded version behind in the + // versionedKeyTable cache, and its response has to declare that table so + // that the double buffer cleans up whatever the request did touch + assertNull(noncurrentVersion(100L)); + assertInstanceOf(OMKeyDeleteMarkerResponse.class, response); + } + /** * A delete marker holds no blocks, so it consumes namespace but no space, * and the superseded version keeps its own usage. From 0737502a31d6d1947922336d64f271822e312e37 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 31 Jul 2026 15:18:17 +0800 Subject: [PATCH 16/23] T3. Base block reclamation on the versioning status, not the legacy flag The reclaim branches skipped S3-versioned buckets only because the legacy isVersionEnabled flag is kept in sync with an ENABLED status. Depend on the status directly, so that dropping that sync cannot strand a version record by reclaiming the blocks it still refers to. OMKeyCommitRequestWithFSO is left alone: isS3VersioningEnabled() requires the OBJECT_STORE layout, so the check is structurally false on the FSO path. Co-Authored-By: Claude Opus 5 --- .../hadoop/ozone/om/request/key/OMKeyCommitRequest.java | 5 ++++- .../s3/multipart/S3MultipartUploadCompleteRequest.java | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index c495db1fde57..5f135b5e92cb 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -335,7 +335,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut correctedSpace -= keyToDelete.getReplicatedSize(); checkBucketQuotaInBytes(omMetadataManager, omBucketInfo, correctedSpace); - } else if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled()) { + } else if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled() + && !s3Versioning) { + // Under S3 versioning the overwritten version is kept in the + // versionedKeyTable, so its blocks must not be reclaimed here. RepeatedOmKeyInfo oldVerKeyInfo = getOldVersionsToCleanUp( keyToDelete, omBucketInfo.getObjectID(), trxnLogIndex); // using pseudoObjId as objectId can be same in case of overwrite key diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java index 841ced7dacce..18876e982874 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java @@ -331,7 +331,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut OmKeyInfo keyToDelete = omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey); boolean isNamespaceUpdate = false; - if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled()) { + // The S3 versioning check is redundant while the legacy flag is kept in + // sync with an ENABLED status, but the reclaim must depend on the + // status rather than on that sync: dropping the previous version's + // blocks would strand the version record kept for it. + if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled() + && !omBucketInfo.isS3VersioningEnabled()) { RepeatedOmKeyInfo oldKeyVersionsToDelete = getOldVersionsToCleanUp( keyToDelete, omBucketInfo.getObjectID(), trxnLogIndex); allKeyInfoToRemove.addAll(oldKeyVersionsToDelete.getOmKeyInfoList()); From a9d68eaa756ae97f32d4f6af3f114565fb60b865 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 31 Jul 2026 16:13:06 +0800 Subject: [PATCH 17/23] T4.1. Read a specific object version by versionId KeyArgs gains versionId and nullVersion: a null version carries a normally generated id like any other version, so the null slot needs a selector of its own rather than a reserved id. The current version is checked before the versionedKeyTable, so naming it costs no extra read, and the null slot is found by a bounded scan of the key's version prefix. Addressing a delete marker by version is reported as KEY_IS_DELETE_MARKER rather than KEY_NOT_FOUND: S3 answers 405 for it and 404 only for a read that lands on a current marker without naming a version. The status mapping itself belongs to the gateway. Co-Authored-By: Claude Opus 5 --- .../ozone/om/exceptions/OMException.java | 6 + .../hadoop/ozone/om/helpers/OmKeyArgs.java | 47 +++++ .../src/main/proto/OmClientProtocol.proto | 12 ++ .../hadoop/ozone/om/KeyManagerImpl.java | 58 ++++++ .../OzoneManagerRequestHandler.java | 4 + .../hadoop/ozone/om/TestKeyManagerUnit.java | 183 ++++++++++++++++++ 6 files changed, 310 insertions(+) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java index 240e99e7d673..6eeb9fd071d8 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/exceptions/OMException.java @@ -281,5 +281,11 @@ public enum ResultCodes { ETAG_NOT_AVAILABLE, ATOMIC_WRITE_CONFLICT, + + /** + * The addressed version exists but is a delete marker. Kept distinct from + * KEY_NOT_FOUND because S3 answers 405 rather than 404 for it. + */ + KEY_IS_DELETE_MARKER, } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java index 5d2de09c48e5..5ea0d4aadcfc 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java @@ -62,6 +62,11 @@ public final class OmKeyArgs extends WithMetadata implements Auditable { // been modified. private Long expectedDataGeneration = null; private final String expectedETag; + // Addresses one specific version of the key instead of its current version. + // versionId names a version by id; nullVersion selects the key's null version + // slot, which has no fixed id of its own. At most one of the two is set. + private final Long versionId; + private final boolean nullVersion; private OmKeyArgs(Builder b) { super(b); @@ -84,6 +89,26 @@ private OmKeyArgs(Builder b) { this.tags = b.tags.build(); this.expectedDataGeneration = b.expectedDataGeneration; this.expectedETag = b.expectedETag; + this.versionId = b.versionId; + this.nullVersion = b.nullVersion; + } + + /** + * @return the addressed versionId, or null when no specific version is + * addressed by id + */ + public Long getVersionId() { + return versionId; + } + + /** @return whether the key's null version slot is addressed */ + public boolean isNullVersion() { + return nullVersion; + } + + /** @return whether a version other than the current one is addressed */ + public boolean addressesVersion() { + return versionId != null || nullVersion; } public boolean getIsMultipartKey() { @@ -240,6 +265,8 @@ public static class Builder extends WithMetadata.Builder { private final AclListBuilder acls; private boolean recursive; private boolean headOp; + private Long versionId; + private boolean nullVersion; private boolean forceUpdateContainerCacheFromSCM; private final MapBuilder tags; private Long expectedDataGeneration = null; @@ -290,6 +317,8 @@ public Builder(OmKeyArgs obj) { obj.forceUpdateContainerCacheFromSCM; this.expectedDataGeneration = obj.expectedDataGeneration; this.expectedETag = obj.expectedETag; + this.versionId = obj.versionId; + this.nullVersion = obj.nullVersion; this.tags = MapBuilder.of(obj.tags); this.acls = AclListBuilder.of(obj.acls); } @@ -410,6 +439,24 @@ public Builder setRecursive(boolean isRecursive) { return this; } + /** Addresses the version with this id; clears any null-version selection. */ + public Builder setVersionId(Long id) { + this.versionId = id; + if (id != null) { + this.nullVersion = false; + } + return this; + } + + /** Addresses the key's null version slot; clears any addressed versionId. */ + public Builder setNullVersion(boolean isNullVersion) { + this.nullVersion = isNullVersion; + if (isNullVersion) { + this.versionId = null; + } + return this; + } + public Builder setHeadOp(boolean isHeadOp) { this.headOp = isHeadOp; return this; diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 9e9a658c622a..a135d0a8bf67 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -598,6 +598,10 @@ enum Status { ETAG_NOT_AVAILABLE = 100; ATOMIC_WRITE_CONFLICT = 101; + + // The addressed version exists but is a delete marker. Distinct from + // KEY_NOT_FOUND: the S3 Gateway maps it to 405, not 404. + KEY_IS_DELETE_MARKER = 102; } /** @@ -1141,6 +1145,14 @@ message KeyArgs { // the given ETag for the operation to succeed. This is used for // S3 conditional writes with the If-Match header. optional string expectedETag = 24; + + // S3-compatible object versioning: addresses one specific version of the key + // instead of its current version. nullVersion selects the key's null version + // slot, which cannot be addressed by id because a null version carries a + // normally generated versionId like any other version. At most one of the + // two may be set. + optional uint64 versionId = 25; + optional bool nullVersion = 26; } message KeyLocation { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 3232f9b1ff33..991903d49f9b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -601,15 +601,31 @@ private OmKeyInfo readKeyInfo(OmKeyArgs args, BucketLayout bucketLayout) bucketLayout); if (bucketLayout.isFileSystemOptimized()) { + if (args.addressesVersion()) { + throw new OMException("Object versioning is only supported on " + + BucketLayout.OBJECT_STORE + " buckets", + ResultCodes.NOT_SUPPORTED_OPERATION); + } value = getOmKeyInfoFSO(volumeName, bucketName, keyName); } else { value = getOmKeyInfo(volumeName, bucketName, keyName, bucketLayout); + if (args.addressesVersion()) { + value = getAddressedVersion(args, volumeName, bucketName, keyName, + value); + } if (value != null) { // For Legacy & OBS buckets, any key is a file by default. This is to // keep getKeyInfo compatible with OFS clients. value.setFile(true); } } + if (value != null && value.isDeleteMarker()) { + // Addressing a delete marker by version is answered with 405 by S3, + // while a request that lands on a current marker is a plain 404. + throw new OMException("Key: " + keyName + " is a delete marker", + args.addressesVersion() + ? ResultCodes.KEY_IS_DELETE_MARKER : ResultCodes.KEY_NOT_FOUND); + } } catch (IOException ex) { if (ex instanceof OMException) { throw ex; @@ -661,6 +677,48 @@ private OmKeyInfo getOmKeyInfo(String volumeName, String bucketName, .get(keyBytes); } + /** + * Resolves the version addressed by {@code args} for a key whose current + * version is {@code current}. The current version is checked first, so a + * request naming the current version costs no extra read; otherwise the + * version is looked up in the versionedKeyTable. + * + * @param current the key's current version, or null when the key has none + * @return the addressed version, or null when it does not exist + */ + private OmKeyInfo getAddressedVersion(OmKeyArgs args, String volumeName, + String bucketName, String keyName, OmKeyInfo current) throws IOException { + if (args.isNullVersion()) { + if (current != null && current.isNullVersion()) { + return current; + } + // The null version carries a normally generated versionId, so it can only + // be found by scanning the key's versions. The scan is bounded by the + // number of versions the key has and stops at the first match, since a key + // has at most one null version. + String prefix = metadataManager + .getVersionedOzoneKeyPrefix(volumeName, bucketName, keyName); + try (Table.KeyValueIterator versions = + metadataManager.getVersionedKeyTable().iterator(prefix)) { + while (versions.hasNext()) { + OmKeyInfo version = versions.next().getValue(); + if (version.isNullVersion()) { + return version; + } + } + } + return null; + } + + long versionId = args.getVersionId(); + if (current != null && current.getVersionId() != null + && current.getVersionId() == versionId) { + return current; + } + return metadataManager.getVersionedKeyTable().get(metadataManager + .getVersionedOzoneKey(volumeName, bucketName, keyName, versionId)); + } + /** * Look up will return only closed fileInfo. This will return null if the * keyName is a directory or if the keyName is still open for writing. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java index 7359065986ab..0e6a589f66db 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java @@ -645,6 +645,8 @@ private LookupKeyResponse lookupKey(LookupKeyRequest request, .setLatestVersionLocation(keyArgs.getLatestVersionLocation()) .setSortDatanodesInPipeline(keyArgs.getSortDatanodes()) .setHeadOp(keyArgs.getHeadOp()) + .setVersionId(keyArgs.hasVersionId() ? keyArgs.getVersionId() : null) + .setNullVersion(keyArgs.getNullVersion()) .build(); OmKeyInfo keyInfo = impl.lookupKey(omKeyArgs); @@ -666,6 +668,8 @@ private GetKeyInfoResponse getKeyInfo(GetKeyInfoRequest request, .setForceUpdateContainerCacheFromSCM( keyArgs.getForceUpdateContainerCacheFromSCM()) .setMultipartUploadPartNumber(keyArgs.getMultipartNumber()) + .setVersionId(keyArgs.hasVersionId() ? keyArgs.getVersionId() : null) + .setNullVersion(keyArgs.getNullVersion()) .build(); KeyInfoWithVolumeContext keyInfo = impl.getKeyInfo(omKeyArgs, request.getAssumeS3Context()); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index 9b1844212073..b2753e659947 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -23,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.anySet; import static org.mockito.Mockito.mock; @@ -64,6 +65,7 @@ import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; @@ -579,6 +581,187 @@ public void testGetKeyInfo() throws IOException { .getContainerWithPipelineBatch(containerIDs); } + private OmKeyInfo versionedKeyInfo(String volume, String bucket, String key, + long versionId, boolean nullVersion, boolean deleteMarker) { + return new OmKeyInfo.Builder() + .setVolumeName(volume) + .setBucketName(bucket) + .setKeyName(key) + .setOmKeyLocationInfos(Collections.emptyList()) + .setCreationTime(Time.now()) + .setModificationTime(Time.now()) + .setDataSize(0) + .setReplicationConfig(RatisReplicationConfig.getInstance(ReplicationFactor.ONE)) + .setVersionId(versionId) + .setNullVersion(nullVersion) + .setDeleteMarker(deleteMarker) + .build(); + } + + @Test + public void testLookupKeyByVersionId() throws Exception { + String volume = "vol-ver"; + String bucket = "buck-ver"; + String key = "obj"; + OMRequestTestUtils.addVolumeAndBucketToDB(volume, bucket, metadataManager, + BucketLayout.OBJECT_STORE); + + // current version 30, one noncurrent version 20, and a null version 10 + metadataManager.getKeyTable(BucketLayout.OBJECT_STORE).put( + metadataManager.getOzoneKey(volume, bucket, key), + versionedKeyInfo(volume, bucket, key, 30L, false, false)); + metadataManager.getVersionedKeyTable().put( + metadataManager.getVersionedOzoneKey(volume, bucket, key, 20L), + versionedKeyInfo(volume, bucket, key, 20L, false, false)); + metadataManager.getVersionedKeyTable().put( + metadataManager.getVersionedOzoneKey(volume, bucket, key, 10L), + versionedKeyInfo(volume, bucket, key, 10L, true, false)); + + OmKeyArgs.Builder base = new OmKeyArgs.Builder() + .setVolumeName(volume).setBucketName(bucket).setKeyName(key) + .setHeadOp(true); + + // no versionId addresses the current version + OmKeyArgs args = base.build(); + assertEquals(30L, keyManager.lookupKey(args, resolveBucket(args), null) + .getVersionId()); + + // the current version can also be addressed by its id + args = base.setVersionId(30L).build(); + assertEquals(30L, keyManager.lookupKey(args, resolveBucket(args), null) + .getVersionId()); + + // a noncurrent version resolves through the versionedKeyTable + args = base.setVersionId(20L).build(); + assertEquals(20L, keyManager.lookupKey(args, resolveBucket(args), null) + .getVersionId()); + + // the null version slot is found by attribute, not by id + args = base.setNullVersion(true).build(); + OmKeyInfo nullVersion = + keyManager.lookupKey(args, resolveBucket(args), null); + assertEquals(10L, nullVersion.getVersionId()); + assertTrue(nullVersion.isNullVersion()); + + // an unknown versionId is a plain not-found + OmKeyArgs unknown = base.setVersionId(999L).build(); + OMException ex = assertThrows(OMException.class, + () -> keyManager.lookupKey(unknown, resolveBucket(unknown), null)); + assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, ex.getResult()); + } + + @Test + public void testLookupOfDeleteMarkerIsDistinguishable() throws Exception { + String volume = "vol-marker"; + String bucket = "buck-marker"; + String key = "obj"; + OMRequestTestUtils.addVolumeAndBucketToDB(volume, bucket, metadataManager, + BucketLayout.OBJECT_STORE); + + // current version is a marker, with a readable version behind it + metadataManager.getKeyTable(BucketLayout.OBJECT_STORE).put( + metadataManager.getOzoneKey(volume, bucket, key), + versionedKeyInfo(volume, bucket, key, 40L, false, true)); + metadataManager.getVersionedKeyTable().put( + metadataManager.getVersionedOzoneKey(volume, bucket, key, 20L), + versionedKeyInfo(volume, bucket, key, 20L, false, false)); + + OmKeyArgs.Builder base = new OmKeyArgs.Builder() + .setVolumeName(volume).setBucketName(bucket).setKeyName(key) + .setHeadOp(true); + + // without a versionId a current marker reads as absent + OmKeyArgs current = base.build(); + OMException ex = assertThrows(OMException.class, + () -> keyManager.lookupKey(current, resolveBucket(current), null)); + assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, ex.getResult()); + + // naming the marker's version is a different condition: S3 answers 405 + OmKeyArgs marker = base.setVersionId(40L).build(); + ex = assertThrows(OMException.class, + () -> keyManager.lookupKey(marker, resolveBucket(marker), null)); + assertEquals(OMException.ResultCodes.KEY_IS_DELETE_MARKER, ex.getResult()); + + // the version behind the marker stays readable + OmKeyArgs behind = base.setVersionId(20L).build(); + assertEquals(20L, + keyManager.lookupKey(behind, resolveBucket(behind), null).getVersionId()); + } + + @Test + public void testCurrentVersionIsResolvedWithoutReadingVersionedKeyTable() + throws Exception { + String volume = "vol-cur"; + String bucket = "buck-cur"; + String key = "obj"; + OMRequestTestUtils.addVolumeAndBucketToDB(volume, bucket, metadataManager, + BucketLayout.OBJECT_STORE); + + // The current version and a decoy stored under the dbKey that version would + // occupy in the versionedKeyTable. Resolving to the current record proves + // the lookup answers from keyTable and never falls through to the scan. + metadataManager.getKeyTable(BucketLayout.OBJECT_STORE).put( + metadataManager.getOzoneKey(volume, bucket, key), + versionedKeyInfo(volume, bucket, key, 30L, false, false)); + metadataManager.getVersionedKeyTable().put( + metadataManager.getVersionedOzoneKey(volume, bucket, key, 30L), + versionedKeyInfo(volume, bucket, key, 30L, false, true)); + + OmKeyArgs args = new OmKeyArgs.Builder() + .setVolumeName(volume).setBucketName(bucket).setKeyName(key) + .setHeadOp(true).setVersionId(30L).build(); + + // the decoy is a delete marker, so reading it would have thrown + assertEquals(30L, keyManager.lookupKey(args, resolveBucket(args), null) + .getVersionId()); + } + + @Test + public void testNullVersionSlotCanBeTheCurrentVersion() throws Exception { + String volume = "vol-null"; + String bucket = "buck-null"; + String key = "obj"; + OMRequestTestUtils.addVolumeAndBucketToDB(volume, bucket, metadataManager, + BucketLayout.OBJECT_STORE); + + // A suspended PUT leaves the null version as the current one, with the + // versions accumulated while enabled behind it. + metadataManager.getKeyTable(BucketLayout.OBJECT_STORE).put( + metadataManager.getOzoneKey(volume, bucket, key), + versionedKeyInfo(volume, bucket, key, 50L, true, false)); + metadataManager.getVersionedKeyTable().put( + metadataManager.getVersionedOzoneKey(volume, bucket, key, 20L), + versionedKeyInfo(volume, bucket, key, 20L, false, false)); + + OmKeyArgs args = new OmKeyArgs.Builder() + .setVolumeName(volume).setBucketName(bucket).setKeyName(key) + .setHeadOp(true).setNullVersion(true).build(); + + OmKeyInfo nullVersion = keyManager.lookupKey(args, resolveBucket(args), null); + assertEquals(50L, nullVersion.getVersionId()); + assertTrue(nullVersion.isNullVersion()); + } + + @Test + public void testVersionIdRejectedOnFileSystemOptimizedBucket() + throws Exception { + String volume = "vol-fso"; + String bucket = "buck-fso"; + OMRequestTestUtils.addVolumeAndBucketToDB(volume, bucket, metadataManager, + BucketLayout.FILE_SYSTEM_OPTIMIZED); + + OmKeyArgs args = new OmKeyArgs.Builder() + .setVolumeName(volume).setBucketName(bucket).setKeyName("obj") + .setHeadOp(true).setVersionId(1L).build(); + ResolvedBucket fso = new ResolvedBucket(volume, bucket, volume, bucket, "", + BucketLayout.FILE_SYSTEM_OPTIMIZED); + + OMException ex = assertThrows(OMException.class, + () -> keyManager.lookupKey(args, fso, null)); + assertEquals(OMException.ResultCodes.NOT_SUPPORTED_OPERATION, + ex.getResult()); + } + private ResolvedBucket resolveBucket(OmKeyArgs keyArgs) { return new ResolvedBucket(keyArgs.getVolumeName(), keyArgs.getBucketName(), keyArgs.getVolumeName(), keyArgs.getBucketName(), "", From 0eec00f0c63ecdacdd113664006191896165cc71 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 31 Jul 2026 16:59:54 +0800 Subject: [PATCH 18/23] T4.2. Permanently delete a noncurrent version by versionId DELETE ?versionId= is the only delete that destroys data on a versioned bucket: the version leaves the versionedKeyTable and its blocks go to the deletedTable, which stays the single path through which version blocks are reclaimed. The null slot is addressed by attribute, so it is found by the same bounded prefix scan the read path uses. Addressing the current version is rejected for now: removing it has to promote the next-newest version to keep the keyTable authoritative, which T4.3 adds. Co-Authored-By: Claude Opus 5 --- .../om/request/key/OMKeyDeleteRequest.java | 104 ++++++++++++++++- .../key/OMKeyVersionDeleteResponse.java | 80 +++++++++++++ .../key/TestOMKeyVersioningRequests.java | 110 ++++++++++++++++++ 3 files changed, 290 insertions(+), 4 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index dc614fc02af6..cad1291ca594 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -19,6 +19,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.DELETED_HSYNC_KEY; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NOT_SUPPORTED_OPERATION; import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.LeveledResource.BUCKET_LOCK; import static org.apache.hadoop.ozone.util.MetricUtil.captureLatencyNs; @@ -56,6 +57,7 @@ import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteMarkerResponse; import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteResponse; +import org.apache.hadoop.ozone.om.response.key.OMKeyVersionDeleteResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeyResponse; @@ -142,6 +144,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut // whether the request took the delete marker path, and so may have left // versionedKeyTable entries in the table cache for this transaction boolean insertingDeleteMarker = false; + // whether the request took the permanent version delete path, and so may + // have left versionedKeyTable entries in the table cache + boolean deletingVersion = false; long startNanos = Time.monotonicNowNanos(); try { String objectKey = @@ -160,7 +165,21 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut OmBucketInfo omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName); - if (omBucketInfo.isS3VersioningEnabled()) { + if (keyArgs.hasVersionId() || keyArgs.getNullVersion()) { + // DELETE ?versionId= permanently removes one version. It is the only + // delete that destroys data on a versioned bucket. + if (!omBucketInfo.isS3VersioningEnabled()) { + throw new OMException("Bucket " + bucketName + + " does not have S3 versioning enabled", + NOT_SUPPORTED_OPERATION); + } + deletingVersion = true; + omClientResponse = deleteVersion(omMetadataManager, omBucketInfo, + omKeyInfo, keyArgs, volumeName, bucketName, keyName, trxnLogIndex, + omResponse); + // Noncurrent versions are invisible to plain reads, so removing one + // does not change the visible key count. + } else if (omBucketInfo.isS3VersioningEnabled()) { // A delete without a versionId removes no data: a delete marker // becomes the current version and the version it supersedes moves to // the versionedKeyTable. Like S3, the marker is inserted even when the @@ -237,9 +256,16 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut // touching the versionedKeyTable cache would otherwise leave an entry // behind that is in no DB and is never cleaned up. OMResponse errorResponse = createErrorOMResponse(omResponse, exception); - omClientResponse = insertingDeleteMarker - ? new OMKeyDeleteMarkerResponse(errorResponse, getBucketLayout()) - : new OMKeyDeleteResponse(errorResponse, getBucketLayout()); + if (insertingDeleteMarker) { + omClientResponse = + new OMKeyDeleteMarkerResponse(errorResponse, getBucketLayout()); + } else if (deletingVersion) { + omClientResponse = + new OMKeyVersionDeleteResponse(errorResponse, getBucketLayout()); + } else { + omClientResponse = + new OMKeyDeleteResponse(errorResponse, getBucketLayout()); + } long endNanosDeleteKeyFailureLatencyNs = Time.monotonicNowNanos(); perfMetrics.setDeleteKeyFailureLatencyNs(endNanosDeleteKeyFailureLatencyNs - startNanos); } finally { @@ -286,6 +312,76 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut * version. When the key does not exist the marker is still inserted, as S3 * does. */ + /** + * Permanently removes the addressed version. Only noncurrent versions are + * handled here: removing the current version has to promote the next-newest + * one to keep the keyTable authoritative, which T4.3 adds. + * + * @param currentVersion the key's current version, or null if it has none + */ + @SuppressWarnings("checkstyle:ParameterNumber") + private OMClientResponse deleteVersion(OMMetadataManager omMetadataManager, + OmBucketInfo omBucketInfo, OmKeyInfo currentVersion, + OzoneManagerProtocolProtos.KeyArgs keyArgs, String volumeName, + String bucketName, String keyName, long trxnLogIndex, + OMResponse.Builder omResponse) throws IOException { + + boolean nullVersion = keyArgs.getNullVersion(); + if (currentVersion != null && (nullVersion + ? currentVersion.isNullVersion() + : Long.valueOf(keyArgs.getVersionId()).equals( + currentVersion.getVersionId()))) { + throw new OMException( + "Permanently deleting the current version of key " + keyName + + " is not supported yet", NOT_SUPPORTED_OPERATION); + } + + // Everything that can fail runs before the first cache entry is added, so + // that a failed request leaves no versionedKeyTable cache entry behind. + String versionedKey; + OmKeyInfo version; + if (nullVersion) { + versionedKey = null; + version = null; + String prefix = omMetadataManager + .getVersionedOzoneKeyPrefix(volumeName, bucketName, keyName); + try (Table.KeyValueIterator versions = + omMetadataManager.getVersionedKeyTable().iterator(prefix)) { + while (versions.hasNext()) { + Table.KeyValue entry = versions.next(); + if (entry.getValue().isNullVersion()) { + versionedKey = entry.getKey(); + version = entry.getValue(); + break; + } + } + } + } else { + versionedKey = omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, keyArgs.getVersionId()); + version = omMetadataManager.getVersionedKeyTable().get(versionedKey); + } + if (version == null) { + throw new OMException("Version not found for key " + keyName, + KEY_NOT_FOUND); + } + + version = version.toBuilder().setUpdateID(trxnLogIndex).build(); + + omMetadataManager.getVersionedKeyTable().addCacheEntry( + new CacheKey<>(versionedKey), CacheValue.get(trxnLogIndex)); + + // A delete marker holds no blocks, so it releases namespace but no space. + long quotaReleased = sumBlockLengths(version); + boolean isVersionNonEmpty = !OmKeyInfo.isKeyEmpty(version); + omBucketInfo.decrUsedBytes(quotaReleased, isVersionNonEmpty); + omBucketInfo.decrUsedNamespace(1L, isVersionNonEmpty); + + return new OMKeyVersionDeleteResponse( + omResponse.setDeleteKeyResponse(DeleteKeyResponse.newBuilder()).build(), + version, versionedKey, omBucketInfo.copyObject()); + } + @SuppressWarnings("checkstyle:ParameterNumber") private OMClientResponse insertDeleteMarker(OzoneManager ozoneManager, OMMetadataManager omMetadataManager, OmBucketInfo omBucketInfo, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java new file mode 100644 index 000000000000..8da277bf9538 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java @@ -0,0 +1,80 @@ +/* + * 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.response.key; + +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DELETED_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VERSIONED_KEY_TABLE; + +import jakarta.annotation.Nonnull; +import java.io.IOException; +import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.response.CleanupTableInfo; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; + +/** + * Response for {@code DELETE ?versionId=} against a noncurrent version: the + * version is removed from the versionedKeyTable and its blocks go to the + * deletedTable, which is the single path through which version blocks are + * reclaimed. The key's current version is untouched. + */ +@CleanupTableInfo(cleanupTables = {VERSIONED_KEY_TABLE, DELETED_TABLE, BUCKET_TABLE}) +public class OMKeyVersionDeleteResponse extends AbstractOMKeyDeleteResponse { + + private final OmKeyInfo deletedVersion; + private final String versionedKeyName; + private final OmBucketInfo omBucketInfo; + + public OMKeyVersionDeleteResponse(@Nonnull OMResponse omResponse, + @Nonnull OmKeyInfo deletedVersion, @Nonnull String versionedKeyName, + @Nonnull OmBucketInfo omBucketInfo) { + super(omResponse, omBucketInfo.getBucketLayout()); + this.deletedVersion = deletedVersion; + this.versionedKeyName = versionedKeyName; + this.omBucketInfo = omBucketInfo; + } + + /** + * For when the request is not successful. + * For a successful request, the other constructor should be used. + */ + public OMKeyVersionDeleteResponse(@Nonnull OMResponse omResponse, + @Nonnull BucketLayout bucketLayout) { + super(omResponse, bucketLayout); + this.deletedVersion = null; + this.versionedKeyName = null; + this.omBucketInfo = null; + checkStatusNotOK(); + } + + @Override + public void addToDBBatch(OMMetadataManager omMetadataManager, + BatchOperation batchOperation) throws IOException { + addDeletionToBatch(omMetadataManager, batchOperation, + omMetadataManager.getVersionedKeyTable(), versionedKeyName, + deletedVersion, omBucketInfo.getObjectID(), true); + + omMetadataManager.getBucketTable().putWithBatch(batchOperation, + omMetadataManager.getBucketKey(omBucketInfo.getVolumeName(), + omBucketInfo.getBucketName()), omBucketInfo); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java index 097f6b1abeb4..8920cca949f7 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -124,6 +124,116 @@ private OMClientResponse deleteAt(long trxnLogIndex) throws Exception { return response; } + private OMRequest deleteVersionRequest(Long versionId, boolean nullVersion) { + KeyArgs.Builder keyArgs = KeyArgs.newBuilder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(keyName) + .setModificationTime(Time.now()); + if (versionId != null) { + keyArgs.setVersionId(versionId); + } + if (nullVersion) { + keyArgs.setNullVersion(true); + } + return OMRequest.newBuilder() + .setDeleteKeyRequest(DeleteKeyRequest.newBuilder().setKeyArgs(keyArgs)) + .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKey) + .setClientId(UUID.randomUUID().toString()).build(); + } + + private OMClientResponse deleteVersionAt(Long versionId, boolean nullVersion, + long trxnLogIndex) throws Exception { + return new OMKeyDeleteRequest(deleteVersionRequest(versionId, nullVersion), + getBucketLayout()).validateAndUpdateCache(ozoneManager, trxnLogIndex); + } + + private void seedNoncurrentVersion(long versionId, boolean nullVersion) + throws Exception { + OmKeyInfo version = OMRequestTestUtils.createOmKeyInfo( + volumeName, bucketName, keyName, replicationConfig) + .setVersionId(versionId) + .setNullVersion(nullVersion) + .build(); + omMetadataManager.getVersionedKeyTable().put( + omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, versionId), version); + } + + @Test + public void testPermanentDeleteRemovesOnlyTheAddressedVersion() + throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(100L, false); + seedNoncurrentVersion(200L, false); + + OMClientResponse response = deleteVersionAt(100L, false, 400L); + assertEquals(OzoneManagerProtocolProtos.Status.OK, + response.getOMResponse().getStatus()); + + assertNull(noncurrentVersion(100L)); + assertNotNull(noncurrentVersion(200L)); + assertEquals(300L, currentVersion().getVersionId()); + } + + @Test + public void testPermanentDeleteReleasesQuota() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(100L, false); + + OmBucketInfo before = omMetadataManager.getBucketTable() + .get(omMetadataManager.getBucketKey(volumeName, bucketName)); + long usedNamespaceBefore = before.getUsedNamespace(); + + deleteVersionAt(100L, false, 400L); + + OmBucketInfo after = omMetadataManager.getBucketTable() + .get(omMetadataManager.getBucketKey(volumeName, bucketName)); + assertEquals(usedNamespaceBefore - 1, after.getUsedNamespace()); + } + + @Test + public void testPermanentDeleteOfNullVersion() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(100L, true); + seedNoncurrentVersion(200L, false); + + OMClientResponse response = deleteVersionAt(null, true, 400L); + assertEquals(OzoneManagerProtocolProtos.Status.OK, + response.getOMResponse().getStatus()); + + assertNull(noncurrentVersion(100L)); + assertNotNull(noncurrentVersion(200L)); + } + + @Test + public void testPermanentDeleteOfUnknownVersionIsNotFound() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + + OMClientResponse response = deleteVersionAt(999L, false, 400L); + assertEquals(OzoneManagerProtocolProtos.Status.KEY_NOT_FOUND, + response.getOMResponse().getStatus()); + } + + /** Removing the current version has to promote a successor; that is T4.3. */ + @Test + public void testPermanentDeleteOfCurrentVersionIsRejectedForNow() + throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(100L, false); + + OMClientResponse response = deleteVersionAt(300L, false, 400L); + assertEquals(OzoneManagerProtocolProtos.Status.NOT_SUPPORTED_OPERATION, + response.getOMResponse().getStatus()); + assertEquals(300L, currentVersion().getVersionId()); + assertNotNull(noncurrentVersion(100L)); + } + private OmKeyInfo currentVersion() throws Exception { return omMetadataManager.getKeyTable(getBucketLayout()).get( omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)); From ba36f7583fb78477d9122ffdc22df4d7a6dd7d83 Mon Sep 17 00:00:00 2001 From: Symious Date: Fri, 31 Jul 2026 17:07:54 +0800 Subject: [PATCH 19/23] T4.3. Promote the next-newest version when the current one is deleted keyTable holds the current version of every key that still has one, so removing the current version has to hand the place over: one seek on the key's version prefix yields the newest remaining version, which moves back into the keyTable in the same WriteBatch as the delete. The record travels unchanged - promotion is positional, and a version keeps the identity it was created with. When no version survives, the key disappears entirely. Deleting a current delete marker this way is exactly S3's restore-an-object flow: the version the marker superseded becomes current again. Co-Authored-By: Claude Opus 5 --- .../om/request/key/OMKeyDeleteRequest.java | 68 ++++++++---- .../ozone/om/request/key/OMKeyRequest.java | 103 ++++++++++++++++++ .../key/OMKeyVersionDeleteResponse.java | 46 ++++++-- .../key/TestOMKeyVersioningRequests.java | 95 +++++++++++++++- 4 files changed, 273 insertions(+), 39 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index cad1291ca594..cdf0fa6e03ca 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -30,6 +30,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.hdds.utils.db.Table; @@ -327,35 +328,23 @@ private OMClientResponse deleteVersion(OMMetadataManager omMetadataManager, OMResponse.Builder omResponse) throws IOException { boolean nullVersion = keyArgs.getNullVersion(); - if (currentVersion != null && (nullVersion + boolean deletingCurrent = currentVersion != null && (nullVersion ? currentVersion.isNullVersion() : Long.valueOf(keyArgs.getVersionId()).equals( - currentVersion.getVersionId()))) { - throw new OMException( - "Permanently deleting the current version of key " + keyName - + " is not supported yet", NOT_SUPPORTED_OPERATION); - } + currentVersion.getVersionId())); // Everything that can fail runs before the first cache entry is added, so // that a failed request leaves no versionedKeyTable cache entry behind. String versionedKey; OmKeyInfo version; - if (nullVersion) { + if (deletingCurrent) { versionedKey = null; - version = null; - String prefix = omMetadataManager - .getVersionedOzoneKeyPrefix(volumeName, bucketName, keyName); - try (Table.KeyValueIterator versions = - omMetadataManager.getVersionedKeyTable().iterator(prefix)) { - while (versions.hasNext()) { - Table.KeyValue entry = versions.next(); - if (entry.getValue().isNullVersion()) { - versionedKey = entry.getKey(); - version = entry.getValue(); - break; - } - } - } + version = currentVersion; + } else if (nullVersion) { + Pair nullSlot = getNoncurrentNullVersion( + omMetadataManager, volumeName, bucketName, keyName); + versionedKey = nullSlot == null ? null : nullSlot.getKey(); + version = nullSlot == null ? null : nullSlot.getValue(); } else { versionedKey = omMetadataManager.getVersionedOzoneKey( volumeName, bucketName, keyName, keyArgs.getVersionId()); @@ -366,10 +355,40 @@ private OMClientResponse deleteVersion(OMMetadataManager omMetadataManager, KEY_NOT_FOUND); } + // Removing the current version leaves the key without one, so the newest + // noncurrent version is promoted to keep the invariant that keyTable holds + // the current version of every key that still has one. The record moves + // unchanged: promotion is positional, the version keeps its identity. + String promotedKey = null; + OmKeyInfo promoted = null; + if (deletingCurrent) { + Pair newest = getNewestNoncurrentVersion( + omMetadataManager, volumeName, bucketName, keyName); + if (newest != null) { + promotedKey = newest.getKey(); + promoted = newest.getValue(); + } + } + version = version.toBuilder().setUpdateID(trxnLogIndex).build(); - omMetadataManager.getVersionedKeyTable().addCacheEntry( - new CacheKey<>(versionedKey), CacheValue.get(trxnLogIndex)); + String objectKey = + omMetadataManager.getOzoneKey(volumeName, bucketName, keyName); + if (deletingCurrent) { + if (promoted != null) { + omMetadataManager.getKeyTable(getBucketLayout()) + .addCacheEntry(objectKey, promoted, trxnLogIndex); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + new CacheKey<>(promotedKey), CacheValue.get(trxnLogIndex)); + } else { + // no version survives, so the key disappears entirely + omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry( + new CacheKey<>(objectKey), CacheValue.get(trxnLogIndex)); + } + } else { + omMetadataManager.getVersionedKeyTable().addCacheEntry( + new CacheKey<>(versionedKey), CacheValue.get(trxnLogIndex)); + } // A delete marker holds no blocks, so it releases namespace but no space. long quotaReleased = sumBlockLengths(version); @@ -379,7 +398,8 @@ private OMClientResponse deleteVersion(OMMetadataManager omMetadataManager, return new OMKeyVersionDeleteResponse( omResponse.setDeleteKeyResponse(DeleteKeyResponse.newBuilder()).build(), - version, versionedKey, omBucketInfo.copyObject()); + version, deletingCurrent ? objectKey : versionedKey, deletingCurrent, + promotedKey, promoted, omBucketInfo.copyObject()); } @SuppressWarnings("checkstyle:ParameterNumber") diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index b0fef6cad794..6839ae2fbf90 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -49,6 +49,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.EncryptedKeyVersion; @@ -63,6 +64,7 @@ import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.security.token.OzoneBlockTokenIdentifier; +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; import org.apache.hadoop.ipc_.Server; @@ -896,6 +898,107 @@ public static long sumBlockLengths(OmKeyInfo omKeyInfo) { return bytesUsed; } + /** + * Returns the newest noncurrent version of the given key as a + * (dbKey, keyInfo) pair, or null if the key has no noncurrent version. + */ + protected Pair getNewestNoncurrentVersion( + OMMetadataManager omMetadataManager, String volumeName, + String bucketName, String keyName) throws IOException { + return findNoncurrentVersion(omMetadataManager, volumeName, bucketName, + keyName, keyInfo -> true); + } + + /** + * Returns the key's null version as a (dbKey, keyInfo) pair, or null if the + * key has no noncurrent null version. A key has at most one. + */ + protected Pair getNoncurrentNullVersion( + OMMetadataManager omMetadataManager, String volumeName, + String bucketName, String keyName) throws IOException { + return findNoncurrentVersion(omMetadataManager, volumeName, bucketName, + keyName, OmKeyInfo::isNullVersion); + } + + /** + * Returns the newest noncurrent version of the key that satisfies + * {@code filter}, or null if there is none. Versions of a key are adjacent + * and ordered newest first, so the smallest matching dbKey wins. + * + *

    Both the table cache and the DB are searched, and neither can stand in + * for the other: + * + *

      + *
    • {@link Table#iterator} reads RocksDB directly and does not consult + * the cache, so a version written by a transaction that the double + * buffer has not flushed yet is invisible to it, and a version removed + * by such a transaction still appears in it;
      • + *
    • the cache only holds the current flush window, so every older + * version of the key exists in the DB only.
      • + *
    + * + *

    The search does not stop at the first cache match either. Versions are + * demoted into the versionedKeyTable in increasing versionId order, so in + * practice a cached version is newer than every flushed one, but that is a + * property of the write paths rather than something enforced here, and + * relying on it would silently promote the wrong version if a later write + * path broke it. The DB search costs one seek, since it stops at the first + * match. + */ + private Pair findNoncurrentVersion( + OMMetadataManager omMetadataManager, String volumeName, + String bucketName, String keyName, Predicate filter) + throws IOException { + final String prefix = omMetadataManager.getVersionedOzoneKeyPrefix( + volumeName, bucketName, keyName); + final Table table = + omMetadataManager.getVersionedKeyTable(); + + String bestKey = null; + OmKeyInfo bestValue = null; + + // Entries of transactions that are not flushed yet. The cache is not + // sorted, so it has to be scanned; it only holds this table's writes from + // the current flush window. + Iterator, CacheValue>> cacheIterator = + table.cacheIterator(); + while (cacheIterator.hasNext()) { + Map.Entry, CacheValue> entry = cacheIterator.next(); + String dbKey = entry.getKey().getCacheKey(); + OmKeyInfo value = entry.getValue().getCacheValue(); + // a null value is a tombstone: the version is deleted but not flushed + if (value == null || !dbKey.startsWith(prefix) || !filter.test(value)) { + continue; + } + if (bestKey == null || dbKey.compareTo(bestKey) < 0) { + bestKey = dbKey; + bestValue = value; + } + } + + try (Table.KeyValueIterator versions = table.iterator(prefix)) { + while (versions.hasNext()) { + Table.KeyValue entry = versions.next(); + String dbKey = entry.getKey(); + CacheValue cached = table.getCacheValue(new CacheKey<>(dbKey)); + if (cached != null && cached.getCacheValue() == null) { + continue; + } + OmKeyInfo value = cached != null ? cached.getCacheValue() : entry.getValue(); + if (!filter.test(value)) { + continue; + } + if (bestKey == null || dbKey.compareTo(bestKey) < 0) { + bestKey = dbKey; + bestValue = value; + } + // DB entries ascend, so the first match is the smallest one in the DB + break; + } + } + return bestKey == null ? null : Pair.of(bestKey, bestValue); + } + /** * Return bucket info for the specified bucket. */ diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java index 8da277bf9538..c76ef0aceaf4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyVersionDeleteResponse.java @@ -19,6 +19,7 @@ import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DELETED_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VERSIONED_KEY_TABLE; import jakarta.annotation.Nonnull; @@ -32,24 +33,33 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; /** - * Response for {@code DELETE ?versionId=} against a noncurrent version: the - * version is removed from the versionedKeyTable and its blocks go to the - * deletedTable, which is the single path through which version blocks are - * reclaimed. The key's current version is untouched. + * Response for {@code DELETE ?versionId=}: the version leaves the table that + * held it and its blocks go to the deletedTable, which is the single path + * through which version blocks are reclaimed. Removing the current version + * promotes the newest remaining one into the keyTable in the same batch, so the + * key never lacks a current version; if none remains the key disappears. */ -@CleanupTableInfo(cleanupTables = {VERSIONED_KEY_TABLE, DELETED_TABLE, BUCKET_TABLE}) +@CleanupTableInfo(cleanupTables = {KEY_TABLE, VERSIONED_KEY_TABLE, DELETED_TABLE, BUCKET_TABLE}) public class OMKeyVersionDeleteResponse extends AbstractOMKeyDeleteResponse { private final OmKeyInfo deletedVersion; - private final String versionedKeyName; + private final String deletedKeyName; + private final boolean deletedCurrent; + private final String promotedKeyName; + private final OmKeyInfo promoted; private final OmBucketInfo omBucketInfo; + @SuppressWarnings("checkstyle:ParameterNumber") public OMKeyVersionDeleteResponse(@Nonnull OMResponse omResponse, - @Nonnull OmKeyInfo deletedVersion, @Nonnull String versionedKeyName, + @Nonnull OmKeyInfo deletedVersion, @Nonnull String deletedKeyName, + boolean deletedCurrent, String promotedKeyName, OmKeyInfo promoted, @Nonnull OmBucketInfo omBucketInfo) { super(omResponse, omBucketInfo.getBucketLayout()); this.deletedVersion = deletedVersion; - this.versionedKeyName = versionedKeyName; + this.deletedKeyName = deletedKeyName; + this.deletedCurrent = deletedCurrent; + this.promotedKeyName = promotedKeyName; + this.promoted = promoted; this.omBucketInfo = omBucketInfo; } @@ -61,7 +71,10 @@ public OMKeyVersionDeleteResponse(@Nonnull OMResponse omResponse, @Nonnull BucketLayout bucketLayout) { super(omResponse, bucketLayout); this.deletedVersion = null; - this.versionedKeyName = null; + this.deletedKeyName = null; + this.deletedCurrent = false; + this.promotedKeyName = null; + this.promoted = null; this.omBucketInfo = null; checkStatusNotOK(); } @@ -69,9 +82,20 @@ public OMKeyVersionDeleteResponse(@Nonnull OMResponse omResponse, @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { + // The version leaves whichever table held it and its blocks go to the + // deletedTable; when it was the current version the successor takes its + // place in the keyTable, so the delete and the promotion land in one batch. addDeletionToBatch(omMetadataManager, batchOperation, - omMetadataManager.getVersionedKeyTable(), versionedKeyName, - deletedVersion, omBucketInfo.getObjectID(), true); + deletedCurrent ? omMetadataManager.getKeyTable(getBucketLayout()) + : omMetadataManager.getVersionedKeyTable(), + deletedKeyName, deletedVersion, omBucketInfo.getObjectID(), true); + + if (promoted != null) { + omMetadataManager.getKeyTable(getBucketLayout()) + .putWithBatch(batchOperation, deletedKeyName, promoted); + omMetadataManager.getVersionedKeyTable() + .deleteWithBatch(batchOperation, promotedKeyName); + } omMetadataManager.getBucketTable().putWithBatch(batchOperation, omMetadataManager.getBucketKey(omBucketInfo.getVolumeName(), diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java index 8920cca949f7..b152a4424c36 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -160,6 +160,17 @@ private void seedNoncurrentVersion(long versionId, boolean nullVersion) volumeName, bucketName, keyName, versionId), version); } + /** A noncurrent version that only exists in the table cache, not in the DB. */ + private void cacheOnlyNoncurrentVersion(long versionId) throws Exception { + OmKeyInfo version = OMRequestTestUtils.createOmKeyInfo( + volumeName, bucketName, keyName, replicationConfig) + .setVersionId(versionId) + .build(); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, versionId), version, 350L); + } + @Test public void testPermanentDeleteRemovesOnlyTheAddressedVersion() throws Exception { @@ -219,21 +230,97 @@ public void testPermanentDeleteOfUnknownVersionIsNotFound() throws Exception { response.getOMResponse().getStatus()); } - /** Removing the current version has to promote a successor; that is T4.3. */ @Test - public void testPermanentDeleteOfCurrentVersionIsRejectedForNow() + public void testDeletingCurrentVersionPromotesTheNextNewest() throws Exception { setupVersionedBucket(); seedCurrentVersion(300L); seedNoncurrentVersion(100L, false); + seedNoncurrentVersion(200L, false); OMClientResponse response = deleteVersionAt(300L, false, 400L); - assertEquals(OzoneManagerProtocolProtos.Status.NOT_SUPPORTED_OPERATION, + assertEquals(OzoneManagerProtocolProtos.Status.OK, response.getOMResponse().getStatus()); - assertEquals(300L, currentVersion().getVersionId()); + + // the newest remaining version takes over, unchanged + OmKeyInfo current = currentVersion(); + assertNotNull(current); + assertEquals(200L, current.getVersionId()); + assertFalse(current.isDeleteMarker()); + // and no longer counts as noncurrent + assertNull(noncurrentVersion(200L)); assertNotNull(noncurrentVersion(100L)); } + /** + * A version written by a transaction that the double buffer has not flushed + * yet lives only in the table cache. Promotion has to see it, otherwise an + * older version takes over and the newest one is orphaned. + */ + @Test + public void testPromotionSeesVersionsStillInCache() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(100L, false); + cacheOnlyNoncurrentVersion(200L); + + deleteVersionAt(300L, false, 400L); + + assertEquals(200L, currentVersion().getVersionId()); + assertNotNull(noncurrentVersion(100L)); + } + + /** + * The mirror case: a version removed by an unflushed transaction is a + * tombstone in the cache while the DB still holds it. Promotion must not + * bring it back. + */ + @Test + public void testPromotionSkipsVersionsTombstonedInCache() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(100L, false); + seedNoncurrentVersion(200L, false); + // 200 is deleted but not flushed yet + omMetadataManager.getVersionedKeyTable().addCacheEntry( + new CacheKey<>(omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, 200L)), + CacheValue.get(350L)); + + deleteVersionAt(300L, false, 400L); + + assertEquals(100L, currentVersion().getVersionId()); + } + + @Test + public void testDeletingTheOnlyVersionRemovesTheKey() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L); + + OMClientResponse response = deleteVersionAt(300L, false, 400L); + assertEquals(OzoneManagerProtocolProtos.Status.OK, + response.getOMResponse().getStatus()); + + assertNull(currentVersion()); + } + + /** Deleting a current delete marker is how S3 restores an object. */ + @Test + public void testDeletingCurrentMarkerRestoresTheObject() throws Exception { + setupVersionedBucket(); + seedCurrentVersion(300L, true); + seedNoncurrentVersion(100L, false); + + OMClientResponse response = deleteVersionAt(300L, false, 400L); + assertEquals(OzoneManagerProtocolProtos.Status.OK, + response.getOMResponse().getStatus()); + + OmKeyInfo current = currentVersion(); + assertNotNull(current); + assertEquals(100L, current.getVersionId()); + assertFalse(current.isDeleteMarker()); + } + private OmKeyInfo currentVersion() throws Exception { return omMetadataManager.getKeyTable(getBucketLayout()).get( omMetadataManager.getOzoneKey(volumeName, bucketName, keyName)); From 71f7856f0367e2095eabdd8f348a6b2eecd92bcf Mon Sep 17 00:00:00 2001 From: Symious Date: Tue, 4 Aug 2026 16:12:37 +0800 Subject: [PATCH 20/23] T5.1. Take the null version slot on a write while versioning is suspended Co-Authored-By: Claude Opus 5 --- .../hadoop/ozone/om/helpers/OmBucketInfo.java | 36 ++++-- .../hadoop/ozone/om/helpers/OmKeyInfo.java | 13 ++ .../request/bucket/OMBucketCreateRequest.java | 9 ++ .../bucket/OMBucketSetPropertyRequest.java | 7 ++ .../om/request/key/OMKeyCommitRequest.java | 54 ++++++-- .../ozone/om/request/key/OMKeyRequest.java | 2 +- .../om/response/key/OMKeyCommitResponse.java | 16 +++ .../TestOMBucketSetPropertyRequest.java | 30 ++++- .../key/TestOMKeyVersioningRequests.java | 118 +++++++++++++++++- 9 files changed, 266 insertions(+), 19 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java index b48d2d04d664..272ce9e84aa8 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java @@ -206,16 +206,36 @@ public boolean hasVersioningStatus() { } /** - * Whether S3-compatible versioning is in effect, i.e. whether a write keeps - * the previous current version as a separate record in the versionedKeyTable. - * True for status ENABLED on an OBJECT_STORE bucket; buckets of other layouts - * carrying the legacy isVersionEnabled flag keep the legacy in-record block - * version behaviour. - * @return whether writes create versionedKeyTable records + * Whether S3 versioning is enabled on this bucket: a write creates a new + * version of the key. A bucket carrying only the legacy isVersionEnabled + * flag is not versioned in this sense - that flag selects the in-record + * block version list, which is a different feature. + * @return whether writes create a new object version */ public boolean isS3VersioningEnabled() { - return versioningStatus == BucketVersioningStatus.ENABLED - && bucketLayout == BucketLayout.OBJECT_STORE; + return getVersioningStatus() == BucketVersioningStatus.ENABLED; + } + + /** + * Whether S3 versioning is suspended on this bucket: a write takes the key's + * null version slot instead of creating a version of its own. + * @return whether writes take the null version slot + */ + public boolean isS3VersioningSuspended() { + return getVersioningStatus() == BucketVersioningStatus.SUSPENDED; + } + + /** + * Whether versioning has ever been enabled on this bucket. This is what + * makes the versions of its keys retained rather than reclaimed, and it is + * the state S3's data-protection promise is phrased against: once a bucket + * has been versioned, a write or a delete without a versionId never destroys + * data. A bucket can never return to UNVERSIONED, so this holds for ENABLED + * and SUSPENDED alike. + * @return whether this bucket's object versions are retained + */ + public boolean hasEverBeenVersioned() { + return isS3VersioningEnabled() || isS3VersioningSuspended(); } /** diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java index 65540af1284a..395f00f4fd75 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java @@ -497,6 +497,19 @@ public boolean isNullVersion() { return isNullVersion; } + /** + * Whether this record is the key's null version, i.e. what a request naming + * version "null" addresses. That is either a record written while versioning + * was suspended, which carries the flag, or a record that predates versioning + * and so carries no versionId at all: enabling versioning does not rewrite + * existing objects, and S3 reports their version as "null". + * + * @return whether version "null" of the key addresses this record + */ + public boolean isNullVersionRecord() { + return isNullVersion || versionId == null; + } + @Override public String toString() { return "OmKeyInfo{" + diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java index 718f329aaaff..51bae769b8c0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java @@ -101,6 +101,15 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { // FSO and LEGACY buckets are not strictly bound to S3 naming semantics. OmUtils.validateBucketName(bucketInfo.getBucketName(), strict); + if (bucketInfo.hasVersioningStatus() + && bucketLayout != BucketLayout.OBJECT_STORE) { + throw new OMException("S3 object versioning is only supported on " + + BucketLayout.OBJECT_STORE + " buckets, but bucket " + + bucketInfo.getBucketName() + " is created with layout " + + bucketLayout + ".", + OMException.ResultCodes.NOT_SUPPORTED_OPERATION); + } + // ACL check during preExecute if (ozoneManager.getAclsEnabled()) { try { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index f897c5423ca7..07fa9b95b7b8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -38,6 +38,7 @@ import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext; import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.BucketVersioningStatus; import org.apache.hadoop.ozone.om.helpers.KeyValueUtil; import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; @@ -195,6 +196,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut ? BucketVersioningStatus.ENABLED : BucketVersioningStatus.SUSPENDED; } if (newVersioningStatus != null) { + if (dbBucketInfo.getBucketLayout() != BucketLayout.OBJECT_STORE) { + throw new OMException("S3 object versioning is only supported on " + + BucketLayout.OBJECT_STORE + " buckets, but bucket " + bucketName + + " has layout " + dbBucketInfo.getBucketLayout() + ".", + OMException.ResultCodes.NOT_SUPPORTED_OPERATION); + } if (!dbBucketInfo.getVersioningStatus().canTransitionTo(newVersioningStatus)) { throw new OMException("Bucket versioning cannot be changed from " + dbBucketInfo.getVersioningStatus() + " to " + newVersioningStatus diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index 5f135b5e92cb..7d6f7a6abed4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -34,6 +34,8 @@ import java.util.Map; import java.util.Objects; import org.apache.commons.lang3.tuple.Pair; +import org.apache.hadoop.hdds.utils.db.cache.CacheKey; +import org.apache.hadoop.hdds.utils.db.cache.CacheValue; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.OzoneManagerVersion; @@ -304,7 +306,13 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut } validateAtomicRewrite(keyToDelete, omKeyInfo, auditMap); - final boolean s3Versioning = omBucketInfo.isS3VersioningEnabled(); + final boolean versioningEnabled = omBucketInfo.isS3VersioningEnabled(); + final boolean keepsVersions = omBucketInfo.hasEverBeenVersioned(); + // A write while versioning is suspended creates no version of its own: it + // takes the key's null version slot, replacing whatever held it. The + // record still carries a generated versionId, so that it sorts among the + // key's versions by the time it was written. + final boolean suspendedWrite = omBucketInfo.isS3VersioningSuspended(); // Set the UpdateID to current transactionLogIndex OmKeyInfo.Builder committedKeyBuilder = omKeyInfo.toBuilder() .setExpectedDataGeneration(null) @@ -312,16 +320,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut commitKeyArgs.getMetadataList())) .setUpdateID(trxnLogIndex) .setDataSize(commitKeyArgs.getDataSize()); - if (s3Versioning) { + if (keepsVersions) { // The version identity is frozen when the version is created: an hsync // re-commit keeps updating the same version, so it keeps its versionId. committedKeyBuilder.setVersionId(isSameHsyncKey ? keyToDelete.getVersionId() : ozoneManager.getVersionIdAllocator().allocate(omMetadataManager, volumeName, bucketName, keyName, trxnLogIndex, keyToDelete)); + committedKeyBuilder.setNullVersion(suspendedWrite); } omKeyInfo = committedKeyBuilder.build(); + // The version a write supersedes is kept as a noncurrent version, except + // for the null version, which a suspended write replaces outright. + final boolean supersededVersionRetained = keyToDelete != null + && keepsVersions + && (versioningEnabled || !keyToDelete.isNullVersionRecord()); + // Update the block length for each block, return the allocated but // uncommitted blocks List uncommitted = @@ -336,9 +351,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut checkBucketQuotaInBytes(omMetadataManager, omBucketInfo, correctedSpace); } else if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled() - && !s3Versioning) { - // Under S3 versioning the overwritten version is kept in the - // versionedKeyTable, so its blocks must not be reclaimed here. + && !supersededVersionRetained) { + // A retained version keeps its blocks: it lives on in the + // versionedKeyTable. What reaches this branch under S3 versioning is + // the null version being replaced by a suspended write. RepeatedOmKeyInfo oldVerKeyInfo = getOldVersionsToCleanUp( keyToDelete, omBucketInfo.getObjectID(), trxnLogIndex); // using pseudoObjId as objectId can be same in case of overwrite key @@ -420,7 +436,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut // key's null version. String dbVersionedKey = null; OmKeyInfo versionedKeyInfo = null; - if (s3Versioning && keyToDelete != null && !isSameHsyncKey) { + if (supersededVersionRetained && !isSameHsyncKey) { versionedKeyInfo = keyToDelete.getVersionId() != null ? keyToDelete : keyToDelete.toBuilder() .setVersionId(VersionIdGenerator.UNSET_VERSION_ID) @@ -432,6 +448,29 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut dbVersionedKey, versionedKeyInfo, trxnLogIndex); } + // A suspended write replaces the key's null version wherever it is. It is + // the current version when the previous write was also suspended, and a + // noncurrent version when versioning was enabled in between - the version + // that demoted it is still current, and is retained above. + String replacedNullVersionKey = null; + if (suspendedWrite && !isSameHsyncKey && supersededVersionRetained) { + Pair nullVersion = getNoncurrentNullVersion( + omMetadataManager, volumeName, bucketName, keyName); + if (nullVersion != null) { + replacedNullVersionKey = nullVersion.getKey(); + oldKeyVersionsToDeleteMap = addKeyInfoToDeleteMap(ozoneManager, + trxnLogIndex, dbOzoneKey, omBucketInfo.getObjectID(), + nullVersion.getValue().withCommittedKeyDeletedFlag(true), + oldKeyVersionsToDeleteMap); + omBucketInfo.decrUsedBytes( + sumBlockLengths(nullVersion.getValue()), true); + omBucketInfo.decrUsedNamespace(1L, true); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + new CacheKey<>(replacedNullVersionKey), + CacheValue.get(trxnLogIndex)); + } + } + omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry( dbOzoneKey, omKeyInfo, trxnLogIndex); @@ -440,7 +479,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut omClientResponse = new OMKeyCommitResponse(omResponse.build(), omKeyInfo, dbOzoneKey, dbOpenKey, omBucketInfo.copyObject(), oldKeyVersionsToDeleteMap, isHSync, newOpenKeyInfo, dbOpenKeyToDeleteKey, openKeyToDelete) - .withVersionedKey(dbVersionedKey, versionedKeyInfo); + .withVersionedKey(dbVersionedKey, versionedKeyInfo) + .withReplacedNullVersion(replacedNullVersionKey); result = Result.SUCCESS; } catch (IOException | InvalidPathException ex) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index 6839ae2fbf90..2a1cab726889 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -1067,7 +1067,7 @@ protected OmKeyInfo prepareFileInfo( // the in-record block version list is not used to accumulate object // versions and always holds a single version. boolean keepInRecordVersions = omBucketInfo.getIsVersionEnabled() - && !omBucketInfo.isS3VersioningEnabled(); + && !omBucketInfo.hasEverBeenVersioned(); dbKeyInfo.addNewVersion(locations, false, keepInRecordVersions); long newSize = size; if (keepInRecordVersions) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java index f36aff6dfe46..ed17907b4fa0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyCommitResponse.java @@ -54,6 +54,7 @@ public class OMKeyCommitResponse extends OmKeyResponse { private String openKeyNameToUpdate; private String versionedKeyName; private OmKeyInfo versionedKeyInfo; + private String replacedNullVersionKey; @SuppressWarnings("checkstyle:ParameterNumber") public OMKeyCommitResponse( @@ -96,6 +97,16 @@ public OMKeyCommitResponse withVersionedKey(String dbVersionedKey, return this; } + /** + * The noncurrent null version this commit replaced, to be removed from the + * versionedKeyTable. Null unless a suspended write replaced a null version + * that was not the current one. + */ + public OMKeyCommitResponse withReplacedNullVersion(String dbVersionedKey) { + this.replacedNullVersionKey = dbVersionedKey; + return this; + } + @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { @@ -117,6 +128,11 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, .putWithBatch(batchOperation, versionedKeyName, versionedKeyInfo); } + if (replacedNullVersionKey != null) { + omMetadataManager.getVersionedKeyTable() + .deleteWithBatch(batchOperation, replacedNullVersionKey); + } + updateDeletedTable(omMetadataManager, batchOperation); handleOpenKeyToUpdate(omMetadataManager, batchOperation); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java index 0d2815cbc0af..cdfe5453a56d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java @@ -431,12 +431,38 @@ public void testValidateAndUpdateCacheWithQuotaNamespaceUsed() "is less than used namespaceQuota"); } + /** + * S3 object versioning is only defined for OBJECT_STORE buckets: combining + * it with the directory and rename semantics of the other layouts is out of + * scope, so the status cannot be set on them at all. + */ + @Test + public void testVersioningStatusRejectedOnNonObjectStoreBucket() + throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager, BucketLayout.FILE_SYSTEM_OPTIMIZED); + + OMClientResponse response = new OMBucketSetPropertyRequest( + createSetVersioningStatusRequest(volumeName, bucketName, + BucketVersioningStatus.ENABLED)).validateAndUpdateCache( + ozoneManager, 1); + + assertFalse(response.getOMResponse().getSuccess()); + assertEquals(OzoneManagerProtocolProtos.Status.NOT_SUPPORTED_OPERATION, + response.getOMResponse().getStatus()); + assertFalse(omMetadataManager.getBucketTable() + .get(omMetadataManager.getBucketKey(volumeName, bucketName)) + .hasVersioningStatus()); + } + @Test public void testVersioningStatusTransitions() throws Exception { String volumeName = UUID.randomUUID().toString(); String bucketName = UUID.randomUUID().toString(); OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, - omMetadataManager); + omMetadataManager, BucketLayout.OBJECT_STORE); String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName); assertEquals(BucketVersioningStatus.UNVERSIONED, @@ -486,7 +512,7 @@ public void testLegacyVersioningFlagMapsToStateMachine() throws Exception { String volumeName = UUID.randomUUID().toString(); String bucketName = UUID.randomUUID().toString(); OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, - omMetadataManager); + omMetadataManager, BucketLayout.OBJECT_STORE); String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName); // legacy false on a never-enabled bucket stays UNVERSIONED diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java index b152a4424c36..d1438685263d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -36,6 +36,7 @@ import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.om.response.key.OMKeyCommitResponse; import org.apache.hadoop.ozone.om.response.key.OMKeyDeleteMarkerResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CommitKeyRequest; @@ -67,14 +68,25 @@ private void setupVersionedBucket(long quotaInBytes, long quotaInNamespace) setupVersionedBucket(quotaInBytes, quotaInNamespace, 0L); } + private void setupSuspendedBucket() throws Exception { + setupVersionedBucket(OzoneConsts.QUOTA_RESET, OzoneConsts.QUOTA_RESET, 0L, + BucketVersioningStatus.SUSPENDED); + } + private void setupVersionedBucket(long quotaInBytes, long quotaInNamespace, long usedNamespace) throws Exception { + setupVersionedBucket(quotaInBytes, quotaInNamespace, usedNamespace, + BucketVersioningStatus.ENABLED); + } + + private void setupVersionedBucket(long quotaInBytes, long quotaInNamespace, + long usedNamespace, BucketVersioningStatus status) throws Exception { OMRequestTestUtils.addVolumeToDB(volumeName, omMetadataManager); OmBucketInfo bucketInfo = OmBucketInfo.newBuilder() .setVolumeName(volumeName) .setBucketName(bucketName) .setBucketLayout(BucketLayout.OBJECT_STORE) - .setVersioningStatus(BucketVersioningStatus.ENABLED) + .setVersioningStatus(status) .setQuotaInBytes(quotaInBytes) .setQuotaInNamespace(quotaInNamespace) .setUsedNamespace(usedNamespace) @@ -92,11 +104,25 @@ private String seedCurrentVersion(Long versionId) throws Exception { private String seedCurrentVersion(Long versionId, boolean deleteMarker) throws Exception { + return seedCurrentVersion(versionId, deleteMarker, false); + } + + private String seedCurrentVersion(Long versionId, boolean deleteMarker, + boolean nullVersion) throws Exception { + return seedCurrentVersion(versionId, deleteMarker, nullVersion, false); + } + + private String seedCurrentVersion(Long versionId, boolean deleteMarker, + boolean nullVersion, boolean withBlocks) throws Exception { OmKeyInfo keyInfo = OMRequestTestUtils.createOmKeyInfo( volumeName, bucketName, keyName, replicationConfig) .setVersionId(versionId) .setDeleteMarker(deleteMarker) + .setNullVersion(nullVersion) .build(); + if (withBlocks) { + OMRequestTestUtils.addKeyLocationInfo(keyInfo, 0L, 1000L); + } String ozoneKey = omMetadataManager.getOzoneKey( volumeName, bucketName, keyName); omMetadataManager.getKeyTable(getBucketLayout()).put(ozoneKey, keyInfo); @@ -634,4 +660,94 @@ public void testDeleteMarkerConsumesNamespaceButNoSpace() throws Exception { assertEquals(usedBytes, after.getUsedBytes()); assertEquals(usedNamespace + 1, after.getUsedNamespace()); } + + /** + * A write while versioning is suspended takes the key's null version slot + * instead of creating a version of its own. + */ + @Test + public void testSuspendedWriteTakesTheNullVersionSlot() throws Exception { + setupSuspendedBucket(); + + commitAt(500L); + + OmKeyInfo current = currentVersion(); + assertNotNull(current); + assertTrue(current.isNullVersion()); + assertEquals(500L, current.getVersionId()); + } + + /** + * Repeated suspended writes replace each other: versions do not accumulate, + * and the replaced record's blocks are queued for reclamation. + */ + @Test + public void testSuspendedWriteReplacesTheCurrentNullVersion() + throws Exception { + setupSuspendedBucket(); + seedCurrentVersion(100L, false, true, true); + + OMKeyCommitResponse response = + (OMKeyCommitResponse) commitAt(500L); + + assertTrue(currentVersion().isNullVersion()); + assertEquals(500L, currentVersion().getVersionId()); + // the replaced null version is not kept as a noncurrent version + assertNull(noncurrentVersion(100L)); + // its blocks are queued for reclamation instead + assertReclaimed(response, 100L); + } + + /** Asserts that the given version was queued for block reclamation. */ + private void assertReclaimed(OMKeyCommitResponse response, long versionId) { + assertNotNull(response.getKeysToDelete()); + assertTrue(response.getKeysToDelete().values().stream() + .flatMap(repeated -> repeated.getOmKeyInfoList().stream()) + .anyMatch(info -> info.getVersionId() != null + && info.getVersionId() == versionId), + "version " + versionId + " was not queued for reclamation"); + } + + /** + * Versions created while versioning was enabled are not touched by a + * suspended write: the one it supersedes becomes noncurrent as usual. + */ + @Test + public void testSuspendedWriteKeepsEnabledEraVersions() throws Exception { + setupSuspendedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(100L, false); + + commitAt(500L); + + assertTrue(currentVersion().isNullVersion()); + // the version it superseded is retained, as is the older one + assertNotNull(noncurrentVersion(300L)); + assertFalse(noncurrentVersion(300L).isNullVersion()); + assertNotNull(noncurrentVersion(100L)); + } + + /** + * The null version may be noncurrent, when versioning was enabled again + * after the write that created it. A suspended write still replaces it, and + * still becomes the current version. + */ + @Test + public void testSuspendedWriteReplacesANoncurrentNullVersion() + throws Exception { + setupSuspendedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(200L, true); + seedNoncurrentVersion(100L, false); + + commitAt(500L); + + OmKeyInfo current = currentVersion(); + assertTrue(current.isNullVersion()); + assertEquals(500L, current.getVersionId()); + // the old null version is gone, everything else is retained + assertNull(noncurrentVersion(200L)); + assertNotNull(noncurrentVersion(300L)); + assertNotNull(noncurrentVersion(100L)); + } } From d1b922c96ad5157dfeab6ad330dd4c8e8fc51ec6 Mon Sep 17 00:00:00 2001 From: Symious Date: Tue, 4 Aug 2026 16:12:48 +0800 Subject: [PATCH 21/23] T5.2. Write a null delete marker while versioning is suspended Co-Authored-By: Claude Opus 5 --- .../om/request/key/OMKeyDeleteRequest.java | 45 +++++++++++-- .../key/OMKeyDeleteMarkerResponse.java | 39 +++++++++++- .../key/TestOMKeyVersioningRequests.java | 63 +++++++++++++++++++ 3 files changed, 140 insertions(+), 7 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index cdf0fa6e03ca..e9765e7e4723 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -50,6 +50,7 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup; +import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; @@ -169,7 +170,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut if (keyArgs.hasVersionId() || keyArgs.getNullVersion()) { // DELETE ?versionId= permanently removes one version. It is the only // delete that destroys data on a versioned bucket. - if (!omBucketInfo.isS3VersioningEnabled()) { + if (!omBucketInfo.hasEverBeenVersioned()) { throw new OMException("Bucket " + bucketName + " does not have S3 versioning enabled", NOT_SUPPORTED_OPERATION); @@ -180,11 +181,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut omResponse); // Noncurrent versions are invisible to plain reads, so removing one // does not change the visible key count. - } else if (omBucketInfo.isS3VersioningEnabled()) { + } else if (omBucketInfo.hasEverBeenVersioned()) { // A delete without a versionId removes no data: a delete marker // becomes the current version and the version it supersedes moves to // the versionedKeyTable. Like S3, the marker is inserted even when the - // key does not exist. + // key does not exist. While versioning is suspended the marker takes + // the key's null version slot instead of creating a version. insertingDeleteMarker = true; omClientResponse = insertDeleteMarker(ozoneManager, omMetadataManager, omBucketInfo, omKeyInfo, objectKey, keyArgs, trxnLogIndex, @@ -412,6 +414,11 @@ private OMClientResponse insertDeleteMarker(OzoneManager ozoneManager, String volumeName = omBucketInfo.getVolumeName(); String bucketName = omBucketInfo.getBucketName(); String keyName = keyArgs.getKeyName(); + // While versioning is suspended the marker is the key's null version, so + // it replaces whatever held that slot instead of superseding it. + final boolean suspended = omBucketInfo.isS3VersioningSuspended(); + final boolean replacesCurrent = suspended && currentVersion != null + && currentVersion.isNullVersionRecord(); // Everything that can fail runs before the first cache entry is added: a // request that throws here is answered with an OMKeyDeleteResponse, whose @@ -450,12 +457,12 @@ private OMClientResponse insertDeleteMarker(OzoneManager ozoneManager, .setUpdateID(trxnLogIndex) .setVersionId(markerVersionId) .setDeleteMarker(true) - .setNullVersion(false) + .setNullVersion(suspended) .build(); String movedVersionedKeyName = null; OmKeyInfo movedVersionedKeyInfo = null; - if (currentVersion != null) { + if (currentVersion != null && !replacesCurrent) { movedVersionedKeyInfo = currentVersion.getVersionId() != null ? currentVersion : currentVersion.toBuilder() @@ -469,6 +476,31 @@ private OMClientResponse insertDeleteMarker(OzoneManager ozoneManager, movedVersionedKeyName, movedVersionedKeyInfo, trxnLogIndex); } + // The null version the marker replaces is removed: the current one when + // the last write was also suspended, and a noncurrent one when versioning + // was enabled in between. + OmKeyInfo replacedNullVersion = replacesCurrent ? currentVersion : null; + String replacedNullVersionKey = null; + if (suspended && !replacesCurrent) { + Pair nullVersion = getNoncurrentNullVersion( + omMetadataManager, volumeName, bucketName, keyName); + if (nullVersion != null) { + replacedNullVersionKey = nullVersion.getKey(); + replacedNullVersion = nullVersion.getValue(); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + new CacheKey<>(replacedNullVersionKey), + CacheValue.get(trxnLogIndex)); + } + } + Map keysToDelete = null; + if (replacedNullVersion != null) { + keysToDelete = addKeyInfoToDeleteMap(ozoneManager, trxnLogIndex, + objectKey, omBucketInfo.getObjectID(), + replacedNullVersion.withCommittedKeyDeletedFlag(true), null); + omBucketInfo.decrUsedBytes(sumBlockLengths(replacedNullVersion), true); + omBucketInfo.decrUsedNamespace(1L, true); + } + omBucketInfo.incrUsedNamespace(1L); omMetadataManager.getKeyTable(getBucketLayout()).addCacheEntry( @@ -477,7 +509,8 @@ private OMClientResponse insertDeleteMarker(OzoneManager ozoneManager, return new OMKeyDeleteMarkerResponse( omResponse.setDeleteKeyResponse(DeleteKeyResponse.newBuilder()).build(), deleteMarker, objectKey, movedVersionedKeyName, movedVersionedKeyInfo, - omBucketInfo.copyObject()); + omBucketInfo.copyObject()) + .withReplacedNullVersion(replacedNullVersionKey, keysToDelete); } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java index 822596d57c9f..50d4821b13e7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteMarkerResponse.java @@ -18,16 +18,20 @@ package org.apache.hadoop.ozone.om.response.key; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DELETED_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VERSIONED_KEY_TABLE; +import com.google.common.annotations.VisibleForTesting; import jakarta.annotation.Nonnull; import java.io.IOException; +import java.util.Map; import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.response.CleanupTableInfo; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -37,7 +41,8 @@ * the keyTable, and the version it supersedes (if the key existed) moves to * the versionedKeyTable. */ -@CleanupTableInfo(cleanupTables = {KEY_TABLE, VERSIONED_KEY_TABLE, BUCKET_TABLE}) +@CleanupTableInfo(cleanupTables = {KEY_TABLE, VERSIONED_KEY_TABLE, BUCKET_TABLE, + DELETED_TABLE}) public class OMKeyDeleteMarkerResponse extends OmKeyResponse { private OmKeyInfo deleteMarker; @@ -45,6 +50,8 @@ public class OMKeyDeleteMarkerResponse extends OmKeyResponse { private String movedVersionedKeyName; private OmKeyInfo movedVersionedKeyInfo; private OmBucketInfo omBucketInfo; + private String replacedNullVersionKey; + private Map keysToDelete; public OMKeyDeleteMarkerResponse(@Nonnull OMResponse omResponse, @Nonnull OmKeyInfo deleteMarker, @Nonnull String ozoneKeyName, @@ -68,6 +75,23 @@ public OMKeyDeleteMarkerResponse(@Nonnull OMResponse omResponse, checkStatusNotOK(); } + /** + * The null version the marker replaced, when versioning is suspended: the + * versionedKeyTable entry to remove, if it had one, and its blocks to queue + * for reclamation. + */ + public OMKeyDeleteMarkerResponse withReplacedNullVersion( + String dbVersionedKey, Map deleteMap) { + this.replacedNullVersionKey = dbVersionedKey; + this.keysToDelete = deleteMap; + return this; + } + + @VisibleForTesting + public Map getKeysToDelete() { + return keysToDelete; + } + @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { @@ -79,6 +103,19 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, movedVersionedKeyName, movedVersionedKeyInfo); } + if (replacedNullVersionKey != null) { + omMetadataManager.getVersionedKeyTable() + .deleteWithBatch(batchOperation, replacedNullVersionKey); + } + + if (keysToDelete != null) { + for (Map.Entry entry + : keysToDelete.entrySet()) { + omMetadataManager.getDeletedTable().putWithBatch(batchOperation, + entry.getKey(), entry.getValue()); + } + } + omMetadataManager.getBucketTable().putWithBatch(batchOperation, omMetadataManager.getBucketKey(omBucketInfo.getVolumeName(), omBucketInfo.getBucketName()), omBucketInfo); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java index d1438685263d..1ee0c566d632 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -750,4 +750,67 @@ public void testSuspendedWriteReplacesANoncurrentNullVersion() assertNotNull(noncurrentVersion(300L)); assertNotNull(noncurrentVersion(100L)); } + + /** + * A delete while versioning is suspended writes a marker into the key's null + * version slot rather than creating a version. + */ + @Test + public void testSuspendedDeleteWritesANullMarker() throws Exception { + setupSuspendedBucket(); + seedCurrentVersion(300L); + + deleteAt(500L); + + OmKeyInfo current = currentVersion(); + assertNotNull(current); + assertTrue(current.isDeleteMarker()); + assertTrue(current.isNullVersion()); + // the version it superseded is retained, as under an enabled bucket + assertNotNull(noncurrentVersion(300L)); + } + + /** The null marker replaces the null version that held the slot. */ + @Test + public void testSuspendedDeleteReplacesTheCurrentNullVersion() + throws Exception { + setupSuspendedBucket(); + seedCurrentVersion(100L, false, true, true); + + OMKeyDeleteMarkerResponse response = + (OMKeyDeleteMarkerResponse) deleteAt(500L); + + OmKeyInfo current = currentVersion(); + assertTrue(current.isDeleteMarker()); + assertTrue(current.isNullVersion()); + // the replaced record is not kept as a noncurrent version + assertNull(noncurrentVersion(100L)); + assertNotNull(response.getKeysToDelete()); + } + + /** + * Versions created while versioning was enabled stay readable and deletable + * by versionId after a suspended delete. + */ + @Test + public void testSuspendedDeleteKeepsEnabledEraVersions() throws Exception { + setupSuspendedBucket(); + seedCurrentVersion(300L); + seedNoncurrentVersion(200L, true); + seedNoncurrentVersion(100L, false); + + deleteAt(500L); + + assertTrue(currentVersion().isDeleteMarker()); + // the superseded version and the older one are retained + assertNotNull(noncurrentVersion(300L)); + assertNotNull(noncurrentVersion(100L)); + // only the null version the marker replaced is gone + assertNull(noncurrentVersion(200L)); + + // and a retained version can still be deleted by versionId + assertEquals(OzoneManagerProtocolProtos.Status.OK, + deleteVersionAt(100L, false, 600L).getOMResponse().getStatus()); + assertNull(noncurrentVersion(100L)); + } } From 504f33025db4bb74b6628dedb02cef1590134706 Mon Sep 17 00:00:00 2001 From: Symious Date: Tue, 4 Aug 2026 16:12:57 +0800 Subject: [PATCH 22/23] T5.3. Address pre-versioning records as the key's null version Co-Authored-By: Claude Opus 5 --- .../hadoop/ozone/om/KeyManagerImpl.java | 4 +- .../om/request/key/OMKeyDeleteRequest.java | 2 +- .../ozone/om/request/key/OMKeyRequest.java | 2 +- .../hadoop/ozone/om/TestKeyManagerUnit.java | 48 +++++++++++++++++++ .../key/TestOMKeyVersioningRequests.java | 41 ++++++++++++++++ 5 files changed, 93 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 991903d49f9b..33b8c2c3a9fd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -689,7 +689,7 @@ private OmKeyInfo getOmKeyInfo(String volumeName, String bucketName, private OmKeyInfo getAddressedVersion(OmKeyArgs args, String volumeName, String bucketName, String keyName, OmKeyInfo current) throws IOException { if (args.isNullVersion()) { - if (current != null && current.isNullVersion()) { + if (current != null && current.isNullVersionRecord()) { return current; } // The null version carries a normally generated versionId, so it can only @@ -702,7 +702,7 @@ private OmKeyInfo getAddressedVersion(OmKeyArgs args, String volumeName, metadataManager.getVersionedKeyTable().iterator(prefix)) { while (versions.hasNext()) { OmKeyInfo version = versions.next().getValue(); - if (version.isNullVersion()) { + if (version.isNullVersionRecord()) { return version; } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index e9765e7e4723..5c9ee1848781 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -331,7 +331,7 @@ private OMClientResponse deleteVersion(OMMetadataManager omMetadataManager, boolean nullVersion = keyArgs.getNullVersion(); boolean deletingCurrent = currentVersion != null && (nullVersion - ? currentVersion.isNullVersion() + ? currentVersion.isNullVersionRecord() : Long.valueOf(keyArgs.getVersionId()).equals( currentVersion.getVersionId())); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java index 2a1cab726889..42eeeace25f6 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java @@ -917,7 +917,7 @@ protected Pair getNoncurrentNullVersion( OMMetadataManager omMetadataManager, String volumeName, String bucketName, String keyName) throws IOException { return findNoncurrentVersion(omMetadataManager, volumeName, bucketName, - keyName, OmKeyInfo::isNullVersion); + keyName, OmKeyInfo::isNullVersionRecord); } /** diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index b2753e659947..ace48d8ffea2 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -23,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.anySet; @@ -598,6 +599,53 @@ private OmKeyInfo versionedKeyInfo(String volume, String bucket, String key, .build(); } + /** + * Enabling versioning does not rewrite the objects a bucket already holds: + * they keep no versionId at all, and S3 reports their version as "null". + */ + @Test + public void testLookupPreVersioningKeyAsNullVersion() throws Exception { + String volume = "vol-legacy"; + String bucket = "buck-legacy"; + String key = "obj"; + OMRequestTestUtils.addVolumeAndBucketToDB(volume, bucket, metadataManager, + BucketLayout.OBJECT_STORE); + + // written before versioning was enabled: no versionId, no null flag + OmKeyInfo legacy = new OmKeyInfo.Builder() + .setVolumeName(volume) + .setBucketName(bucket) + .setKeyName(key) + .setOmKeyLocationInfos(Collections.emptyList()) + .setCreationTime(Time.now()) + .setModificationTime(Time.now()) + .setDataSize(0) + .setReplicationConfig( + RatisReplicationConfig.getInstance(ReplicationFactor.ONE)) + .build(); + metadataManager.getKeyTable(BucketLayout.OBJECT_STORE).put( + metadataManager.getOzoneKey(volume, bucket, key), legacy); + + OmKeyArgs.Builder base = new OmKeyArgs.Builder() + .setVolumeName(volume).setBucketName(bucket).setKeyName(key) + .setHeadOp(true); + + // a plain read still returns it + OmKeyArgs args = base.build(); + assertNull(keyManager.lookupKey(args, resolveBucket(args), null) + .getVersionId()); + + // and version "null" addresses it, without the record having been rewritten + args = base.setNullVersion(true).build(); + OmKeyInfo nullVersion = + keyManager.lookupKey(args, resolveBucket(args), null); + assertNull(nullVersion.getVersionId()); + assertTrue(nullVersion.isNullVersionRecord()); + assertEquals(legacy, metadataManager + .getKeyTable(BucketLayout.OBJECT_STORE) + .get(metadataManager.getOzoneKey(volume, bucket, key))); + } + @Test public void testLookupKeyByVersionId() throws Exception { String volume = "vol-ver"; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java index 1ee0c566d632..1cab35d136e6 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyVersioningRequests.java @@ -813,4 +813,45 @@ public void testSuspendedDeleteKeepsEnabledEraVersions() throws Exception { deleteVersionAt(100L, false, 600L).getOMResponse().getStatus()); assertNull(noncurrentVersion(100L)); } + + /** + * A key written before versioning was enabled carries no versionId and no + * null flag. It is the key's null version all the same, so version "null" + * addresses it - without the record ever having been rewritten. + */ + @Test + public void testPreVersioningCurrentIsAddressableAsNullVersion() + throws Exception { + setupVersionedBucket(); + String ozoneKey = seedCurrentVersion(null); + OmKeyInfo legacy = currentVersion(); + assertNull(legacy.getVersionId()); + assertFalse(legacy.isNullVersion()); + assertTrue(legacy.isNullVersionRecord()); + + // deleting version "null" hits it even though it carries no flag + assertEquals(OzoneManagerProtocolProtos.Status.OK, + deleteVersionAt(null, true, 500L).getOMResponse().getStatus()); + assertNull(omMetadataManager.getKeyTable(getBucketLayout()).get(ozoneKey)); + } + + /** + * A suspended write replaces a pre-versioning record, since that record is + * the key's null version. + */ + @Test + public void testSuspendedWriteReplacesAPreVersioningRecord() + throws Exception { + setupSuspendedBucket(); + seedCurrentVersion(null, false, false, true); + + OMKeyCommitResponse response = (OMKeyCommitResponse) commitAt(500L); + + OmKeyInfo current = currentVersion(); + assertTrue(current.isNullVersion()); + assertEquals(500L, current.getVersionId()); + // it was replaced, not kept as a noncurrent version + assertNull(noncurrentVersion(VersionIdGenerator.UNSET_VERSION_ID)); + assertNotNull(response.getKeysToDelete()); + } } From 7a5bc5012d37608e7ac63b30239b23f6a27551ab Mon Sep 17 00:00:00 2001 From: Symious Date: Tue, 4 Aug 2026 16:13:06 +0800 Subject: [PATCH 23/23] T5.4. Keep the superseded version when a multipart upload completes Co-Authored-By: Claude Opus 5 --- .../S3MultipartUploadCompleteRequest.java | 64 ++++++++++++++-- .../S3MultipartUploadCompleteResponse.java | 29 +++++++- .../s3/multipart/S3MultipartRequestTests.java | 3 + .../TestS3MultipartUploadCompleteRequest.java | 73 +++++++++++++++++-- 4 files changed, 156 insertions(+), 13 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java index 18876e982874..0a6a224ea828 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java @@ -34,6 +34,7 @@ import java.util.function.BiFunction; 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.ReplicationConfig; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; @@ -55,6 +56,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartPartInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartPartKey; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.VersionIdGenerator; import org.apache.hadoop.ozone.om.request.file.OMFileRequest; import org.apache.hadoop.ozone.om.request.key.OMKeyRequest; import org.apache.hadoop.ozone.om.request.util.OMMultipartUploadUtils; @@ -331,12 +333,23 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut OmKeyInfo keyToDelete = omMetadataManager.getKeyTable(getBucketLayout()).get(dbOzoneKey); boolean isNamespaceUpdate = false; - // The S3 versioning check is redundant while the legacy flag is kept in - // sync with an ENABLED status, but the reclaim must depend on the - // status rather than on that sync: dropping the previous version's - // blocks would strand the version record kept for it. + // Completing a multipart upload creates a version like any other + // write: the version it supersedes is kept instead of reclaimed, + // except for the null version, which a suspended write replaces. + final boolean supersededVersionRetained = keyToDelete != null + && omBucketInfo.hasEverBeenVersioned() + && (omBucketInfo.isS3VersioningEnabled() + || !keyToDelete.isNullVersionRecord()); + if (omBucketInfo.hasEverBeenVersioned()) { + omKeyInfo = omKeyInfo.toBuilder() + .setVersionId(ozoneManager.getVersionIdAllocator().allocate( + omMetadataManager, volumeName, bucketName, keyName, + trxnLogIndex, keyToDelete)) + .setNullVersion(omBucketInfo.isS3VersioningSuspended()) + .build(); + } if (keyToDelete != null && !omBucketInfo.getIsVersionEnabled() - && !omBucketInfo.isS3VersioningEnabled()) { + && !supersededVersionRetained) { RepeatedOmKeyInfo oldKeyVersionsToDelete = getOldVersionsToCleanUp( keyToDelete, omBucketInfo.getObjectID(), trxnLogIndex); allKeyInfoToRemove.addAll(oldKeyVersionsToDelete.getOmKeyInfoList()); @@ -347,6 +360,41 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut isNamespaceUpdate = true; } + // The superseded version becomes a noncurrent version. Without this the + // record would be dropped from the keyTable, kept out of the + // deletedTable by the check above, and leak. + String dbVersionedKey = null; + OmKeyInfo versionedKeyInfo = null; + if (supersededVersionRetained) { + versionedKeyInfo = keyToDelete.getVersionId() != null ? keyToDelete + : keyToDelete.toBuilder() + .setVersionId(VersionIdGenerator.UNSET_VERSION_ID) + .setNullVersion(true) + .build(); + dbVersionedKey = omMetadataManager.getVersionedOzoneKey( + volumeName, bucketName, keyName, versionedKeyInfo.getVersionId()); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + dbVersionedKey, versionedKeyInfo, trxnLogIndex); + } + + // A suspended write replaces the key's null version wherever it is. + String replacedNullVersionKey = null; + if (omBucketInfo.isS3VersioningSuspended() + && supersededVersionRetained) { + Pair nullVersion = getNoncurrentNullVersion( + omMetadataManager, volumeName, bucketName, keyName); + if (nullVersion != null) { + replacedNullVersionKey = nullVersion.getKey(); + allKeyInfoToRemove.add(nullVersion.getValue() + .withCommittedKeyDeletedFlag(true)); + usedBytesDiff -= nullVersion.getValue().getReplicatedSize(); + omBucketInfo.decrUsedNamespace(1L, true); + omMetadataManager.getVersionedKeyTable().addCacheEntry( + new CacheKey<>(replacedNullVersionKey), + CacheValue.get(trxnLogIndex)); + } + } + String dbBucketKey = omMetadataManager.getBucketKey( omBucketInfo.getVolumeName(), omBucketInfo.getBucketName()); if (usedBytesDiff != 0) { @@ -371,10 +419,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut long volumeId = omMetadataManager.getVolumeId(volumeName); long bucketId = omMetadataManager.getBucketId(volumeName, bucketName); omClientResponse = - getOmClientResponse(multipartKey, omResponse, dbMultipartOpenKey, + ((S3MultipartUploadCompleteResponse) getOmClientResponse(multipartKey, omResponse, dbMultipartOpenKey, omKeyInfo, allKeyInfoToRemove, omBucketInfo, volumeId, bucketId, missingParentInfos, multipartKeyInfo, - multipartPartKeysToDelete); + multipartPartKeysToDelete)) + .withVersionedKey(dbVersionedKey, versionedKeyInfo, + replacedNullVersionKey); result = Result.SUCCESS; } else { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java index a1dfe4ed317d..b9da28f2bbb4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadCompleteResponse.java @@ -23,6 +23,7 @@ import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_PARTS_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE; +import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VERSIONED_KEY_TABLE; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; @@ -48,7 +49,8 @@ * 3) Delete unused parts. */ @CleanupTableInfo(cleanupTables = {OPEN_KEY_TABLE, KEY_TABLE, DELETED_TABLE, - MULTIPART_INFO_TABLE, MULTIPART_PARTS_TABLE, BUCKET_TABLE}) + MULTIPART_INFO_TABLE, MULTIPART_PARTS_TABLE, BUCKET_TABLE, + VERSIONED_KEY_TABLE}) public class S3MultipartUploadCompleteResponse extends OmKeyResponse { private String multipartKey; private String multipartOpenKey; @@ -57,6 +59,9 @@ public class S3MultipartUploadCompleteResponse extends OmKeyResponse { private List multipartPartKeysToDelete; private OmBucketInfo omBucketInfo; private long bucketId; + private String versionedKeyName; + private OmKeyInfo versionedKeyInfo; + private String replacedNullVersionKey; @SuppressWarnings("parameternumber") public S3MultipartUploadCompleteResponse( @@ -128,6 +133,19 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, } } + /** + * The version this upload superseded, to be kept in the versionedKeyTable as + * a noncurrent version, and the null version it replaced, if any. Both null + * for buckets that have never been versioned. + */ + public S3MultipartUploadCompleteResponse withVersionedKey( + String dbVersionedKey, OmKeyInfo keyInfo, String replacedNullVersion) { + this.versionedKeyName = dbVersionedKey; + this.versionedKeyInfo = keyInfo; + this.replacedNullVersionKey = replacedNullVersion; + return this; + } + protected String addToKeyTable(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { @@ -135,6 +153,15 @@ protected String addToKeyTable(OMMetadataManager omMetadataManager, omKeyInfo.getBucketName(), omKeyInfo.getKeyName()); omMetadataManager.getKeyTable(getBucketLayout()) .putWithBatch(batchOperation, ozoneKey, omKeyInfo); + + if (versionedKeyInfo != null) { + omMetadataManager.getVersionedKeyTable() + .putWithBatch(batchOperation, versionedKeyName, versionedKeyInfo); + } + if (replacedNullVersionKey != null) { + omMetadataManager.getVersionedKeyTable() + .deleteWithBatch(batchOperation, replacedNullVersionKey); + } return ozoneKey; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartRequestTests.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartRequestTests.java index e23b84f52939..e621c0c74d6a 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartRequestTests.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartRequestTests.java @@ -45,6 +45,7 @@ import org.apache.hadoop.ozone.om.OmMetadataReader; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.ResolvedBucket; +import org.apache.hadoop.ozone.om.VersionIdAllocator; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.KeyValueUtil; import org.apache.hadoop.ozone.om.request.OMClientRequest; @@ -112,6 +113,8 @@ public void setup() throws Exception { when(lvm.getMetadataLayoutVersion()).thenReturn(0); when(ozoneManager.getVersionManager()).thenReturn(lvm); when(ozoneManager.getConfiguration()).thenReturn(ozoneConfiguration); + when(ozoneManager.getVersionIdAllocator()) + .thenReturn(new VersionIdAllocator(ozoneConfiguration)); when(ozoneManager.getConfig()).thenReturn(ozoneConfiguration.getObject(OmConfig.class)); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/TestS3MultipartUploadCompleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/TestS3MultipartUploadCompleteRequest.java index 819f4bf448d2..1e1cf5d18fe4 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/TestS3MultipartUploadCompleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/multipart/TestS3MultipartUploadCompleteRequest.java @@ -20,9 +20,11 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeFalse; import java.io.IOException; import java.util.ArrayList; @@ -35,6 +37,8 @@ import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.BucketVersioningStatus; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; @@ -98,6 +102,51 @@ public void testValidateAndUpdateCacheSuccess() throws Exception { checkDeleteTableCount(volumeName, bucketName, keyName, 1, uploadId); } + + /** + * Completing a multipart upload over an existing key on a versioned bucket + * creates a version like any other write: the version it supersedes has to + * be kept in the versionedKeyTable, not dropped from the keyTable and left + * out of the deletedTable. + */ + @Test + public void testVersionedOverwriteKeepsPreviousVersion() throws Exception { + // versioning is only supported on OBJECT_STORE buckets + assumeFalse(getBucketLayout().isFileSystemOptimized()); + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + String keyName = getKeyName(); + OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, omMetadataManager, + OmBucketInfo.newBuilder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + // versioning is only allowed on OBJECT_STORE buckets; it shares + // the keyTable with LEGACY, so the request helpers still apply + .setBucketLayout(BucketLayout.OBJECT_STORE) + .setVersioningStatus(BucketVersioningStatus.ENABLED)); + + checkValidateAndUpdateCacheSuccess(volumeName, bucketName, keyName, + new HashMap<>(), new HashMap<>(), 0L, 1L); + OmKeyInfo firstVersion = omMetadataManager.getKeyTable(getBucketLayout()) + .get(getOzoneDBKey(volumeName, bucketName, keyName)); + assertNotNull(firstVersion.getVersionId()); + + checkValidateAndUpdateCacheSuccess(volumeName, bucketName, keyName, + new HashMap<>(), new HashMap<>(), 10L, 2L); + + OmKeyInfo current = omMetadataManager.getKeyTable(getBucketLayout()) + .get(getOzoneDBKey(volumeName, bucketName, keyName)); + assertNotEquals(firstVersion.getVersionId(), current.getVersionId()); + + // the superseded version survives as a noncurrent version + OmKeyInfo noncurrent = omMetadataManager.getVersionedKeyTable().get( + omMetadataManager.getVersionedOzoneKey(volumeName, bucketName, keyName, + firstVersion.getVersionId())); + assertNotNull(noncurrent, + "the version the upload superseded was neither kept nor reclaimed"); + assertEquals(firstVersion.getVersionId(), noncurrent.getVersionId()); + } + public void checkDeleteTableCount(String volumeName, String bucketName, String keyName, int count, String uploadId) throws Exception { @@ -122,6 +171,21 @@ public void checkDeleteTableCount(String volumeName, private String checkValidateAndUpdateCacheSuccess(String volumeName, String bucketName, String keyName, Map metadata, Map tags) throws Exception { + return checkValidateAndUpdateCacheSuccess(volumeName, bucketName, keyName, + metadata, tags, 0L, getNamespaceCount()); + } + + /** + * @param trxnBase offset added to the transaction indexes, so that a test can + * run the flow more than once: versionIds have to increase within a key. + * @param expectedNamespace the bucket's used namespace once the upload + * completes; a versioned overwrite adds a record rather than replacing + * one, so the count grows. + */ + private String checkValidateAndUpdateCacheSuccess(String volumeName, + String bucketName, String keyName, Map metadata, + Map tags, long trxnBase, long expectedNamespace) + throws Exception { OMRequest initiateMPURequest = doPreExecuteInitiateMPU(volumeName, bucketName, keyName, metadata, tags); @@ -130,7 +194,7 @@ private String checkValidateAndUpdateCacheSuccess(String volumeName, getS3InitiateMultipartUploadReq(initiateMPURequest); OMClientResponse omClientResponse = - s3InitiateMultipartUploadRequest.validateAndUpdateCache(ozoneManager, 1L); + s3InitiateMultipartUploadRequest.validateAndUpdateCache(ozoneManager, trxnBase + 1L); long clientID = Time.now(); String multipartUploadID = omClientResponse.getOMResponse() @@ -145,7 +209,7 @@ private String checkValidateAndUpdateCacheSuccess(String volumeName, // Add key to open key table. addKeyToTable(volumeName, bucketName, keyName, clientID); - s3MultipartUploadCommitPartRequest.validateAndUpdateCache(ozoneManager, 2L); + s3MultipartUploadCommitPartRequest.validateAndUpdateCache(ozoneManager, trxnBase + 2L); List partList = new ArrayList<>(); @@ -166,7 +230,7 @@ private String checkValidateAndUpdateCacheSuccess(String volumeName, getS3MultipartUploadCompleteReq(completeMultipartRequest); omClientResponse = - s3MultipartUploadCompleteRequest.validateAndUpdateCache(ozoneManager, 3L); + s3MultipartUploadCompleteRequest.validateAndUpdateCache(ozoneManager, trxnBase + 3L); BatchOperation batchOperation = omMetadataManager.getStore().initBatchOperation(); @@ -201,8 +265,7 @@ private String checkValidateAndUpdateCacheSuccess(String volumeName, .getCacheValue(new CacheKey<>( omMetadataManager.getBucketKey(volumeName, bucketName))) .getCacheValue(); - assertEquals(getNamespaceCount(), - omBucketInfo.getUsedNamespace()); + assertEquals(expectedNamespace, omBucketInfo.getUsedNamespace()); return multipartUploadID; }