From ede5f93f2735b039061ea43821e0f83451fcd4c3 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Thu, 21 Aug 2025 17:21:48 +0530 Subject: [PATCH] mds: for logging generate only 10 final components of dentry path Generating full absolute path for dentries for printing in MDS logs slows the down the FS to a great extent especially when the path is very long (imagine a path with 2000 components). Printing such long paths in MDS logs is not only pointless but also greatly reduces the readability of MDS logs. Therefore, generate only 10 final components of the dentry paths for logging. Fixes: https://tracker.ceph.com/issues/72779 Signed-off-by: Rishabh Dave --- src/include/filepath.cc | 8 ++++++++ src/include/filepath.h | 4 ++++ src/mds/CDentry.cc | 33 ++++++++++++++++++++++++++++----- src/mds/CDentry.h | 3 ++- src/mds/CInode.cc | 4 ++-- src/mds/CInode.h | 3 ++- 6 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/include/filepath.cc b/src/include/filepath.cc index 243bf127d0b..f0324f603c4 100644 --- a/src/include/filepath.cc +++ b/src/include/filepath.cc @@ -41,6 +41,14 @@ void filepath::parse_bits() const { } } +void filepath::set_trimmed() { + if (trimmed) + return; + // indicates that the path has been shortened. + path += "..."; + trimmed = true; +} + void filepath::set_path(std::string_view s) { if (!s.empty() && s[0] == '/') { path = s.substr(1); diff --git a/src/include/filepath.h b/src/include/filepath.h index 67be4a08759..920ffcb95cb 100644 --- a/src/include/filepath.h +++ b/src/include/filepath.h @@ -38,6 +38,9 @@ namespace ceph { class Formatter; } class filepath { inodeno_t ino = 0; // base inode. ino=0 implies pure relative path. std::string path; // relative path. + // tells get_path() whether it should prefix path with "..." to indicate that + // it was shortened. + bool trimmed = false; /** bits - path segments * this is ['a', 'b', 'c'] for both the aboslute and relative case. @@ -82,6 +85,7 @@ class filepath { // accessors inodeno_t get_ino() const { return ino; } const std::string& get_path() const { return path; } + void set_trimmed(); const char *c_str() const { return path.c_str(); } int length() const { return path.length(); } diff --git a/src/mds/CDentry.cc b/src/mds/CDentry.cc index d2796d4c709..19ad3c7fd88 100644 --- a/src/mds/CDentry.cc +++ b/src/mds/CDentry.cc @@ -85,7 +85,7 @@ const LockType CDentry::versionlock_type(CEPH_LOCK_DVERSION); ostream& operator<<(ostream& out, const CDentry& dn) { filepath path; - dn.make_path(path); + dn.make_path(path, false, 10); out << "[dentry " << path; @@ -312,11 +312,34 @@ void CDentry::make_path_string(string& s, bool projected, s.append(name.data(), name.length()); } -void CDentry::make_path(filepath& fp, bool projected) const +/* path_comp_count = path components count. default value is -1 which implies + * generate entire path. + * + * XXX Generating more than 10 components of a path for printing in logs will + * consume too much time when the path is too long (imagine a path with 2000 + * components) since the path would've to be generated indidividually for each + * log entry. + * + * Besides consuming too much time, such long paths in logs are not only not + * useful but also it makes reading logs harder. Therefore, shorten the path + * when used for logging. + */ +void CDentry::make_path(filepath& fp, bool projected, + int path_comp_count) const { - ceph_assert(dir); - dir->inode->make_path(fp, projected); - fp.push_dentry(get_name()); + fp.set_trimmed(); + + if (path_comp_count == -1) { + ceph_assert(dir); + dir->inode->make_path(fp, projected, path_comp_count); + fp.push_dentry(get_name()); + } else if (path_comp_count >= 1) { + --path_comp_count; + + ceph_assert(dir); + dir->inode->make_path(fp, projected, path_comp_count); + fp.push_dentry(get_name()); + } } /* diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h index 67cf9323666..8a88da41582 100644 --- a/src/mds/CDentry.h +++ b/src/mds/CDentry.h @@ -254,7 +254,8 @@ public: // misc void make_path_string(std::string& s, bool projected=false, int path_comp_count=-1) const; - void make_path(filepath& fp, bool projected=false) const; + void make_path(filepath& fp, bool projected=false, + int path_comp_count=-1) const; // -- version -- version_t get_version() const { return version; } diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc index 6d99ccd7a65..94e1dde89a9 100644 --- a/src/mds/CInode.cc +++ b/src/mds/CInode.cc @@ -1144,12 +1144,12 @@ void CInode::make_path_string(string& s, bool projected, } } -void CInode::make_path(filepath& fp, bool projected) const +void CInode::make_path(filepath& fp, bool projected, int path_comp_count) const { const CDentry *use_parent = projected ? get_projected_parent_dn() : parent; if (use_parent) { ceph_assert(!is_base()); - use_parent->make_path(fp, projected); + use_parent->make_path(fp, projected, path_comp_count); } else { fp = filepath(ino()); } diff --git a/src/mds/CInode.h b/src/mds/CInode.h index b00a2a22cd0..2b9e7884440 100644 --- a/src/mds/CInode.h +++ b/src/mds/CInode.h @@ -745,7 +745,8 @@ class CInode : public MDSCacheObject, public InodeStoreBase, public Counter