From: Rishabh Dave Date: Sun, 17 Aug 2025 18:13:40 +0000 (+0530) Subject: mds: for logging generate only 10 final components of inode path X-Git-Tag: testing/wip-jcollin-testing-20251016.112434-squid^2~6 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f7bbbd3927677f2268d78786eec73c4b6d18dd50;p=ceph-ci.git mds: for logging generate only 10 final components of inode path Generating full absolute path for inodes for printing in MDS logs slows down the FS to a great extent especially when the path is very long (imagine a path with 2000 components). Also printing such long paths in MDS logs is not only pointless but also greatly reduces the readability of the MDS logs. Therefore, generate only 10 final components of inode paths for logging. Fixes: https://tracker.ceph.com/issues/72779 Signed-off-by: Rishabh Dave (cherry picked from commit 1518690210f3a4473978c7a9274e902fccaad862) Conflicts: src/mds/CDir.cc - Certain code region where trimmed inode path was generated was modified to generated inode path but that code region is absent on this branch. --- diff --git a/src/mds/CDentry.cc b/src/mds/CDentry.cc index b9a232798d8..78fbaf72e1c 100644 --- a/src/mds/CDentry.cc +++ b/src/mds/CDentry.cc @@ -242,10 +242,11 @@ void CDentry::clear_auth() } } -void CDentry::make_path_string(string& s, bool projected) const +void CDentry::make_path_string(string& s, bool projected, + int path_comp_count) const { if (dir) { - dir->inode->make_path_string(s, projected); + dir->inode->make_path_string(s, projected, NULL, path_comp_count); } else { s = "???"; } diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h index 2566395d185..99f4911ca55 100644 --- a/src/mds/CDentry.h +++ b/src/mds/CDentry.h @@ -239,7 +239,8 @@ public: const CDentry& operator= (const CDentry& right); // misc - void make_path_string(std::string& s, bool projected=false) const; + 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; // -- version -- diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc index 7ae76cba27d..8d555995d08 100644 --- a/src/mds/CDir.cc +++ b/src/mds/CDir.cc @@ -86,7 +86,7 @@ public: ostream& operator<<(ostream& out, const CDir& dir) { - out << "[dir " << dir.dirfrag() << " " << dir.get_path() << "/" + out << "[dir " << dir.dirfrag() << " " << dir.get_trimmed_path() << "/" << " [" << dir.first << ",head]"; if (dir.is_auth()) { out << " auth"; @@ -2009,7 +2009,7 @@ void CDir::_omap_fetched(bufferlist& hdrbl, map& omap, dout(0) << "_fetched missing object for " << *this << dendl; clog->error() << "dir " << dirfrag() << " object missing on disk; some " - "files may be lost (" << get_path() << ")"; + "files may be lost (" << get_trimmed_path() << ")"; go_bad(complete); return; @@ -2024,14 +2024,14 @@ void CDir::_omap_fetched(bufferlist& hdrbl, map& omap, derr << "Corrupt fnode in dirfrag " << dirfrag() << ": " << err.what() << dendl; clog->warn() << "Corrupt fnode header in " << dirfrag() << ": " - << err.what() << " (" << get_path() << ")"; + << err.what() << " (" << get_trimmed_path() << ")"; go_bad(complete); return; } if (!p.end()) { clog->warn() << "header buffer of dir " << dirfrag() << " has " << hdrbl.length() - p.get_off() << " extra bytes (" - << get_path() << ")"; + << get_trimmed_path() << ")"; go_bad(complete); return; } @@ -2149,7 +2149,7 @@ void CDir::_omap_fetched(bufferlist& hdrbl, map& omap, } catch (const buffer::error &err) { mdcache->mds->clog->warn() << "Corrupt dentry '" << key.name << "' in " "dir frag " << dirfrag() << ": " - << err.what() << "(" << get_path() << ")"; + << err.what() << "(" << get_trimmed_path() << ")"; // Remember that this dentry is damaged. Subsequent operations // that try to act directly on it will get their EIOs, but this @@ -3765,7 +3765,22 @@ bool CDir::scrub_local() return good; } -std::string CDir::get_path() const +/* XXX: Return string containing only final 10 components of the path. This + * shortened path is to be used usually for only logging. This prevents the + * unnecessary generation of full version of a very long path (imagine a path + * with 2000 components) from inode because not only repeatedly printing long + * path in logs is unnecessary and unreadable but more importantly because it + * is an expensive process (since it's done for more or less individual log + * entries). + */ +std::string CDir::get_trimmed_path() const +{ + std::string path; + get_inode()->make_trimmed_path_string(path, true); + return path; +} + +std::string CDir::get_path(bool trim_path) const { std::string path; get_inode()->make_path_string(path, true); diff --git a/src/mds/CDir.h b/src/mds/CDir.h index a5cd3101744..e7092c95747 100644 --- a/src/mds/CDir.h +++ b/src/mds/CDir.h @@ -783,7 +783,8 @@ private: void steal_dentry(CDentry *dn); // from another dir. used by merge/split. void finish_old_fragment(MDSContext::vec& waiters, bool replay); void init_fragment_pins(); - std::string get_path() const; + std::string get_trimmed_path() const; + std::string get_path(bool trim_path=false) const; // -- authority -- /* diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc index 0d5bf7a8c5f..e3dcaa04960 100644 --- a/src/mds/CInode.cc +++ b/src/mds/CInode.cc @@ -151,7 +151,7 @@ int num_cinode_locks = sizeof(cinode_lock_info) / sizeof(cinode_lock_info[0]); ostream& operator<<(ostream& out, const CInode& in) { string path; - in.make_path_string(path, true); + in.make_trimmed_path_string(path); out << "[inode " << in.ino(); out << " [" @@ -1031,15 +1031,29 @@ bool CInode::is_projected_ancestor_of(const CInode *other) const * use_parent is NULL and projected is true, the primary parent's projected * inode is used all the way up the path chain. Otherwise the primary parent * stable inode is used. + * + * path_comp_count = path components count. default value is -1, which implies + * generate full path. */ -void CInode::make_path_string(string& s, bool projected, const CDentry *use_parent) const +void CInode::make_path_string(string& s, bool projected, + const CDentry *use_parent, + int path_comp_count) const { if (!use_parent) { use_parent = projected ? get_projected_parent_dn() : parent; } if (use_parent) { - use_parent->make_path_string(s, projected); + if (path_comp_count == -1) { + use_parent->make_path_string(s, projected, path_comp_count); + } else if (path_comp_count >= 1) { + --path_comp_count; + use_parent->make_path_string(s, projected, path_comp_count); + } else if (path_comp_count == 0) { + // this indicates that path has been shortened. + s = "..."; + return; + } } else if (is_root()) { s = ""; } else if (is_mdsdir()) { @@ -1056,6 +1070,25 @@ void CInode::make_path_string(string& s, bool projected, const CDentry *use_pare } } +/* 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. + * + * path_comp_count = path components count. default value is 10, which implies + * generate full path. + */ +void CInode::make_trimmed_path_string(string& s, bool projected, + const CDentry* use_parent, + int path_comp_count) const +{ + make_path_string(s, projected, use_parent, path_comp_count); +} + void CInode::make_path(filepath& fp, bool projected) const { const CDentry *use_parent = projected ? get_projected_parent_dn() : parent; @@ -2624,7 +2657,7 @@ void CInode::finish_scatter_gather_update(int type, MutationRef& mut) if (pi->dirstat.nfiles < 0 || pi->dirstat.nsubdirs < 0) { std::string path; - make_path_string(path); + make_trimmed_path_string(path); clog->error() << "Inconsistent statistics detected: fragstat on inode " << ino() << " (" << path << "), inode has " << pi->dirstat; ceph_assert(!"bad/negative fragstat" == g_conf()->mds_verify_scatter); @@ -4879,7 +4912,7 @@ next: if (!results->backtrace.passed && in->scrub_infop->header->get_repair()) { std::string path; - in->make_path_string(path); + in->make_trimmed_path_string(path); in->mdcache->mds->clog->warn() << "bad backtrace on inode " << in->ino() << "(" << path << "), rewriting it"; in->mark_dirty_parent(in->mdcache->mds->mdlog->get_current_segment(), diff --git a/src/mds/CInode.h b/src/mds/CInode.h index 7258a45e897..004468800fe 100644 --- a/src/mds/CInode.h +++ b/src/mds/CInode.h @@ -718,8 +718,15 @@ class CInode : public MDSCacheObject, public InodeStoreBase, public Counter* visited=nullptr) const; bool is_projected_ancestor_of(const CInode *other) const; - void make_path_string(std::string& s, bool projected=false, const CDentry *use_parent=NULL) const; + void make_path_string(std::string& s, bool projected=false, + const CDentry *use_parent=NULL, + int path_comp_count=-1) const; void make_path(filepath& s, bool projected=false) const; + void make_trimmed_path_string(std::string& s, bool projected=false, + const CDentry *use_parent=NULL, + int path_comp_count=10) const; + void make_path(filepath& s, bool projected=false, + int path_comp_count=-1) const; void name_stray_dentry(std::string& dname); // -- dirtyness --