You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/12/17 14:53:13 UTC

[GitHub] [ozone] linyiqun commented on a change in pull request #1706: HDDS-4277. Support Bucket Namespace Quota Updates

linyiqun commented on a change in pull request #1706:
URL: https://github.com/apache/ozone/pull/1706#discussion_r545145014



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -932,6 +932,50 @@ public void testVolumeUsedNamespace() throws IOException {
     Assert.assertEquals(0L, volume.getUsedNamespace());
   }
 
+  @Test
+  public void testBucketUsedNamespace() throws IOException {
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+    String key1 = UUID.randomUUID().toString();
+    String key2 = UUID.randomUUID().toString();
+    String key3 = UUID.randomUUID().toString();
+    OzoneVolume volume = null;
+    OzoneBucket bucket = null;
+
+    String value = "sample value";
+    int countException = 0;
+
+    store.createVolume(volumeName);
+    volume = store.getVolume(volumeName);
+    volume.createBucket(bucketName);
+    bucket = volume.getBucket(bucketName);
+    bucket.setQuota(OzoneQuota.parseQuota(Long.MAX_VALUE + " Bytes", 2));
+
+    writeKey(bucket, key1, ONE, value, value.length());
+    Assert.assertEquals(1L,
+        store.getVolume(volumeName).getBucket(bucketName).getUsedNamespace());
+
+    writeKey(bucket, key2, ONE, value, value.length());
+    Assert.assertEquals(2L,
+        store.getVolume(volumeName).getBucket(bucketName).getUsedNamespace());
+
+    try {
+      writeKey(bucket, key3, ONE, value, value.length());
+    } catch (IOException ex) {
+      countException++;

Review comment:
       countException is not actually needed, we could add fail() after writeKey and ensure this exception should be happened.
   ```java
    try {
         writeKey(bucket, key3, ONE, value, value.length());
         Assert.fail("Write key should be failed")
       } catch (IOException ex) {
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -309,6 +309,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
 
       omBucketInfo.incrUsedBytes(preAllocatedSpace);
 
+      // Update namespace quota
+      checkBucketQuotaInNamespace(omBucketInfo, 1L);

Review comment:
       Same comment like above.

##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -932,6 +932,50 @@ public void testVolumeUsedNamespace() throws IOException {
     Assert.assertEquals(0L, volume.getUsedNamespace());
   }
 
+  @Test
+  public void testBucketUsedNamespace() throws IOException {
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+    String key1 = UUID.randomUUID().toString();
+    String key2 = UUID.randomUUID().toString();
+    String key3 = UUID.randomUUID().toString();
+    OzoneVolume volume = null;
+    OzoneBucket bucket = null;
+
+    String value = "sample value";
+    int countException = 0;
+
+    store.createVolume(volumeName);
+    volume = store.getVolume(volumeName);
+    volume.createBucket(bucketName);
+    bucket = volume.getBucket(bucketName);
+    bucket.setQuota(OzoneQuota.parseQuota(Long.MAX_VALUE + " Bytes", 2));
+
+    writeKey(bucket, key1, ONE, value, value.length());
+    Assert.assertEquals(1L,
+        store.getVolume(volumeName).getBucket(bucketName).getUsedNamespace());
+
+    writeKey(bucket, key2, ONE, value, value.length());
+    Assert.assertEquals(2L,
+        store.getVolume(volumeName).getBucket(bucketName).getUsedNamespace());
+
+    try {
+      writeKey(bucket, key3, ONE, value, value.length());
+    } catch (IOException ex) {
+      countException++;
+      GenericTestUtils.assertExceptionContains("QUOTA_EXCEEDED", ex);
+    }
+
+    // Write failed, bucket usedBytes should remain as 1
+    Assert.assertEquals(2L,

Review comment:
       This comment should be updated

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -299,6 +299,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
 
       omBucketInfo.incrUsedBytes(preAllocatedSpace);
 
+      // Update namespace quota
+      checkBucketQuotaInNamespace(omBucketInfo, 1L);
+      omBucketInfo.incrUsedNamespace(1L);

Review comment:
       We should move checkBucketQuotaInNamespace check after checkBucketQuotaInBytes. Here we add cache entry before quota check, this will lead the dirty data.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org