This is an automated email from the ASF dual-hosted git repository. ckj pushed a commit to branch ozone-1.3 in repository https://gitbox.apache.org/repos/asf/ozone.git
commit bdfd3f128c5a66bdb395559a1f48f20101c9e97d Author: Stephen O'Donnell <[email protected]> AuthorDate: Fri Nov 18 10:58:48 2022 +0000 HDDS-7346. Cannot set bucket args when the bucket has quota set (#3975) --- .../hadoop/ozone/om/helpers/OmBucketArgs.java | 65 +++++++++++++++++----- .../hadoop/ozone/om/helpers/TestOmBucketArgs.java | 61 ++++++++++++++++++++ .../request/bucket/OMBucketSetPropertyRequest.java | 10 ++++ .../bucket/TestOMBucketSetPropertyRequest.java | 46 +++++++++++++++ 4 files changed, 169 insertions(+), 13 deletions(-) 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 b23da41879..b038f894ad 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 @@ -50,8 +50,10 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { */ private StorageType storageType; - private long quotaInBytes; - private long quotaInNamespace; + private long quotaInBytes = OzoneConsts.QUOTA_RESET; + private long quotaInNamespace = OzoneConsts.QUOTA_RESET; + private boolean quotaInBytesSet = false; + private boolean quotaInNamespaceSet = false; private DefaultReplicationConfig defaultReplicationConfig = null; /** * Bucket Owner Name. @@ -64,21 +66,16 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { * @param bucketName - Bucket name. * @param isVersionEnabled - Bucket version flag. * @param storageType - Storage type to be used. - * @param quotaInBytes Volume quota in bytes. - * @param quotaInNamespace Volume quota in counts. */ @SuppressWarnings("checkstyle:ParameterNumber") private OmBucketArgs(String volumeName, String bucketName, Boolean isVersionEnabled, StorageType storageType, - Map<String, String> metadata, long quotaInBytes, long quotaInNamespace, - String ownerName) { + Map<String, String> metadata, String ownerName) { this.volumeName = volumeName; this.bucketName = bucketName; this.isVersionEnabled = isVersionEnabled; this.storageType = storageType; this.metadata = metadata; - this.quotaInBytes = quotaInBytes; - this.quotaInNamespace = quotaInNamespace; this.ownerName = ownerName; } @@ -122,6 +119,13 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { return quotaInBytes; } + /** + * Returns true if quotaInBytes has been set to a non default value. + */ + public boolean hasQuotaInBytes() { + return quotaInBytesSet; + } + /** * Returns Bucket Quota in key counts. * @return quotaInNamespace. @@ -130,6 +134,14 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { return quotaInNamespace; } + /** + * Returns true if quotaInNamespace has been set to a non default value. + */ + public boolean hasQuotaInNamespace() { + return quotaInNamespaceSet; + } + + /** * Returns Bucket default replication config. * @return @@ -146,6 +158,16 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { this.defaultReplicationConfig = defaultReplicationConfig; } + private void setQuotaInBytes(long quotaInBytes) { + this.quotaInBytesSet = true; + this.quotaInBytes = quotaInBytes; + } + + private void setQuotaInNamespace(long quotaInNamespace) { + this.quotaInNamespaceSet = true; + this.quotaInNamespace = quotaInNamespace; + } + /** * Returns Bucket Owner Name. * @@ -190,7 +212,9 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { private Boolean isVersionEnabled; private StorageType storageType; private Map<String, String> metadata; + private boolean quotaInBytesSet = false; private long quotaInBytes; + private boolean quotaInNamespaceSet = false; private long quotaInNamespace; private DefaultReplicationConfig defaultReplicationConfig; private String ownerName; @@ -228,11 +252,13 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { } public Builder setQuotaInBytes(long quota) { + quotaInBytesSet = true; quotaInBytes = quota; return this; } public Builder setQuotaInNamespace(long quota) { + quotaInNamespaceSet = true; quotaInNamespace = quota; return this; } @@ -257,8 +283,14 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { Preconditions.checkNotNull(bucketName); OmBucketArgs omBucketArgs = new OmBucketArgs(volumeName, bucketName, isVersionEnabled, - storageType, metadata, quotaInBytes, quotaInNamespace, ownerName); + storageType, metadata, ownerName); omBucketArgs.setDefaultReplicationConfig(defaultReplicationConfig); + if (quotaInBytesSet) { + omBucketArgs.setQuotaInBytes(quotaInBytes); + } + if (quotaInNamespaceSet) { + omBucketArgs.setQuotaInNamespace(quotaInNamespace); + } return omBucketArgs; } } @@ -276,10 +308,12 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { if (storageType != null) { builder.setStorageType(storageType.toProto()); } - if (quotaInBytes > 0 || quotaInBytes == OzoneConsts.QUOTA_RESET) { + if (quotaInBytesSet && ( + quotaInBytes > 0 || quotaInBytes == OzoneConsts.QUOTA_RESET)) { builder.setQuotaInBytes(quotaInBytes); } - if (quotaInNamespace > 0 || quotaInNamespace == OzoneConsts.QUOTA_RESET) { + if (quotaInNamespaceSet && ( + quotaInNamespace > 0 || quotaInNamespace == OzoneConsts.QUOTA_RESET)) { builder.setQuotaInNamespace(quotaInNamespace); } if (defaultReplicationConfig != null) { @@ -305,14 +339,19 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { bucketArgs.hasStorageType() ? StorageType.valueOf( bucketArgs.getStorageType()) : null, KeyValueUtil.getFromProtobuf(bucketArgs.getMetadataList()), - bucketArgs.getQuotaInBytes(), - bucketArgs.getQuotaInNamespace(), bucketArgs.hasOwnerName() ? bucketArgs.getOwnerName() : null); // OmBucketArgs ctor already has more arguments, so setting the default // replication config separately. omBucketArgs.setDefaultReplicationConfig( new DefaultReplicationConfig(bucketArgs.getDefaultReplicationConfig())); + + if (bucketArgs.hasQuotaInBytes()) { + omBucketArgs.setQuotaInBytes(bucketArgs.getQuotaInBytes()); + } + if (bucketArgs.hasQuotaInNamespace()) { + omBucketArgs.setQuotaInNamespace(bucketArgs.getQuotaInNamespace()); + } return omBucketArgs; } } 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 new file mode 100644 index 0000000000..2e843f84b2 --- /dev/null +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketArgs.java @@ -0,0 +1,61 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.junit.Assert; +import org.junit.Test; + +/** + * Tests for the OmBucketArgs class. + */ +public class TestOmBucketArgs { + + @Test + public void testQuotaIsSetFlagsAreCorrectlySet() { + OmBucketArgs bucketArgs = OmBucketArgs.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .build(); + + Assert.assertEquals(false, bucketArgs.hasQuotaInBytes()); + Assert.assertEquals(false, bucketArgs.hasQuotaInNamespace()); + + OmBucketArgs argsFromProto = OmBucketArgs.getFromProtobuf( + bucketArgs.getProtobuf()); + + Assert.assertEquals(false, argsFromProto.hasQuotaInBytes()); + Assert.assertEquals(false, argsFromProto.hasQuotaInNamespace()); + + bucketArgs = OmBucketArgs.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .setQuotaInNamespace(123) + .setQuotaInBytes(456) + .build(); + + Assert.assertEquals(true, bucketArgs.hasQuotaInBytes()); + Assert.assertEquals(true, bucketArgs.hasQuotaInNamespace()); + + argsFromProto = OmBucketArgs.getFromProtobuf( + bucketArgs.getProtobuf()); + + Assert.assertEquals(true, argsFromProto.hasQuotaInBytes()); + Assert.assertEquals(true, argsFromProto.hasQuotaInNamespace()); + } +} 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 8f71526635..daa9fc3f6c 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 @@ -266,6 +266,11 @@ public class OMBucketSetPropertyRequest extends OMClientRequest { public boolean checkQuotaBytesValid(OMMetadataManager metadataManager, OmVolumeArgs omVolumeArgs, OmBucketArgs omBucketArgs) throws IOException { + if (!omBucketArgs.hasQuotaInBytes()) { + // Quota related values are not in the request, so we don't need to check + // them as they have not changed. + return false; + } long quotaInBytes = omBucketArgs.getQuotaInBytes(); if (quotaInBytes == OzoneConsts.QUOTA_RESET && @@ -307,6 +312,11 @@ public class OMBucketSetPropertyRequest extends OMClientRequest { public boolean checkQuotaNamespaceValid(OmVolumeArgs omVolumeArgs, OmBucketArgs omBucketArgs) { + if (!omBucketArgs.hasQuotaInNamespace()) { + // Quota related values are not in the request, so we don't need to check + // them as they have not changed. + return false; + } long quotaInNamespace = omBucketArgs.getQuotaInNamespace(); if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) { 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 456fa08a96..77efd3562e 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 @@ -21,6 +21,9 @@ package org.apache.hadoop.ozone.om.request.bucket; import java.util.UUID; +import org.apache.hadoop.hdds.client.DefaultReplicationConfig; +import org.apache.hadoop.hdds.client.ECReplicationConfig; +import org.apache.hadoop.ozone.om.helpers.OmBucketArgs; import org.slf4j.LoggerFactory; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -38,6 +41,7 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos .SetBucketPropertyRequest; +import static org.apache.hadoop.hdds.client.ReplicationType.EC; import static org.apache.hadoop.ozone.OzoneConsts.GB; /** @@ -238,4 +242,46 @@ public class TestOMBucketSetPropertyRequest extends TestBucketRequest { String message = omClientResponse.getOMResponse().getMessage(); Assert.assertTrue(message, message.contains("Cannot set property on link")); } + + @Test + public void testSettingRepConfigWithQuota() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + + OMRequestTestUtils.addVolumeToDB( + volumeName, omMetadataManager, 10 * GB); + OMRequestTestUtils.addBucketToDB( + volumeName, bucketName, omMetadataManager, 8 * GB); + + BucketArgs bucketArgs = OmBucketArgs.newBuilder() + .setDefaultReplicationConfig(new DefaultReplicationConfig( + EC, new ECReplicationConfig(3, 2))) + .setBucketName(bucketName) + .setVolumeName(volumeName) + .setIsVersionEnabled(true) + .build() + .getProtobuf(); + + OMRequest omRequest = OMRequest.newBuilder().setSetBucketPropertyRequest( + SetBucketPropertyRequest.newBuilder().setBucketArgs(bucketArgs)) + .setCmdType(OzoneManagerProtocolProtos.Type.SetBucketProperty) + .setClientId(UUID.randomUUID().toString()).build(); + + OMBucketSetPropertyRequest omBucketSetPropertyRequest = + new OMBucketSetPropertyRequest(omRequest); + + OMClientResponse omClientResponse = omBucketSetPropertyRequest + .validateAndUpdateCache(ozoneManager, 1, + ozoneManagerDoubleBufferHelper); + + Assert.assertEquals(true, omClientResponse.getOMResponse().getSuccess()); + + String bucketKey = omMetadataManager.getBucketKey(volumeName, bucketName); + OmBucketInfo dbBucketInfo = + omMetadataManager.getBucketTable().get(bucketKey); + + Assert.assertEquals(8 * GB, dbBucketInfo.getQuotaInBytes()); + Assert.assertEquals(EC, + dbBucketInfo.getDefaultReplicationConfig().getType()); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
