Support set/get xattr #18053

Jackson-Wang-7 · 2023-08-23T06:50:40Z

What changes are proposed in this pull request?

Supports setXattr API of Alluxio FileSystem, it will set the corresponding attributes to UFS. If UFS is a filesystem, it's set through the setXattr interface of UFS. if UFS is object storage, it's set through setTagging API.

Why are the changes needed?

Based on the 3.x architecture, recover support for the setXattr interface. There are still many places where the interface is used, and it should not be supported at present.

� Conflicts: � dora/core/server/proxy/src/main/java/alluxio/proxy/s3/S3BucketTask.java � dora/core/server/proxy/src/main/java/alluxio/proxy/s3/S3RestUtils.java � dora/core/server/worker/src/main/java/alluxio/worker/dora/PagedDoraWorker.java � dora/underfs/web/src/main/java/alluxio/underfs/web/WebUnderFileSystem.java

elega · 2023-09-18T12:14:45Z

dora/core/common/src/main/java/alluxio/util/io/FileUtils.java

+                                                         byte[] value) {
+    try {
+      UserDefinedFileAttributeView attributeView =
+          Files.getFileAttributeView(Paths.get(filePath), UserDefinedFileAttributeView.class);


can you add a comment to explain what is an attribute view what is this for?

is the attr only set locally and will lose after a worker restarts?

is the attr only set locally and will lose after a worker restarts?

No, it sets the attribute to the local file system, so it will not change after a worker restarts. (works in CentOS, not work in MacOS)

+1 to add an example in javadoc.

elega · 2023-09-18T12:15:26Z

dora/core/server/worker/src/main/java/alluxio/worker/dora/PagedDoraWorker.java

   * @return a FileInfo
   */
-  public alluxio.grpc.FileInfo buildFileInfoFromUfsStatus(UfsStatus status, String ufsFullPath) {
+  public alluxio.grpc.FileInfo buildFileInfoFromUfsStatus(UfsStatus status, String ufsFullPath,


nit: add a nullable annotation here

jiacheliu3

Thanks a lot for the work! I left some comments PTAL

dora/core/common/src/main/java/alluxio/underfs/UnderFileSystem.java

dora/core/common/src/main/java/alluxio/underfs/UnderFileSystemWithLogging.java

dora/core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java

jiacheliu3 · 2023-09-22T03:20:46Z

dora/core/common/src/main/java/alluxio/util/io/FileUtils.java

+                                                         byte[] value) {
+    try {
+      UserDefinedFileAttributeView attributeView =
+          Files.getFileAttributeView(Paths.get(filePath), UserDefinedFileAttributeView.class);


+1 to add an example in javadoc.

dora/core/common/src/main/java/alluxio/util/io/FileUtils.java

dora/underfs/obs/src/main/java/alluxio/underfs/obs/OBSUnderFileSystem.java

dora/underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java

dora/underfs/s3a/src/main/java/alluxio/underfs/s3a/S3AUnderFileSystem.java

dora/core/common/src/main/java/alluxio/underfs/UnderFileSystem.java

Jackson-Wang-7 · 2023-09-20T06:33:14Z

dora/core/server/proxy/src/main/java/alluxio/proxy/s3/S3BucketTask.java

@@ -214,7 +214,12 @@ public Response continueTask() {
          try {
            List<URIStatus> children = mHandler.getMetaFS().listStatus(new AlluxioURI(
                    S3RestUtils.MULTIPART_UPLOADS_METADATA_DIR));
-            return ListMultipartUploadsResult.buildFromStatuses(bucket, children);
+            List<URIStatus> mpuList = new ArrayList<>();


add an annotation to explain why we need to get file status again.

dora/core/common/src/main/java/alluxio/underfs/UnderFileSystem.java

dora/core/common/src/main/java/alluxio/underfs/UnderFileSystemWithLogging.java

dora/core/common/src/main/java/alluxio/underfs/ObjectUnderFileSystem.java

dora/core/common/src/main/java/alluxio/util/io/FileUtils.java

Jackson-Wang-7 · 2023-09-25T03:07:07Z

dora/core/server/proxy/src/main/java/alluxio/proxy/s3/S3BucketTask.java

+            // GetListing doesn't contain the xattr info, so get status for MPU files.
+            List<URIStatus> mpuList = new ArrayList<>();
+            for (URIStatus status : children) {
+              URIStatus filemeta = mHandler.getMetaFS().getStatus(new AlluxioURI(status.getPath()));


yes, when we use the MPU APIs of the north bound s3 api

Jackson-Wang-7 · 2023-09-25T03:07:48Z

dora/core/server/worker/src/main/java/alluxio/worker/dora/PagedDoraWorker.java

@@ -457,6 +461,11 @@ public alluxio.grpc.FileInfo buildFileInfoFromUfsStatus(UfsStatus status, String
        .setGroup(status.getGroup())
        .setCompleted(true)
        .setPersisted(true);
+    if (xattrMap != null) {
+      for (Map.Entry<String, String> entry : xattrMap.entrySet()) {
+        infoBuilder.putXattr(entry.getKey(), ByteString.copyFromUtf8(entry.getValue()));


Jackson-Wang-7 · 2023-09-25T03:08:41Z

dora/core/server/worker/src/main/java/alluxio/worker/dora/PagedFileReader.java

@@ -220,7 +220,6 @@ public synchronized void close() throws IOException {
    }
    mClosed = true;
    mPositionReader.close();
-    mUfs.close();


a bugfix that is already in main branch, cherrypick it for testing. will remove it.

Jackson-Wang-7 · 2023-09-25T03:11:52Z

dora/core/server/worker/src/main/java/alluxio/worker/grpc/DoraWorkerClientServiceHandler.java

@@ -232,8 +232,9 @@ public void listStatus(ListStatusPRequest request,
        UfsStatus status = statuses[i];
        String ufsFullPath = PathUtils.concatPath(request.getPath(), status.getName());

+        // the list statues do not include xattr now.


Jackson-Wang-7 · 2023-09-25T03:20:07Z

dora/underfs/cos/src/main/java/alluxio/underfs/cos/COSUnderFileSystem.java

+      Tag tag = new Tag(name, value);
+      tagList.add(tag);
+    }
+    mClient.setObjectTagging(


add a comment like this to mention the case:

// It's a read-and-update race condition. When there is a competive conflict scenario, // it may lead to inconsistent final results. The final conflict occurs in UFS, // UFS will determine the final result.

jiacheliu3

Mostly LGTM with some relatively minor comments. Good to go when you see fit, and I'm stamping ahead of time. Thanks a lot for the work!

We might be able to add more UT cases, but you can do that in follow up PRs (and not necessarily but you alone)

jiacheliu3 · 2023-09-26T06:28:48Z

dora/core/common/src/main/java/alluxio/underfs/UnderFileSystemWithLogging.java

@@ -1172,17 +1173,17 @@ public String methodName() {

      @Override
      public String toString() {
-        return String.format("path=%s, name=%s, value=%s", path, name, value);
+        return String.format("path=%s, name=%s, value=%s", path, name, Arrays.toString(value));


nit: we have helpers you can use, it's fine as-is here but you have more options next time

return MoreObjects.toStringHelper(this).add(xxx,xxx).toString();

dora/core/common/src/main/java/alluxio/util/io/FileUtils.java

jiacheliu3 · 2023-09-26T06:29:41Z

dora/core/common/src/main/java/alluxio/util/io/FileUtils.java

+      Map<String, String> attrMap = new HashMap<>();
+      for (String attributeName : attributeView.list()) {
+        int attributeSize = attributeView.size(attributeName);
+        ByteBuffer attributeBuffer = ByteBuffer.allocate(attributeSize);


sanity check ^

dora/underfs/hdfs/src/main/java/alluxio/underfs/hdfs/HdfsUnderFileSystem.java

jiacheliu3 · 2023-09-26T06:32:37Z

...nderfs/hdfs/src/test/java/alluxio/underfs/hdfs/hdfs3/HdfsUnderFileSystemIntegrationTest.java

+    Map<String, String> attrMap =  mUfs.getAttributes(testFilePath);
+    assertEquals(attrMap.size(), 1);
+    assertEquals(attrMap.get(attrKey), attrValue2);
+    mUfs.deleteFile(testFilePath);


delete at the end of the test is not guaranted to happen. Could you move that to @After?

I will move it to the finally part. I'm not sure if deletion is necessary. I haven't seen it in other test cases, I just follow my personal habits.

yyongycy · 2023-10-10T02:09:09Z

What is the max size of tagging?

Jackson-Wang-7 · 2023-10-10T02:21:51Z

What is the max size of tagging?

we have the configuration property to limit the tag number we can set, the max size is 10.

Jackson-Wang-7 · 2023-10-10T03:18:39Z

alluxio-bot, merge this please

alluxio-bot · 2023-10-10T03:18:41Z

merge failed:
Merge refused because pull request does not have label start with type-

Jackson-Wang-7 · 2023-10-10T03:18:58Z

alluxio-bot, merge this please

### What changes are proposed in this pull request? Supports setXattr API of Alluxio FileSystem, it will set the corresponding attributes to UFS. If UFS is a filesystem, it's set through the setXattr interface of UFS. if UFS is object storage, it's set through setTagging API. ### Why are the changes needed? Based on the 3.x architecture, recover support for the setXattr interface. There are still many places where the interface is used, and it should not be supported at present. pr-link: Alluxio#18053 change-id: cid-4f8c96c88cced4374d54b6387bfc618b47d2a423

Jackson-Wang-7 added 14 commits August 3, 2023 14:23

support setXattr in Dora

9a5fc54

support setXattr in Dora

b7609f3

Merge branch 'main' of github.com:Alluxio/alluxio into support-set-xattr

c507dad

support setXattr in Dora

b0a9483

support setXattr in Dora

339a1a6

fix the list with xattr

b716625

fix the list with xattr

6dcb2e4

fix the unsupported case in local ufs

63a7963

fix checkstyle

a8ae9dc

Merge branch 'main' of github.com:Alluxio/alluxio into support-set-xattr

141fa22

fix directory tagging

c5bcd14

Merge branch 'main' of github.com:Alluxio/alluxio into support-set-xattr

19d55f9

fix hdfs set/get Xattr

1cf4826

Xenorith force-pushed the main branch from dfa74af to 5fb603d Compare September 11, 2023 01:34

merge the main branch

b6f669a

Jackson-Wang-7 requested review from elega and huanghua78 September 18, 2023 09:31

elega reviewed Sep 18, 2023

View reviewed changes

Jackson-Wang-7 added 5 commits September 20, 2023 11:04

add the Nullable annotation

b8aadfc

add the Nullable annotation

91f35a9

fix the bug of adding multiple tags to OBS

16881bb

fix the bug of adding multiple tags to OBS

6927b30

Fix the bug of using a terminated executor in S3 UFS

ca1becb

jiacheliu3 requested changes Sep 22, 2023

View reviewed changes

Jackson-Wang-7 added 2 commits September 25, 2023 11:43

fix some review comments

c1f9ee0

add some basic hdfs ufs testcases

8951f79

Jackson-Wang-7 commented Sep 25, 2023

View reviewed changes

jiacheliu3 approved these changes Sep 26, 2023

View reviewed changes

Jackson-Wang-7 added 3 commits September 28, 2023 11:27

fix some code review comments

cceadb7

fix checkstyle

da401b3

Merge branch 'main' of github.com:Alluxio/alluxio into support-set-xattr

edabb38

Jackson-Wang-7 added the type-feature This issue is a feature request label Oct 10, 2023

alluxio-bot merged commit bf997fe into Alluxio:main Oct 10, 2023
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support set/get xattr #18053

Support set/get xattr #18053

Jackson-Wang-7 commented Aug 23, 2023 •

edited

Loading

elega Sep 18, 2023

elega Sep 18, 2023

Jackson-Wang-7 Sep 19, 2023 •

edited

Loading

jiacheliu3 Sep 22, 2023

elega Sep 18, 2023

jiacheliu3 left a comment

jiacheliu3 Sep 22, 2023

Jackson-Wang-7 Sep 20, 2023

Jackson-Wang-7 Sep 25, 2023

Jackson-Wang-7 Sep 25, 2023

Jackson-Wang-7 Sep 25, 2023

Jackson-Wang-7 Sep 25, 2023

Jackson-Wang-7 Sep 25, 2023

jiacheliu3 left a comment •

edited

Loading

jiacheliu3 Sep 26, 2023

jiacheliu3 Sep 26, 2023

jiacheliu3 Sep 26, 2023

Jackson-Wang-7 Sep 28, 2023

yyongycy commented Oct 10, 2023

Jackson-Wang-7 commented Oct 10, 2023

Jackson-Wang-7 commented Oct 10, 2023

alluxio-bot commented Oct 10, 2023

Jackson-Wang-7 commented Oct 10, 2023

Support set/get xattr #18053

Support set/get xattr #18053

Conversation

Jackson-Wang-7 commented Aug 23, 2023 • edited Loading

What changes are proposed in this pull request?

Why are the changes needed?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jackson-Wang-7 Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiacheliu3 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiacheliu3 left a comment • 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

yyongycy commented Oct 10, 2023

Jackson-Wang-7 commented Oct 10, 2023

Jackson-Wang-7 commented Oct 10, 2023

alluxio-bot commented Oct 10, 2023

Jackson-Wang-7 commented Oct 10, 2023

Jackson-Wang-7 commented Aug 23, 2023 •

edited

Loading

Jackson-Wang-7 Sep 19, 2023 •

edited

Loading

jiacheliu3 left a comment •

edited

Loading