HDDS-7497. Fix mkdir does not update bucket's usedNamespace #3969

xichen01 · 2022-11-16T12:16:31Z

What changes were proposed in this pull request?

Fix Ozone fs mkdir does not update bucket's usedNamespace

What is the link to the Apache JIRA

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

How was this patch tested?

Before this PR:
Bucket property usedNamespace will become negative after delete a directory, because the mkdir will not increase the usedNamespace of bucket but rmdir will decrease usedNamespace.

"usedNamespace" can be updated when `mkdir`

[root@Linux /root/ozone]% ozone sh bucket create s3v/bucket1
[root@Linux /root/ozone]% ozone fs -mkdir ofs://localhost/s3v/bucket1/dir1
[root@Linux /root/ozone]% ozone sh bucket info s3v/bucket1
{
  "metadata" : { },
  "volumeName" : "s3v",
  "name" : "bucket1",
  "storageType" : "DISK",
  "versioning" : false,
  "usedBytes" : 0,
  "usedNamespace" : 0,
  "creationTime" : "2022-11-16T11:17:26.512Z",
  "modificationTime" : "2022-11-16T11:17:26.512Z",
  "quotaInBytes" : -1,
  "quotaInNamespace" : -1,
  "bucketLayout" : "OBJECT_STORE",
  "owner" : "root"
}

[root@Linux /root/ozone]% ozone fs -rmdir ofs://localhost/s3v/bucket1/dir1
[root@Linux /root/ozone]% ozone sh bucket info s3v/bucket1
{
  "metadata" : { },
  "volumeName" : "s3v",
  "name" : "bucket1",
  "storageType" : "DISK",
  "versioning" : false,
  "usedBytes" : 0,
  "usedNamespace" : -1,
  "creationTime" : "2022-11-16T11:17:26.512Z",
  "modificationTime" : "2022-11-16T11:17:26.512Z",
  "quotaInBytes" : -1,
  "quotaInNamespace" : -1,
  "bucketLayout" : "OBJECT_STORE",
  "owner" : "root"
}
[root@Linux /root/ozone]%

After this PR:
Bucket property usedNamespace will not be negative

[root@Linux /root/ozone]% ozone sh bucket create s3v/bucket1
[root@Linux /root/ozone]% ozone fs -mkdir ofs://localhost/s3v/bucket1/dir1
[root@Linux /root/ozone]% ozone sh bucket info s3v/bucket1
{
  "metadata" : { },
  "volumeName" : "s3v",
  "name" : "bucket1",
  "storageType" : "DISK",
  "versioning" : false,
  "usedBytes" : 0,
  "usedNamespace" : 1,
  "creationTime" : "2022-11-16T11:17:26.512Z",
  "modificationTime" : "2022-11-16T11:17:26.512Z",
  "quotaInBytes" : -1,
  "quotaInNamespace" : -1,
  "bucketLayout" : "OBJECT_STORE",
  "owner" : "root"
}

[root@Linux /root/ozone]% ozone fs -rmdir ofs://localhost/s3v/bucket1/dir1
[root@Linux /root/ozone]% ozone sh bucket info s3v/bucket1
{
  "metadata" : { },
  "volumeName" : "s3v",
  "name" : "bucket1",
  "storageType" : "DISK",
  "versioning" : false,
  "usedBytes" : 0,
  "usedNamespace" : 0,
  "creationTime" : "2022-11-16T11:17:26.512Z",
  "modificationTime" : "2022-11-16T11:17:26.512Z",
  "quotaInBytes" : -1,
  "quotaInNamespace" : -1,
  "bucketLayout" : "OBJECT_STORE",
  "owner" : "root"
}
[root@Linux /root/ozone]%

`FSO` bucket will have the same result as above

sadanand48

Thanks @xichen01 for working on this, left a few comments inline , please take a look

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

...r/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequestWithFSO.java

...ration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java

kerneltime · 2022-11-21T17:42:01Z

@DaveTeng0

kerneltime · 2022-11-21T17:44:25Z

This is a user facing change, it would be good to describe the desired behavior for FSO, OBS and Legacy.
cc @xBis7

xBis7 · 2022-11-22T13:51:16Z

This is more of a system specification rather than a bug. usedNamespace refers to the number of files. Although, a directory is a key, for ozone it doesn't take up space and therefore it's not calculated. If we were to accept this change then we would have to modify the whole system.

This is from master

bash-4.2$ ozone sh bucket create -l FILE_SYSTEM_OPTIMIZED /s3v/bucket1
bash-4.2$ ozone fs -mkdir /s3v/bucket1/dir1  
bash-4.2$ ozone sh bucket info /s3v/bucket1
{
  "metadata" : { },
  "volumeName" : "s3v",
  "name" : "bucket1",
  "storageType" : "DISK",
  "versioning" : false,
  "usedBytes" : 0,
  "usedNamespace" : 0,
  "creationTime" : "2022-11-22T13:19:47.398Z",
  "modificationTime" : "2022-11-22T13:19:47.398Z",
  "quotaInBytes" : -1,
  "quotaInNamespace" : -1,
  "bucketLayout" : "FILE_SYSTEM_OPTIMIZED",
  "owner" : "hadoop",
  "link" : false
}
bash-4.2$ ozone fs -touch /s3v/bucket1/dir1/key1
bash-4.2$ ozone sh bucket info /s3v/bucket1
{
  "metadata" : { },
  "volumeName" : "s3v",
  "name" : "bucket1",
  "storageType" : "DISK",
  "versioning" : false,
  "usedBytes" : 0,
  "usedNamespace" : 1,
  "creationTime" : "2022-11-22T13:19:47.398Z",
  "modificationTime" : "2022-11-22T13:19:47.398Z",
  "quotaInBytes" : -1,
  "quotaInNamespace" : -1,
  "bucketLayout" : "FILE_SYSTEM_OPTIMIZED",
  "owner" : "hadoop",
  "link" : false
}
bash-4.2$ ozone sh key list /s3v/bucket1
[ {
  "volumeName" : "s3v",
  "bucketName" : "bucket1",
  "name" : "dir1/",
  "dataSize" : 0,
  "creationTime" : "2022-11-22T13:20:29.846Z",
  "modificationTime" : "2022-11-22T13:20:29.846Z",
  "replicationConfig" : {
    "replicationFactor" : "THREE",
    "requiredNodes" : 3,
    "replicationType" : "RATIS"
  },
  "metadata" : { }
}, {
  "volumeName" : "s3v",
  "bucketName" : "bucket1",
  "name" : "dir1/key1",
  "dataSize" : 0,
  "creationTime" : "2022-11-22T13:20:51.596Z",
  "modificationTime" : "2022-11-22T13:20:51.625Z",
  "replicationConfig" : {
    "replicationFactor" : "ONE",
    "requiredNodes" : 1,
    "replicationType" : "RATIS"
  },
  "metadata" : { }
} ]

Another example, would be considering how Recon's namespace disk usage works. When calculating disk usage we only consider the dataSize of the keys, ignoring any directories since they take up no space.

As you can see below, usedNamespace now is 2 because we have 2 files. The space taken up by the bucket, equals to the dataSize of the files while directories are ignored.

bash-4.2$ ozone sh key put /s3v/bucket1/dir2/key2 README.md
bash-4.2$ ozone admin namespace du /s3v/bucket1/dir2/key2
Connecting to Recon: http://recon:9888/api/v1/namespace/du?path=/s3v/bucket1/dir2/key2 ...

Path : /s3v/bucket1/dir2/key2
Total Size : 3 KB

There's no immediate sub-path under this object.
Add -f as an option to visualize files as sub-path, if any.

bash-4.2$ ozone admin namespace du /s3v/bucket1/dir2     
Connecting to Recon: http://recon:9888/api/v1/namespace/du?path=/s3v/bucket1/dir2 ...

Path : /s3v/bucket1/dir2
Total Size : 3 KB
Size of Direct Keys : 3 KB

There's no immediate sub-path under this object.
Add -f as an option to visualize files as sub-path, if any.

bash-4.2$ ozone admin namespace du /s3v/bucket1          
Connecting to Recon: http://recon:9888/api/v1/namespace/du?path=/s3v/bucket1 ...

Path : /s3v/bucket1
Total Size : 3 KB
Size of Direct Keys : 0 bytes

DU
  Size      Path Name
  0 bytes   /s3v/bucket1/dir1/
  3 KB      /s3v/bucket1/dir2/

bash-4.2$ ozone sh bucket info /s3v/bucket1
{
  "metadata" : { },
  "volumeName" : "s3v",
  "name" : "bucket1",
  "storageType" : "DISK",
  "versioning" : false,
  "usedBytes" : 11637,
  "usedNamespace" : 2,
  "creationTime" : "2022-11-22T13:40:33.510Z",
  "modificationTime" : "2022-11-22T13:40:33.510Z",
  "quotaInBytes" : -1,
  "quotaInNamespace" : -1,
  "bucketLayout" : "FILE_SYSTEM_OPTIMIZED",
  "owner" : "hadoop",
  "link" : false
}

With all that said, I don't think we should make these changes.

sadanand48 · 2022-11-22T14:43:19Z

Thanks @xBis7 for your comments.

If we were to accept this change then we would have to modify the whole system.

Currently Ozone directory create doesn’t account to an increment in the usedNamespace field of a bucket although delete directory decrements , the usedNamespace. I had also filed this issue once.
Although when I checked the definition of namespace online in hadoop context I got

In Hadoop we refer to a Namespace as a file or directory which is handled by the Name Node

I agree directories are zero sized keys and so they don't account for usedBytes calculation, but I'm not very sure about the namespace. If we don't consider it, we shouldn't decrement it as part of delete.

cc @ChenSammi @sumitagrawl any thoughts?

xBis7 · 2022-11-22T15:05:41Z

although delete directory decrements , the usedNamespace.

@sadanand48 I wasn't aware of this issue. You are right, we don't have to correlate the namespace with the disk usage.

In Hadoop we refer to a Namespace as a file or directory which is handled by the Name Node

Although Hadoop is a separate project and Name Node probably refers to HDFS, it makes sense to consider directories as part of the namespace. At least Recon does it.

xichen01 · 2022-11-22T15:50:26Z

The initial issue I encountered was finding that the usedNamespace of our bucket was a negative value, and then locating that this was caused by mkdir will not increase usedNamespace but rmdir will decrease it
Maybe I should add this part of the description.

xBis7

@xichen01 I've left some comments inline. I confirmed in docker that it works with ozone fs commands such as mkdir and touch but usedNamespace should also be counting directories created with the ozone sh commands. It counts a key if it is created with ozone sh key put but if the keyname contains a directory that doesn't exist but is to be created then this directory won't be counted.

We might be able to fix this without having a different implementation for each bucket layout, by counting the levels of the keyName before incrementing usedNamespace. For this to work we should check if the directory already exists or needs to be created.

xBis7 · 2022-11-23T15:38:10Z

...anager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMDirectoryCreateResponse.java

-      @Nonnull List<OmKeyInfo> parentKeyInfos, @Nonnull Result result,
-      @Nonnull BucketLayout bucketLayout) {
+      @Nonnull List<OmKeyInfo> parentKeyInfos, @Nonnull OmBucketInfo bucketInfo,
+      @Nonnull Result result, @Nonnull BucketLayout bucketLayout) {


When adding a new parameter to a method, it's best to add it at the end. This will make debugging easier and diffs much cleaner.

Suggested change

@Nonnull Result result, @Nonnull BucketLayout bucketLayout) {

@Nonnull BucketLayout bucketLayout, @Nonnull OmBucketInfo bucketInfo) {

xBis7 · 2022-11-23T15:39:55Z

...anager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMDirectoryCreateResponse.java

    super(omResponse, bucketLayout);
    this.dirKeyInfo = dirKeyInfo;
    this.parentKeyInfos = parentKeyInfos;
+    this.bucketInfo = bucketInfo;


Nit: Since we are changing the order of the parameters and adding bucketInfo at the end, this should also be moved at the end.

xBis7 · 2022-11-23T15:42:26Z

...src/main/java/org/apache/hadoop/ozone/om/response/file/OMDirectoryCreateResponseWithFSO.java

      @Nonnull OmDirectoryInfo dirInfo,
-      @Nonnull List<OmDirectoryInfo> pDirInfos, @Nonnull Result result,
+      @Nonnull List<OmDirectoryInfo> pDirInfos,
+      @Nonnull OmBucketInfo bucketInfo, @Nonnull Result result,


Same here, @Nonnull OmBucketInfo bucketInfo should be the last parameter of the method.

xBis7 · 2022-11-23T15:43:39Z

...src/main/java/org/apache/hadoop/ozone/om/response/file/OMDirectoryCreateResponseWithFSO.java

    super(omResponse, bucketLayout);
    this.dirInfo = dirInfo;
    this.parentDirInfos = pDirInfos;
+    this.bucketInfo = bucketInfo;


This should be below result.

xBis7 · 2022-11-23T16:04:19Z

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

            Optional.of(missingParentInfos), trxnLogIndex);
+        OmBucketInfo omBucketInfo =
+            getBucketInfo(omMetadataManager, volumeName, bucketName);
+        omBucketInfo.incrUsedNamespace(numMissingParents + 1L);


Should we be adding the number of missing keys to the used namespace? I can see that below call to logResult() is updating the metrics with the number.

xBis7 · 2022-11-23T16:06:36Z

...test/java/org/apache/hadoop/ozone/om/response/file/TestOMDirectoryCreateResponseWithFSO.java

    OMDirectoryCreateResponseWithFSO omDirectoryCreateResponseWithFSO =
        new OMDirectoryCreateResponseWithFSO(omResponse, volumeId, bucketId,
-                omDirInfo, new ArrayList<>(),
+                omDirInfo, new ArrayList<>(), omBucketInfo,


Order of parameters

xBis7 · 2022-11-23T16:18:39Z

...ration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java

+  public void testBucketUsedNamespace(BucketLayout layout) throws IOException {
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+    OzoneVolume volume = null;


Nit: This is redundant. We can declare OzoneVolume along with the initialization below.

xBis7 · 2022-11-23T16:18:59Z

...ration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java

+    String value = "sample value";
+    int valueLength = value.getBytes(UTF_8).length;
+    store.createVolume(volumeName);
+    volume = store.getVolume(volumeName);


Suggested change

volume = store.getVolume(volumeName);

OzoneVolume volume = store.getVolume(volumeName);

xBis7 · 2022-11-23T16:25:46Z

...ration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java

+    client.createDirectory(volumeName, bucketName, directoryName);
+    Assert.assertEquals(1L,
+        store.getVolume(volumeName).getBucket(bucketName).getUsedNamespace());
+


It would be best to create multiple keys and directories and check in every step that we have the expected value in usedNamespace. Also we should check for the case where we are providing the directory directly with the key.

For example, if we create dir1/key1 and dir1/key2 without creating dir1 in advance, then unless we have an OBS bucket, the system will create dir1 and then two keys under it. In this case, usedNamespace should be 3.

xBis7 · 2022-11-23T16:30:39Z

...er/src/test/java/org/apache/hadoop/ozone/om/response/file/TestOMDirectoryCreateResponse.java

    OMDirectoryCreateResponse omDirectoryCreateResponse =
        new OMDirectoryCreateResponse(omResponse, omKeyInfo,
-            new ArrayList<>(), Result.SUCCESS, getBucketLayout());
+            new ArrayList<>(), omBucketInfo, Result.SUCCESS, getBucketLayout());


Order of parameters.

xichen01 · 2022-11-24T18:01:53Z

@xBis7
If execute ozone sh key put ${keyname} but if the ${keyname} contains a directory that doesn't exist.
For FSO bucket, these "missing paths will be created automatically, I think these automatically created directories should increase Usednamespace.

[root@linux ~/root/ozone]$ ozone sh bucket create s3v/bucket4 -l FILE_SYSTEM_OPTIMIZED
[root@linux ~/root/ozone]$ ozone sh key put /s3v/bucket4/dir1/dir2/dir3/dir4/file119 ~/1M.img
[root@linux ~/root/ozone]$ ozone sh key ls s3v/bucket4 | grep name
  "name" : "dir1/",
  "name" : "dir1/dir2/",
  "name" : "dir1/dir2/dir3/",
  "name" : "dir1/dir2/dir3/dir4/",
  "name" : "dir1/dir2/dir3/dir4/file119",

But for the Object bucket, these "missing paths" will not be created, they are not real, and should not increase Usednamespace

Do you have any suggestions?

xBis7 · 2022-11-24T18:36:11Z

For FSO bucket, these "missing paths will be created automatically, I think these automatically created directories should increase Usednamespace.

@xichen01 From what I saw, usedNamespace value doesn't increment in this case.

But for the Object bucket, these "missing paths" will not be created, they are not real, and should not increase Usednamespace

I agree.

Do you have any suggestions?

You could get the BucketInfo and from that, check the BucketLayout. A LEGACY bucket with ozone.om.enable.filesystem.paths set to false, acts exactly like an OBS bucket, so in addition to the layout you should also check configs.

We should count directories only for FSO buckets and LEGACY buckets with ozone.om.enable.filesystem.paths set to true. @sadanand48 any thoughts on this?

sadanand48 · 2022-11-24T18:52:08Z

We should count directories only for FSO buckets and LEGACY buckets with ozone.om.enable.filesystem.paths set to true.

Yes agree @xBis7 , we should increment only if missing directories are created as part of key creates which is not the case for OBS.
@xichen01 I think you can look into OMDirectoryCreateRequest#getAllParentInfo and I guess the namespace increment should be done here as this is the place when the missing dirs get created.

sadanand48

Thanks @xichen01 for updating the PR, the changes look good to me.

xBis7 · 2022-12-01T15:55:15Z

Thanks @xichen01 for the changes and @sadanand48 for the review. LGTM! I tested it locally and everything works as expected.

)

* master: (110 commits) HDDS-7472. EC: Fix NSSummaryEndpoint#getDiskUsage for EC keys (apache#3987) HDDS-5704. Ozone URI syntax description in help content needs to mention about ozone service id (apache#3862) HDDS-7555. Upgrade Ratis to 2.4.2-8b8bdda-SNAPSHOT. (apache#4028) HDDS-7541. FSO recursive delete directory with hierarchy takes much time for cleanup (apache#4008) HDDS-7581. Fix update-jar-report for snapshot (apache#4034) HDDS-7253. Fix exception when '/' in key name (apache#4038) HDDS-7579. Use Netty 4.1.77 for consistency (apache#4031) HDDS-7562. Suppress warning about long filenames in tar (apache#4017) HDDS-7563. Add a handler for under replicated Ratis containers in RM (apache#4025) HDDS-7497. Fix mkdir does not update bucket's usedNamespace (apache#3969) HDDS-7567. Invalid entries in LICENSE (apache#4020) HDDS-7575. Correct showing of RATIS-THREE icon in Recon UI (apache#4026) HDDS-7540. Let reusable workflow inherit secrets (apache#4012) HDDS-7568. Bump copyright year in NOTICE (apache#4018) HDDS-7394. OM RPC FairCallQueue decay decision metrics list caller username in the metric (apache#3878) HDDS-7510. Recon: Return number of open containers in `/clusterState` endpoint (apache#3989) HDDS-7561. Improve setquota, clrquota CLI usage (apache#4016) HDDS-6615. EC: Improve write performance by pipelining encode and flush (apache#3994) HDDS-7554. Recon UI should show DORMANT in pipeline status filter (apache#4010) HDDS-7540. Separate scheduled CI from push/PR workflows (apache#4004) ...

HDDS-7497. Fix mkdir does not update bucket's usedNamespace

6dd0ba2

sadanand48 requested changes Nov 16, 2022

View reviewed changes

xichen01 added 4 commits November 17, 2022 20:11

Add persist omBucketInfo

de4256e

Add comments

6410dc8

Fix findbugs

957cdc1

Passing omBucketInfo by copy instead of directly

4368156

xBis7 reviewed Nov 23, 2022

View reviewed changes

xichen01 added 2 commits November 27, 2022 18:52

move new paramete to end

3a060ef

Increase usedNamespace for automatically created dir when put a key

4c3d099

sadanand48 reviewed Dec 1, 2022

View reviewed changes

sadanand48 requested a review from xBis7 December 1, 2022 10:07

xBis7 approved these changes Dec 1, 2022

View reviewed changes

sadanand48 approved these changes Dec 2, 2022

View reviewed changes

sadanand48 merged commit 50a9d46 into apache:master Dec 2, 2022

Galsza pushed a commit to Galsza/ozone that referenced this pull request Dec 7, 2022

HDDS-7497. Fix mkdir does not update bucket's usedNamespace (apache#3969

1c5616d

)

	@Nonnull Result result, @Nonnull BucketLayout bucketLayout) {
	@Nonnull BucketLayout bucketLayout, @Nonnull OmBucketInfo bucketInfo) {

	volume = store.getVolume(volumeName);
	OzoneVolume volume = store.getVolume(volumeName);

HDDS-7497. Fix mkdir does not update bucket's usedNamespace #3969

HDDS-7497. Fix mkdir does not update bucket's usedNamespace #3969

Uh oh!

Conversation

xichen01 commented Nov 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

sadanand48 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kerneltime commented Nov 21, 2022

Uh oh!

kerneltime commented Nov 21, 2022

Uh oh!

xBis7 commented Nov 22, 2022

Uh oh!

sadanand48 commented Nov 22, 2022

Uh oh!

xBis7 commented Nov 22, 2022

Uh oh!

xichen01 commented Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xBis7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xichen01 commented Nov 24, 2022

Uh oh!

xBis7 commented Nov 24, 2022

Uh oh!

sadanand48 commented Nov 24, 2022

Uh oh!

sadanand48 left a comment

Choose a reason for hiding this comment

Uh oh!

xBis7 commented Dec 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xichen01 commented Nov 16, 2022 •

edited

Loading

xichen01 commented Nov 22, 2022 •

edited

Loading