curvefs/client: now we support hadoop sdk #2807

Wine93 · 2023-10-16T09:12:30Z

No description provided.

Wine93 · 2023-10-16T12:05:02Z

cicheck

Wine93 · 2023-10-17T07:05:43Z

cicheck

SeanHai · 2023-10-17T11:54:55Z

curvefs/src/client/fuse_client.cpp

@@ -1086,6 +1098,11 @@ CURVEFS_ERROR FuseClient::FuseOpSetXattr(fuse_req_t req, fuse_ino_t ino,
    VLOG(1) << "FuseOpSetXattr ino: " << ino << ", name: " << name
            << ", size = " << size
            << ", strvalue: " << strvalue;
+
+    if (option_.fileSystemOption.disableXAttr && !IsSpecialXAttr(name)) {


The speciacl xattrs should not allowed to set by general user.

YunhuiChen · 2023-10-18T02:45:51Z

cicheck

YunhuiChen · 2023-10-18T02:46:22Z

cicheck

YunhuiChen · 2023-10-18T02:50:43Z

cicheck

YunhuiChen · 2023-10-18T02:52:29Z

cicheck

YunhuiChen · 2023-10-18T02:57:33Z

cicheck

YunhuiChen · 2023-10-18T03:07:35Z

cicheck

YunhuiChen · 2023-10-18T09:26:25Z

cicheck

YunhuiChen · 2023-10-19T01:45:46Z

cicheck

SeanHai · 2023-10-18T03:29:15Z

curvefs/sdk/java/src/main/java/io/opencurve/curve/fs/hadoop/CurveFSConfigKeys.java

+
+import org.apache.hadoop.fs.CommonConfigurationKeys;
+
+public class CurveFSConfigKeys extends CommonConfigurationKeys {}


where the class CurveFSConfigKeys used?

where the class CurveFSConfigKeys used?

Fixed, i had delete it.

SeanHai · 2023-10-18T03:36:26Z

curvefs/sdk/java/src/main/java/io/opencurve/curve/fs/hadoop/CurveFSInputStream.java

+      fileHandle = fh;
+      closed = false;
+      curve = curvefs;
+      buffer = new byte[1<<21];


How to determine buffer size?

How to determine buffer size?

It's a const tradeoff :)

SeanHai · 2023-10-19T01:52:58Z

curvefs/src/client/fuse_client.cpp

-        *realSize += strlen(XATTRRSUBDIRS) + 1;
-        *realSize += strlen(XATTRRENTRIES) + 1;
-        *realSize += strlen(XATTRRFBYTES) + 1;
+        *realSize += strlen(XATTR_DIR_RFILES) + 1;


listxattr also need check option_.fileSystemOption.disableXAttr

SeanHai · 2023-10-19T02:15:09Z

curvefs/src/client/vfs/operations.cpp

+                                    const std::string& name,
+                                    uint16_t mode,
+                                    EntryOut* entryOut) {
+    auto ctx = NewFuseContext();


Is it possible to use FuseContext as a variable instead of creating a new one every time?

Is it possible to use FuseContext as a variable instead of creating a new one every time?

No, because we should get the latest uid and gid which can modified on fly.

Wine93 · 2023-10-19T02:41:37Z

cicheck

Wine93 · 2023-10-19T07:13:26Z

cicheck

Wine93 · 2023-10-19T07:37:14Z

cicheck

Wine93 · 2023-10-19T08:02:35Z

cicheck

Wine93 · 2023-10-19T08:19:44Z

cicheck

Wine93 · 2023-10-19T08:23:10Z

cicheck

h0hmj · 2023-10-20T08:32:35Z

it's too hard to review such a huge pr with 4 commit. over 100,000 lines changed.
need more clear statement about what changed (add/delete) to guide review.

curvefs/sdk/java/native/BUILD

util/playground.sh

curvefs/src/client/fuse_client.cpp

curvefs/src/client/sdk_helper.cpp

h0hmj · 2023-10-20T09:56:15Z

curvefs/src/client/vfs/cache.cpp

+    bool yes = lru_->Get(key, &value);
+    if (!yes) {
+        return false;
+    } else if (value.expire < Now()) {


expired item should be removed from lru cache.
seems no periodical task to clean up expired items in lru cache also.

expired item should be removed from lru cache. seems no periodical task to clean up expired items in lru cache also.

Add todo for this.

h0hmj · 2023-10-23T07:27:22Z

curvefs/src/client/vfs/utils.cpp

+    return s;
+}
+
+bool HasPrefix(const std::string& str, const std::string& prefix) {


#include "absl/strings/match.h"
absl::StartsWith()

#include "absl/strings/match.h" absl::StartsWith()

Fixed.

h0hmj · 2023-10-23T07:28:45Z

curvefs/src/client/vfs/utils.cpp

+    return str.rfind(prefix, 0) == 0;
+}
+
+std::vector<std::string> Split(const std::string& str,


why wrap it?

why wrap it?

In order to:

Form a unified namespace, like you can find all string related function in strings namespace.

One header include all util functions: you can find all functions once you include the utils.h header file and if you use absl library, you should find the function in which header and include it.

It is more easy to use.

h0hmj · 2023-10-23T07:35:03Z

curvefs/src/client/vfs/utils.cpp

+
+}  // namespace strings
+
+namespace filepath {


use butil::FilePath?
seems many utils can reuse util in butil or absl

use butil::FilePath? seems many utils can reuse util in butil or absl

ditto

h0hmj · 2023-10-23T07:46:54Z

curvefs/src/client/vfs/vfs.cpp

+    return DoMkDir(path, mode);
+}
+
+CURVEFS_ERROR VFS::MkDirs(const std::string& path, uint16_t mode) {


logic is wrong, cannot handle case below

touch a
mkdir -p a

expect return: File Exist
actual return: OK

logic is wrong, cannot handle case below

touch a mkdir -p a

expect return: File Exist actual return: OK

Fixed.

h0hmj · 2023-10-23T08:00:06Z

curvefs/src/client/vfs/vfs.cpp

+    return CURVEFS_ERROR::OK;
+}
+
+CURVEFS_ERROR VFS::Lookup(const std::string& path,


careful about stack overflow

careful about stack overflow

Yep.

Wine93 · 2023-10-23T08:41:43Z

it's too hard to review such a huge pr with 4 commit. over 100,000 lines changed. need more clear statement about what changed (add/delete) to guide review.

Agree!

Wine93 · 2023-10-23T11:26:42Z

cicheck

Signed-off-by: Wine93 <wine93.info@gmail.com>

Wine93 · 2023-10-24T06:52:20Z

cicheck

Signed-off-by: Wine93 <wine93.info@gmail.com>

Wine93 · 2023-10-24T10:50:56Z

cicheck

Wine93 · 2023-10-24T10:52:11Z

cicheck

Wine93 · 2023-10-24T10:53:39Z

cicheck

Wine93 · 2023-10-24T10:55:06Z

cicheck

Wine93 · 2023-10-24T11:35:36Z

cicheck

Wine93 changed the title ~~Sdk/hadoop~~ curvefs/client: now we support hadoop sdk Oct 16, 2023

Wine93 force-pushed the sdk/hadoop branch from 4aa33ae to 525d297 Compare October 16, 2023 11:37

Wine93 force-pushed the sdk/hadoop branch 4 times, most recently from e3b73c8 to f7f55e2 Compare October 17, 2023 02:18

Wine93 force-pushed the sdk/hadoop branch 2 times, most recently from 60c01b6 to 13ba40f Compare October 17, 2023 08:12

SeanHai reviewed Oct 18, 2023

View reviewed changes

YunhuiChen closed this Oct 18, 2023

YunhuiChen reopened this Oct 18, 2023

YunhuiChen closed this Oct 18, 2023

YunhuiChen reopened this Oct 18, 2023

Wine93 force-pushed the sdk/hadoop branch 2 times, most recently from b31fb56 to 1f8b2f6 Compare October 18, 2023 12:38

SeanHai reviewed Oct 19, 2023

View reviewed changes

Wine93 force-pushed the sdk/hadoop branch from 1f8b2f6 to c32bcfc Compare October 19, 2023 02:36

Wine93 closed this Oct 19, 2023

Wine93 force-pushed the sdk/hadoop branch from 388ce22 to b5a0cc5 Compare October 19, 2023 07:57

h0hmj reviewed Oct 23, 2023

View reviewed changes

Wine93 force-pushed the sdk/hadoop branch 2 times, most recently from 8aee77a to 8ffe333 Compare October 23, 2023 11:24

SeanHai approved these changes Oct 23, 2023

View reviewed changes

Wine93 closed this Oct 24, 2023

Wine93 reopened this Oct 24, 2023

Wine93 added 3 commits October 24, 2023 14:40

curvefs/client: fix modify access logging switch on the fly not works.

ecc63f3

Signed-off-by: Wine93 <wine93.info@gmail.com>

curvefs/client: fix memory usage is so high.

877d0d5

Signed-off-by: Wine93 <wine93.info@gmail.com>

curvefs/client: let curve specified xattr works even if disable xattr.

f2d4060

Signed-off-by: Wine93 <wine93.info@gmail.com>

Wine93 force-pushed the sdk/hadoop branch from 8ffe333 to a4f96c6 Compare October 24, 2023 06:43

h0hmj approved these changes Oct 24, 2023

View reviewed changes

curvefs/client: now we support hadoop sdk.

446a60f

Signed-off-by: Wine93 <wine93.info@gmail.com>

Wine93 force-pushed the sdk/hadoop branch from a4f96c6 to 446a60f Compare October 24, 2023 10:48

Wine93 merged commit d616bd3 into opencurve:master Oct 24, 2023
5 checks passed

SeanHai mentioned this pull request Nov 30, 2023

curvefs: fix list xattr miss someone #2937

Merged

2 tasks


		import org.apache.hadoop.fs.CommonConfigurationKeys;

		public class CurveFSConfigKeys extends CommonConfigurationKeys {}

curvefs/client: now we support hadoop sdk #2807

curvefs/client: now we support hadoop sdk #2807

Conversation

Wine93 commented Oct 16, 2023 • edited

Wine93 commented Oct 16, 2023

Wine93 commented Oct 17, 2023

Choose a reason for hiding this comment

YunhuiChen commented Oct 18, 2023

YunhuiChen commented Oct 18, 2023

YunhuiChen commented Oct 18, 2023

YunhuiChen commented Oct 18, 2023

YunhuiChen commented Oct 18, 2023

YunhuiChen commented Oct 18, 2023

YunhuiChen commented Oct 18, 2023

YunhuiChen commented Oct 19, 2023

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

Wine93 commented Oct 19, 2023

Wine93 commented Oct 19, 2023

Wine93 commented Oct 19, 2023

Wine93 commented Oct 19, 2023

Wine93 commented Oct 19, 2023

Wine93 commented Oct 19, 2023

h0hmj commented Oct 20, 2023

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

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

Wine93 commented Oct 23, 2023

Wine93 commented Oct 23, 2023

Wine93 commented Oct 24, 2023

Wine93 commented Oct 24, 2023

Wine93 commented Oct 24, 2023

Wine93 commented Oct 24, 2023

Wine93 commented Oct 24, 2023

Wine93 commented Oct 24, 2023

Wine93 commented Oct 16, 2023 •

edited