HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. #1296

captainzmc · 2020-08-06T13:41:37Z

What changes were proposed in this pull request?

In addition, the current Quota setting does not take effect. HDDS-541 gives all the work needed to perfect Quota.
This PR is a subtask of HDDS-541.
First, we increase quotaUsageInBytes of volume. Later, we will judge whether the volume can be written based on this when we write the key.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4053

How was this patch tested?

UT is added

captainzmc · 2020-08-24T11:19:47Z

@ChenSammi CC

cxorm · 2020-09-05T03:21:29Z

Hi @captainzmc ,
Could you be so kind to rebase this PR ?

I would take a look on this.

captainzmc · 2020-09-07T09:03:50Z

Hi @captainzmc ,
Could you be so kind to rebase this PR ?

I would take a look on this.

Thanks for yisheng's attention, PR has been updated. @cxorm Now this PR can be reviewed.

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java

cxorm

Thank you @captainzmc for the work.

Some comments are added inline.

All variable-naming comments could be discussed if you have an idea.
A little confuse on OMKeyCommitRequest.java should be addressed.

The test-part are not reviewed, if someone would take a look on this PR, you could start from here.

cxorm · 2020-09-08T01:45:23Z

hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto

+    optional uint64 quotaUsageInBytes = 12;
+


Suggested change

optional uint64 quotaUsageInBytes = 12;

optional uint64 quotaUsage = 12;

cxorm · 2020-09-08T01:46:17Z

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java

@@ -269,6 +269,7 @@ private OzoneConsts() {
  public static final String KEY = "key";
  public static final String SRC_KEY = "srcKey";
  public static final String DST_KEY = "dstKey";
+  public static final String QUOTA_USAGE_IN_BYTES = "quotaUsageInBytes";


How do we use QUOTA_USAGE = "quotaUsage" here ?

Could you please update the rest part if you agree the idea ?

Using quotaUsage here may not be appropriate because it makes it impossible to distinguish between QuotaUsageInBytes and later QuotaUsageInCount.
Here we can use the usage in ContainerInfo, use usedBytes.

cxorm · 2020-09-08T01:54:09Z

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java

+    this(conf, proxy, name, admin, owner, quotaInBytes, quotaInCounts,
+        creationTime, acls, metadata);
+    this.modificationTime = Instant.ofEpochMilli(modificationTime);
+    this.quotaUsageInBytes = quotaUsageInBytes;


Suggested change

this(conf, proxy, name, admin, owner, quotaInBytes, quotaInCounts,

creationTime, acls, metadata);

this.modificationTime = Instant.ofEpochMilli(modificationTime);

this.quotaUsageInBytes = quotaUsageInBytes;

this(conf, proxy, name, admin, owner, quotaInBytes, quotaInCounts,

creationTime, modificationTime, acls, metadata);

this.quotaUsageInBytes = quotaUsageInBytes;

cxorm · 2020-09-08T02:17:37Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java

@@ -46,6 +47,7 @@
  private long quotaInBytes;
  private long quotaInCounts;
  private final OmOzoneAclMap aclMap;
+  private LongAdder quotaUsageInBytes = new LongAdder();


I'm fine with the LongAdder here.

Out of curiosity,
Could you be so kind to tell me why we use it here ?
(Or please describe the bad pattern if we use just long or Long)

QuotaUsageInBytes is a property of Volume that needs to be updated each time when CreateKey, AllocateBlock, CommitKey, DeleteKey. and at the beginning, we used the volume lock.
But, Previously, only Bucket locks were used for these operation, and use volume lock greatly affect the concurrency performance of different buckets under same volume.
So to avoid Volume locking for poor performance, LongAdder is used here to complete the atomic update of quotaUsageInBytes.
I did a performance test with freon. Multi-threading wrote different buckets under the same volume, and using LongAdder had little impact on the original performance.

cxorm · 2020-09-08T03:38:11Z

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java

@@ -222,6 +218,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,

      acquireLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
          volumeName, bucketName);
+


Suggested change

cxorm · 2020-09-09T01:21:11Z

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java

+      long updateNum = omKeyInfo.getDataSize() * factor -
+          locationInfoList.size() * scmBlockSize * factor;


I'm confused about the meaning of the variable.
(Please let me know the meaning of the variable if I miss something.)

Could you please add some comments about it ?

I will add comment to describe the calculation here.

cxorm · 2020-09-09T01:22:11Z

...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java

@@ -292,14 +289,21 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
          bucketName, Optional.absent(), Optional.of(missingParentInfos),
          trxnLogIndex);

+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = newLocationList.size() * scmBlockSize


Suggested change

long updateNum = newLocationList.size() * scmBlockSize

long preAllocatedSpace = newLocationList.size() * scmBlockSize

cxorm · 2020-09-09T01:23:11Z

...one-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java

@@ -210,10 +206,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
          new CacheKey<>(openKeyName),
          new CacheValue<>(Optional.of(openKeyInfo), trxnLogIndex));

+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = newLocationList.size() * scmBlockSize


Suggested change

long updateNum = newLocationList.size() * scmBlockSize

long preAllocatedSpace = newLocationList.size() * scmBlockSize

cxorm · 2020-09-09T01:24:14Z

...java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java

+      long updateNum = omKeyInfo.getDataSize() * factor -
+          keyArgs.getKeyLocationsList().size() * scmBlockSize * factor;


The same as OMKeyCommitRequest

cxorm · 2020-09-09T01:27:30Z

...one-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java

+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = newLocationList.size() * scmBlockSize
+          * openKeyInfo.getFactor().getNumber();
+      // atom update quotaUsage.


Suggested change

// atom update quotaUsage.

// update quotaUsage atomically.

captainzmc · 2020-09-09T14:00:15Z

Thanks for @cxorm @xiaoyuyao to the review. The new commit has been updated. Could you help take another look?

xiaoyuyao

Thanks @captainzmc for working on this. The patch LGTM, +1. Only concern I have is the cost of volume usage update per key write/delete. If you can post the perf diff with/wo update using freon, that will be great.

ChenSammi · 2020-09-08T11:35:47Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java

@@ -46,6 +47,7 @@
  private long quotaInBytes;
  private long quotaInCounts;
  private final OmOzoneAclMap aclMap;
+  private LongAdder quotaUsageInBytes = new LongAdder();


ChenSammi · 2020-09-08T11:36:28Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java

+    return quotaUsageInBytes;
+  }
+
+  public void setQuotaUsageInBytes(long quotaUsageInBytes) {


setQuotaUsageInBytes -> increase***

ChenSammi · 2020-09-08T12:09:46Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java

-        quotaInCounts, cloneMetadata, cloneAclMap, creationTime,
-        modificationTime, objectID, updateID);
+        quotaInCounts, cloneMetadata, quotaUsageInBytes.sum(), cloneAclMap,
+        creationTime, modificationTime, objectID, updateID);
  }
 }


Should getObjectInfo also be changed?

captainzmc · 2020-09-15T02:32:05Z

Thanks @captainzmc for working on this. The patch LGTM, +1. Only concern I have is the cost of volume usage update per key write/delete. If you can post the perf diff with/wo update using freon, that will be great.

Thanks for @xiaoyuyao 's review. Here are the freon test results:
Using 10-100 threads to write data to different buckets under the same volume, adding quota Usage has little impact on performance. (LongAdder disperses the concurrency of a single cell to each cell, improving concurrency efficiency compared to atomicLong). In my test, I used three virtual machines, each with a key size of 10K.

captainzmc · 2020-09-15T10:05:17Z

Had updated PR to fix review issues and resolve conflict. @ChenSammi Could you help take another look?

ChenSammi · 2020-09-16T04:02:04Z

Thanks @captainzmc for the contribution and @cxorm @xiaoyuyao for review the patch.

* master: (47 commits) HDDS-4104. Provide a way to get the default value and key of java-based-configuration easily (apache#1369) HDDS-4250. Fix wrong logger name (apache#1429) HDDS-4244. Container deleted wrong replica cause mis-replicated. (apache#1423) HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. (apache#1296) HDDS-4210. ResolveBucket during checkAcls fails. (apache#1398) HDDS-4075. Retry request on different OM on AccessControlException (apache#1303) HDDS-4166. Documentation index page redirects to the wrong address (apache#1372) HDDS-4039. Reduce the number of fields in hdds.proto to improve performance (apache#1289) HDDS-4155. Directory and filename can end up with same name in a path. (apache#1361) HDDS-3927. Rename Ozone OM,DN,SCM runtime options to conform to naming conventions (apache#1401) HDDS-4119. Improve performance of the BufferPool management of Ozone client (apache#1336) HDDS-4217.Remove test TestOzoneContainerRatis (apache#1408) HDDS-4218.Remove test TestRatisManager (apache#1409) HDDS-4129. change MAX_QUOTA_IN_BYTES to Long.MAX_VALUE. (apache#1337) HDDS-4228: add field 'num' to ALLOCATE_BLOCK of scm audit log. (apache#1413) HDDS-4196. Add an endpoint in Recon to query Prometheus (apache#1390) HDDS-4211. [OFS] Better owner and group display for listing Ozone volumes and buckets (apache#1397) HDDS-4150. recon.api.TestEndpoints test is flaky (apache#1396) HDDS-4170 - Fix typo in method description. (apache#1406) HDDS-4064. Show container verbose info with verbose option (apache#1290) ...

…ponse * HDDS-4122-quota-attempt2: (51 commits) Remove redundant check status calls in children of AbstractOMOpenKeyDeleteRequest Remove unused inports and fix super constructor calls Move common volume byte usage update code to AbstractOMKeyDeleteResponse Add volume quota update to open key delete response, and group duplicate code HDDS-4104. Provide a way to get the default value and key of java-based-configuration easily (apache#1369) HDDS-4250. Fix wrong logger name (apache#1429) HDDS-4244. Container deleted wrong replica cause mis-replicated. (apache#1423) HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. (apache#1296) HDDS-4210. ResolveBucket during checkAcls fails. (apache#1398) HDDS-4075. Retry request on different OM on AccessControlException (apache#1303) HDDS-4166. Documentation index page redirects to the wrong address (apache#1372) HDDS-4039. Reduce the number of fields in hdds.proto to improve performance (apache#1289) HDDS-4155. Directory and filename can end up with same name in a path. (apache#1361) HDDS-3927. Rename Ozone OM,DN,SCM runtime options to conform to naming conventions (apache#1401) HDDS-4119. Improve performance of the BufferPool management of Ozone client (apache#1336) HDDS-4217.Remove test TestOzoneContainerRatis (apache#1408) HDDS-4218.Remove test TestRatisManager (apache#1409) HDDS-4129. change MAX_QUOTA_IN_BYTES to Long.MAX_VALUE. (apache#1337) HDDS-4228: add field 'num' to ALLOCATE_BLOCK of scm audit log. (apache#1413) HDDS-4196. Add an endpoint in Recon to query Prometheus (apache#1390) ...

captainzmc force-pushed the space-quota-1 branch from a00b566 to fe3011b Compare August 6, 2020 14:06

captainzmc changed the title ~~HDDS-4053. Volume space: add quotaUsageInBytes and update it when create and delete key.~~ HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. Aug 6, 2020

captainzmc force-pushed the space-quota-1 branch from fe3011b to 07f7c17 Compare August 6, 2020 14:18

captainzmc closed this Aug 7, 2020

captainzmc reopened this Aug 7, 2020

captainzmc force-pushed the space-quota-1 branch 2 times, most recently from 642710a to ce40e90 Compare August 14, 2020 01:05

captainzmc closed this Aug 24, 2020

captainzmc reopened this Aug 24, 2020

captainzmc force-pushed the space-quota-1 branch from d200a65 to 20b4104 Compare September 3, 2020 09:29

cxorm self-requested a review September 3, 2020 21:40

captainzmc force-pushed the space-quota-1 branch 3 times, most recently from a8251c3 to cc4327a Compare September 7, 2020 07:43

xiaoyuyao reviewed Sep 8, 2020

View reviewed changes

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java Show resolved Hide resolved

cxorm requested changes Sep 9, 2020

View reviewed changes

xiaoyuyao approved these changes Sep 11, 2020

View reviewed changes

ChenSammi reviewed Sep 14, 2020

View reviewed changes

add volume quota usage

3427790

captainzmc force-pushed the space-quota-1 branch from 411d86f to 0117885 Compare September 15, 2020 08:52

fix review issues and resolve conflict.

c272d01

captainzmc force-pushed the space-quota-1 branch from 0117885 to c272d01 Compare September 15, 2020 10:19

captainzmc added 2 commits September 15, 2020 19:52

fix review issues

bce9bf4

fix s3 MultipartUploadAbort.

203804e

ChenSammi merged commit 7beb2d0 into apache:master Sep 16, 2020

This was referenced Sep 17, 2020

HDDS-4254. Bucket space: add usedBytes and update it when create and delete key. #1431

Merged

HDDS-3727. Volume space: check quotaUsageInBytes when write key. #1434

Merged

errose28 mentioned this pull request Oct 5, 2020

HDDS-4122. Implement OM Delete Expired Open Key Request and Response #1435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. #1296

HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. #1296

captainzmc commented Aug 6, 2020

captainzmc commented Aug 24, 2020

cxorm commented Sep 5, 2020

captainzmc commented Sep 7, 2020

cxorm left a comment

cxorm Sep 8, 2020

cxorm Sep 8, 2020

captainzmc Sep 9, 2020

cxorm Sep 8, 2020

cxorm Sep 8, 2020

captainzmc Sep 9, 2020 •

edited

Loading

cxorm Sep 8, 2020

cxorm Sep 9, 2020

captainzmc Sep 9, 2020

cxorm Sep 9, 2020

cxorm Sep 9, 2020

cxorm Sep 9, 2020

cxorm Sep 9, 2020

captainzmc commented Sep 9, 2020

xiaoyuyao left a comment

ChenSammi Sep 8, 2020

ChenSammi Sep 8, 2020

ChenSammi Sep 8, 2020

captainzmc commented Sep 15, 2020 •

edited

Loading

captainzmc commented Sep 15, 2020

ChenSammi commented Sep 16, 2020

	optional uint64 quotaUsageInBytes = 12;
	optional uint64 quotaUsage = 12;

		@@ -222,6 +218,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,

		acquireLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
		volumeName, bucketName);

		long updateNum = omKeyInfo.getDataSize() * factor -
		locationInfoList.size() * scmBlockSize * factor;

	long updateNum = newLocationList.size() * scmBlockSize
	long preAllocatedSpace = newLocationList.size() * scmBlockSize

		long updateNum = omKeyInfo.getDataSize() * factor -
		keyArgs.getKeyLocationsList().size() * scmBlockSize * factor;

HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. #1296

HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. #1296

Conversation

captainzmc commented Aug 6, 2020

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

captainzmc commented Aug 24, 2020

cxorm commented Sep 5, 2020

captainzmc commented Sep 7, 2020

cxorm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainzmc Sep 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainzmc commented Sep 9, 2020

xiaoyuyao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainzmc commented Sep 15, 2020 • edited Loading

captainzmc commented Sep 15, 2020

ChenSammi commented Sep 16, 2020

captainzmc Sep 9, 2020 •

edited

Loading

captainzmc commented Sep 15, 2020 •

edited

Loading