curvefs/client: improve metadata performance. #2387

Wine93 · 2023-04-11T03:21:01Z

No description provided.

Wine93 · 2023-04-25T09:46:46Z

cicheck

SeanHai · 2023-04-27T05:44:13Z

Makefile

+playground:
+	@bash util/playground.sh
+
+check:


what does 'make check' do?

what does 'make check' do?

It does what cpplint does, see util.check.sh.

SeanHai · 2023-04-27T05:45:41Z

curvefs/conf/client.conf

+fs.maxNameLength=255
+fs.disableXattr=true
+fs.accessLogging=true
+fs.kernelCache.attrTimeout=3600


Add unit to timeout conf item

Add unit to timeout conf item

fixed.

Is the latest code not pushed up?
fs.kernelCache.attrTimeoutSec or fs.kernelCache.attrTimeoutMs?

Is the latest code not pushed up? fs.kernelCache.attrTimeoutSec or fs.kernelCache.attrTimeoutMs?

fs.kernelCache.attrTimeoutSec.

SeanHai · 2023-04-27T05:46:48Z

curvefs/conf/client.conf

+fs.kernelCache.entryTimeout=3600
+fs.kernelCache.dirEntryTimeout=3600
+fs.lookupCache.negativeTimeout=0
+fs.lookupCache.lruSize=100000


The unit if **Size is byte?

The unit if **Size is byte?

The number of directory entries.

SeanHai · 2023-04-27T05:52:05Z

curvefs/src/client/client_operator.cpp

@@ -418,8 +419,6 @@ CURVEFS_ERROR RenameOperator::UpdateInodeCtime() {
 }

 void RenameOperator::UpdateCache() {
-    dentryManager_->DeleteCache(parentId_, name_);


Why is this step unnecessary?

Why is this step unnecessary?

yep, because:

it works well if you only wants to show files under directory, like ls -l does.

we control all cache consistent by cache timeout, you can adjust cache timeout for some specified cases if the attribute is sensitive.

SeanHai · 2023-04-27T05:56:11Z

curvefs/src/client/common/config.h

+
+// { filesystem option
+struct KernelCacheOption {
+    uint32_t entryTimeout;


Add unit

fixed.

SeanHai · 2023-04-27T06:13:38Z

curvefs/src/client/curve_fuse_op.cpp

+                         ino, name, size, flags, StrErr(rc));
+    });
+
+    if (IsSpecialReq(name)) {


Maybe 'IsWarmupReq' is better, some other special requests may support later.

Maybe 'IsWarmupReq' is better, some other special requests may support later.

fixed.

SeanHai · 2023-04-27T06:20:40Z

curvefs/src/client/dentry_cache_manager.h

@@ -130,8 +102,6 @@ class DentryCacheManagerImpl : public DentryCacheManager {

 private:
    std::shared_ptr<MetaServerClient> metaClient_;
-    // key is parentId + name
-    std::shared_ptr<TimedLRUCache<std::string, Dentry>> dCache_;


the dCache is needn't? where the dentry will be cached?

the dCache is needn't? where the dentry will be cached?

All dir entries will be cached in kernel.

SeanHai · 2023-04-27T06:56:15Z

curvefs/src/client/filesystem/dir_cache.cpp

+    lru_->Put(parent, entries);
+    nentries_ += entries->Size();
+
+    LOG(INFO) << "Insert directory cache: parent = " << parent


log level is suitable？

log level is suitable？

Maybe VLOG(1) is better.

ilixiaocui · 2023-05-08T02:27:03Z

.obm.cfg

@@ -0,0 +1,2 @@
+container_name: curve-build-playground-master
+container_image: opencurvedocker/curve-base:build-debian9


What is this new file for?

What is this new file for?

The manifest for build.

ilixiaocui · 2023-05-08T02:43:05Z

curvefs/conf/client.conf

+fs.cto=true
+fs.maxNameLength=255
+fs.disableXattr=true
+fs.accessLogging=true


ilixiaocui · 2023-05-08T02:44:59Z

curvefs/conf/client.conf

+fs.maxNameLength=255
+fs.disableXattr=true
+fs.accessLogging=true
+fs.kernelCache.attrTimeout=3600


Is the latest code not pushed up?
fs.kernelCache.attrTimeoutSec or fs.kernelCache.attrTimeoutMs?

ilixiaocui · 2023-05-08T02:50:34Z

curvefs/src/client/common/config.cpp

+                               &o->dirAttrTimeout);
+        c->GetValueFatalIfFail("fs.kernelCache.entryTimeout",
+                               &o->entryTimeout);
+        c->GetValueFatalIfFail("fs.kernelCache.dirEntryTimeout",


if fs.kernelCache.entryTimeout=true, fs.kernelCache.dirEntryTimeout=false, dir entry will not cache, but file entry will be cached?

if fs.kernelCache.entryTimeout=false, fs.kernelCache.dirEntryTimeout=false, only dir entry cached?

what's the diffrence between fs.kernelCache.dirEntryTimeout and fs.kernelCache.entryTimeout?

if fs.kernelCache.entryTimeout=true, fs.kernelCache.dirEntryTimeout=false, dir entry will not cache, but file entry will be cached?

Yep.

if fs.kernelCache.entryTimeout=false, fs.kernelCache.dirEntryTimeout=false, only dir entry cached?

All entries will not cached.

what's the diffrence between fs.kernelCache.dirEntryTimeout and fs.kernelCache.entryTimeout?

This designed for assgin different timeout for different types of entris. If the entry is file, we will reply to kernel with timeout fs.kernelCache.entryTimeout, otherwise reply with fs.kernelCache.dirEntryTimeout.

void FileSystem::SetEntryTimeout(EntryOut* entryOut) { auto option = option_.kernelCacheOption; if (IsDir(entryOut->attr)) { entryOut->entryTimeout = option.dirEntryTimeout; entryOut->attrTimeout = option.dirAttrTimeout; } else { entryOut->entryTimeout = option.entryTimeout; entryOut->attrTimeout = option.attrTimeout; } }

ilixiaocui · 2023-05-08T02:55:20Z

WORKSPACE

+# spdlog
+http_archive(
+  name = "spdlog",
+  urls = ["https://github.com/gabime/spdlog/archive/refs/tags/v1.11.0.tar.gz"],


Why a new logging component needs to be introduced？

Why a new logging component needs to be introduced？

Why we need spdlog:

it faster than glog

we need a separate file to output access log. if we use glog, it may not be so easy to achieve

we hope the access log have no performance impact

ilixiaocui · 2023-05-08T03:23:50Z

curvefs/src/client/filesystem/README.md

+----------------+
+```
+
+(1) kernel cache timeout


ilixiaocui · 2023-05-08T03:30:27Z

curvefs/src/client/filesystem/README.md

+|                       Kernel Cache                    |  (L1 Cache)
+-------------------------------------------------------+
+                            ^  (update length)
+                            |


This figure does not reflect which layer the curve-fuse is on

This figure does not reflect which layer the curve-fuse is on

Actually, everything except the MetaServer and Kernerl Cache is belongs to curve-fuse.

ilixiaocui · 2023-05-08T03:31:37Z

curvefs/src/client/filesystem/README.md

+                            |
+--------------------------------------------------------+
+|                       Open File Cache                  |  (L2 Cache)
+|                   (only length and chunk)              |


What is the basis for caching at different levels?

What is the basis for caching at different levels?

It depends on our solution design.

Wine93 · 2023-05-24T09:42:16Z

cicheck

Wine93 · 2023-05-25T03:03:35Z

cicheck

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

Wine93 · 2023-05-25T07:36:40Z

cicheck

ilixiaocui · 2023-05-30T11:02:45Z

curvefs/conf/client.conf

+fs.lookupCache.negativeTimeoutSec=0
+fs.lookupCache.minUses=1
+fs.lookupCache.lruSize=100000
+fs.dirCache.lruSize=5000000


Now there are more lru caches, have you paid attention to their memory usage during the test?

Now there are more lru caches, have you paid attention to their memory usage during the test?

For reference only: I've seen that if all 5,000,000 directory entries are cached, the fuse process will consume about 8~9GB.

BTW: we can watching it in the next self-test for v2.6.

ilixiaocui · 2023-05-30T14:23:01Z

curvefs/src/client/fuse_client.cpp

-                                   FsFileType type, dev_t rdev, bool internal,
-                                   fuse_entry_param *e) {
-    if (strlen(name) > option_.maxNameLength) {
+CURVEFS_ERROR FuseClient::MakeNode(


MakeNode creates inode and denty. Why not use the filesystem to handle, and cache dentries and attrs into the corresponding cache.

MakeNode creates inode and denty. Why not use the filesystem to handle, and cache dentries and attrs into the corresponding cache.

To ensure the reliability of the file system in this version, we prefer to minimize changes and use the original function unless it is necessary to rewrite the function.

ilixiaocui · 2023-05-31T01:45:58Z

curvefs/src/client/filesystem/filesystem.h

+    void Destory();
+
+    // fuse request
+    CURVEFS_ERROR Lookup(Request req,


Why the following FuseOpXX are handled through the filesystem, and others not?

Why the following FuseOpXX are handled through the filesystem, and others not?

The fuse requests which should been rewrite will be taken over by the filesystem, actually, the part of those requests are usually related to cache. on the other hand, i think we should rewrite all request in filesystem, but to ensure the reliability of the file system, i haven't done it yet.

ilixiaocui · 2023-05-31T02:35:50Z

curvefs/src/client/filesystem/openfile.cpp

+            << ", mtime = " << InodeMtime(file->inode);
+}
+
+void OpenFiles::Evit(size_t size) {


If the file has been opened from the upper layer, but it is better than the limit of LRU, the open state here is cleared, what is the impact？

We will reopen the file which will fetch inode from metaserver, maybe there is a pefomance skew, but actually this is not often happend.

ilixiaocui · 2023-05-31T02:40:00Z

curvefs/src/client/filesystem/openfile.cpp

+    std::shared_ptr<OpenFile> file;
+    bool yes = files_->Get(ino, &file);
+    if (yes) {
+        file->refs++;


What is the meaning of ref++ for this variable, and it does not return.

What is the meaning of ref++ for this variable, and it does not return.

The refs is used for the file which had opened multi-times in the same time, for this we shouldn't really close it.

ilixiaocui · 2023-05-31T02:46:56Z

curvefs/src/client/filesystem/openfile.cpp

+    }
+
+    if (file->refs == 0) {
+        Delete(ino, file, false);  // file already flushed before close


file already flushed before close
Why not Delete(ino, file, true);

file already flushed before close Why not Delete(ino, file, true);

Because the inode already sync to metaserver in FuseOpFlush which trigger by close() system call, so it is no need to sync again here.

ilixiaocui · 2023-05-31T02:55:18Z

curvefs/src/client/filesystem/lookup_cache.cpp

+    auto key = CacheKey(parent, name);
+    bool yes = lru_->Get(key, &entry);
+    if (yes) {
+        entry.uses++;


Under what circumstances will there be multiple puts, making uses++?

Will only not found entry be cached in lookup?

Under what circumstances will there be multiple puts, making uses++?

This is a fairly general design, and one that appears in some caching systems, like nginx: proxy_cache_min_uses, we originally introduced this to solve kernel compilation problems.

Will only not found entry be cached in lookup?

Because the positive result had been cached in kernel.

ilixiaocui · 2023-05-31T03:30:07Z

curvefs/src/client/filesystem/defer_sync.cpp

+
+void DeferSync::Start() {
+    if (!running_.exchange(true)) {
+        thread_ = std::thread(&DeferSync::SyncTask, this);


A single thread doing sync may have a performance bottleneck.

A single thread doing sync may have a performance bottleneck.

Actually we invoke the inode flush by asynchronously in this thread, so i think we can accpet this model.

ilixiaocui · 2023-05-31T06:00:53Z

curvefs/src/client/filesystem/dir_cache.cpp

+                      std::shared_ptr<DirEntryList> entries,
+                      bool evit) {
+    nentries_ -= entries->Size();
+    mq_->Publish(entries);  // clear entries in background


Why do you need to manually clean up the content in the Dentrylist？

Why do you need to manually clean up the content in the Dentrylist？

For free the memory and does not affect performance, the time complexity of the vector.clear() is O(n), so if the entries contains a lot of directory entry which will be an overhead.

Wine93 force-pushed the improve/perf_meta branch 5 times, most recently from 38f9fdd to 76d2633 Compare April 16, 2023 14:09

Wine93 force-pushed the improve/perf_meta branch 7 times, most recently from 3edc197 to 2a864cf Compare April 25, 2023 09:45

SeanHai reviewed Apr 27, 2023

View reviewed changes

ilixiaocui reviewed May 8, 2023

View reviewed changes

wuhongsong added the repairing bug repairing label May 22, 2023

Wine93 force-pushed the improve/perf_meta branch from 2a864cf to ca14517 Compare May 24, 2023 09:41

Wine93 force-pushed the improve/perf_meta branch from ca14517 to 5c0dd33 Compare May 25, 2023 03:00

Wine93 force-pushed the improve/perf_meta branch from 5c0dd33 to 20e79cb Compare May 25, 2023 06:58

curvefs/client: improve metadata performance.

923f12d

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

Wine93 force-pushed the improve/perf_meta branch from 20e79cb to 923f12d Compare May 25, 2023 07:36

SeanHai approved these changes May 25, 2023

View reviewed changes

ilixiaocui reviewed May 31, 2023

View reviewed changes

ilixiaocui approved these changes Jun 1, 2023

View reviewed changes

Wine93 merged commit 4ae45e5 into opencurve:master Jun 1, 2023
3 checks passed

		@@ -0,0 +1,2 @@
		container_name: curve-build-playground-master
		container_image: opencurvedocker/curve-base:build-debian9

curvefs/client: improve metadata performance. #2387

curvefs/client: improve metadata performance. #2387

Conversation

Wine93 commented Apr 11, 2023

Wine93 commented Apr 25, 2023

Choose a reason for hiding this comment

Wine93 Apr 27, 2023 • edited

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 Apr 27, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wine93 Apr 27, 2023 • edited

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 Apr 27, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wine93 Apr 27, 2023 • edited

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 May 25, 2023 • edited

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 May 24, 2023

Wine93 commented May 25, 2023

Wine93 commented May 25, 2023

Choose a reason for hiding this comment

Wine93 Jun 1, 2023 • edited

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 Jun 1, 2023 • edited

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 Jun 1, 2023 • edited

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 Apr 27, 2023 •

edited

Wine93 Apr 27, 2023 •

edited

Wine93 Apr 27, 2023 •

edited

Wine93 Apr 27, 2023 •

edited

Wine93 Apr 27, 2023 •

edited

Wine93 May 25, 2023 •

edited

Wine93 Jun 1, 2023 •

edited

Wine93 Jun 1, 2023 •

edited

Wine93 Jun 1, 2023 •

edited