This is an automated email from the ASF dual-hosted git repository.
sodonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 6a0d4d5c7f HDDS-7346. Cannot set bucket args when the bucket has quota
set (#3975)
6a0d4d5c7f is described below
commit 6a0d4d5c7fab97c244c338fbc0b2ee6c3878e086
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]
