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 <ridave@redhat.com>
(cherry picked from commit 1430cd67d8f7bd7d98b241a7511fa3ceb7e5ba2e)

Conflicts:
src/include/filepath.cc
- Unlike main branch, this file is absent in squid

diff --git a/src/include/filepath.h b/src/include/filepath.h
index 03ae03ee239a3bcec99555bf06611787bb10482d..92d4b06b2b0eb71519f67bd7f4e4fb94db4743f5 100644 (file)
--- a/src/include/filepath.h
+++ b/src/include/filepath.h
@@ -39,6 +39,9 @@
 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.
@@ -93,6 +96,14 @@ class filepath {
   filepath(std::string_view s) { set_path(s); }
   filepath(const char* s) { set_path(s); }
 
+  void set_trimmed() {
+    if (trimmed)
+      return;
+    // indicates that the path has been shortened.
+    path += "...";
+    trimmed = true;
+  }
+
   void set_path(std::string_view s, inodeno_t b) {
     path = s;
     ino = b;

diff --git a/src/mds/CDentry.cc b/src/mds/CDentry.cc
index 78fbaf72e1cef67251ee6b48a2580edada35f5ff..f11690497608623d555edbd4850c64fad49c6876 100644 (file)
--- a/src/mds/CDentry.cc
+++ b/src/mds/CDentry.cc
@@ -47,7 +47,7 @@ const LockType CDentry::versionlock_type(CEPH_LOCK_DVERSION);
 ostream& operator<<(ostream& out, const CDentry& dn)
 {
   filepath path;
-  dn.make_path(path);
+  dn.make_trimmed_path(path);
   
   out << "[dentry " << path;
   
@@ -254,13 +254,44 @@ 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 component count. default value is -1 which implies
+ * generate entire path.
+ */
+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());
+  }
 }
 
+/* path_comp_count = path component count. default value is 10 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_trimmed_path(filepath& fp, bool projected,
+                               int path_comp_count) const
+{
+  make_path(fp, projected, path_comp_count);
+}
 /*
  * we only add ourselves to remote_parents when the linkage is
  * active (no longer projected).  if the passed dnl is projected,

diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h
index 99f4911ca5508327f80bbf32d457ee58ec3b59f0..745397c87330c618cfbfecbd28911c16e4d26cf1 100644 (file)
--- a/src/mds/CDentry.h
+++ b/src/mds/CDentry.h
@@ -241,7 +241,10 @@ 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;
+  void make_trimmed_path(filepath& fp, bool projected=false,
+                        int path_comp_count=10) const;
 
   // -- version --
   version_t get_version() const { return version; }

diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc
index e3dcaa04960d5663be8b35b7acedb954ed2fcd83..c71156a9c1b77341d55ebf0b7d69a56db100ad7e 100644 (file)
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -1089,12 +1089,12 @@ void CInode::make_trimmed_path_string(string& s, bool projected,
   make_path_string(s, projected, use_parent, path_comp_count);
 }
 
-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 004468800fe36496d75396c08c9b3b47d0887784..6734d908c9781a8e64dc349d050edae800aba716 100644 (file)
--- a/src/mds/CInode.h
+++ b/src/mds/CInode.h
@@ -721,7 +721,6 @@ class CInode : public MDSCacheObject, public InodeStoreBase, public Counter<CIno
   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;