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/11 16:02:04 UTC
[GitHub] [ozone] linyiqun commented on a change in pull request #1445: HDDS-4272. Volume namespace: add usedNamespace and update it when create and delete bucket
linyiqun commented on a change in pull request #1445:
URL: https://github.com/apache/ozone/pull/1445#discussion_r541040652
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -270,6 +270,7 @@ private OzoneConsts() {
public static final String SRC_KEY = "srcKey";
public static final String DST_KEY = "dstKey";
public static final String USED_BYTES = "usedBytes";
+ public static final String USED_NAMESPACE = "usedNamespace";
public static final String QUOTA_IN_BYTES = "quotaInBytes";
public static final String QUOTA_IN_COUNTS = "quotaInCounts";
Review comment:
As I see we defined usedBytes corresponding to quotaInBytes, so quotaInCounts expected to be quotaInNamespace
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -885,6 +885,42 @@ public void testCheckUsedBytesQuota() throws IOException {
Assert.assertEquals(3, countException);
}
+ @Test
+ public void testVolumnUsedNamespace() throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ OzoneVolume volume = null;
+
+ store.createVolume(volumeName);
+ volume = store.getVolume(volumeName);
+ // The initial value should be 0
+ Assert.assertEquals(0L, volume.getUsedNamespace());
+ volume.createBucket(bucketName);
+ // Used namespace should be 1
+ volume = store.getVolume(volumeName);
+ Assert.assertEquals(1L, volume.getUsedNamespace());
+
+ // test linked bucket
+ String targetVolName = UUID.randomUUID().toString();
+ store.createVolume(targetVolName);
+ OzoneVolume volumeWithLinkedBucket = store.getVolume(targetVolName);
+ String targetBucketName = UUID.randomUUID().toString();
+ BucketArgs.Builder argsBuilder = new BucketArgs.Builder()
+ .setStorageType(StorageType.DEFAULT)
+ .setVersioning(false)
+ .setSourceVolume(volumeName)
+ .setSourceBucket(bucketName);
+ volumeWithLinkedBucket.createBucket(targetBucketName, argsBuilder.build());
+ // Used namespace should be 0 because linked bucket does not consume
+ // namespace quota
+ Assert.assertEquals(0L, volumeWithLinkedBucket.getUsedNamespace());
Review comment:
Not fully get this point, why this bucket create way will not increased the namespace quota? Just curious for this.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -205,14 +205,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// Add default acls from volume.
addDefaultAcls(omBucketInfo, omVolumeArgs);
+ // update used namespace for volume
+ omVolumeArgs.incrUsedNamespace(1L);
+
Review comment:
Before this update, we need to do the namespace quota check in checkQuotaBytesValid.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
##########
@@ -134,9 +135,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
omResponse.setDeleteBucketResponse(
DeleteBucketResponse.newBuilder().build());
+ // update used namespace for volume
+ String volumeKey = omMetadataManager.getVolumeKey(volumeName);
+ OmVolumeArgs omVolumeArgs =
+ omMetadataManager.getVolumeTable().getReadCopy(volumeKey);
+ omVolumeArgs.incrUsedNamespace(-1L);
+
Review comment:
Volume table cache also need to be updated here.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
##########
@@ -134,9 +135,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
omResponse.setDeleteBucketResponse(
DeleteBucketResponse.newBuilder().build());
+ // update used namespace for volume
+ String volumeKey = omMetadataManager.getVolumeKey(volumeName);
+ OmVolumeArgs omVolumeArgs =
+ omMetadataManager.getVolumeTable().getReadCopy(volumeKey);
Review comment:
Can you add an empty check for omVolumeArgs before updating the namespace used?
```java
if (volumeArgs == null) {
throw new OMException("Volume " + volume + " is not found",
OMException.ResultCodes.VOLUME_NOT_FOUND);
}
```
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -70,6 +70,10 @@
* Quota of bucket count allocated for the Volume.
*/
private long quotaInCounts;
+ /**
+ * Bucket namespace quota usage.
+ */
+ private long usedNamespace;
Review comment:
Similar comment for renamed quotaInCounts to quotaInNamespace
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -205,14 +205,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
// Add default acls from volume.
addDefaultAcls(omBucketInfo, omVolumeArgs);
+ // update used namespace for volume
+ omVolumeArgs.incrUsedNamespace(1L);
+
// Update table cache.
metadataManager.getBucketTable().addCacheEntry(new CacheKey<>(bucketKey),
new CacheValue<>(Optional.of(omBucketInfo), transactionLogIndex));
Review comment:
We should also update the volume table cache like above, as omVolumeArgs be updated.
----------------------------------------------------------------
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