mds: track dirty dentries in separate list

this should improve performance of large directory Fixes: http://tracker.ceph.com/issues/19578 Signed-off-by: "Yan, Zheng" <[email protected]>
LeoHubery · Dec 21, 2017 · 4955966 · 4955966
1 parent 80c186f
commit 4955966
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 35 deletions.
diff --git a/src/mds/CDentry.cc b/src/mds/CDentry.cc
@@ -159,8 +159,9 @@ void CDentry::_mark_dirty(LogSegment *ls)
   // state+pin
   if (!state_test(STATE_DIRTY)) {
     state_set(STATE_DIRTY);
-    dir->inc_num_dirty();
     get(PIN_DIRTY);
+    dir->inc_num_dirty();
+    dir->dirty_dentries.push_back(&item_dir_dirty);
     assert(ls);
   }
   if (ls) 
@@ -189,15 +190,14 @@ void CDentry::mark_clean()
   // not always true for recalc_auth_bits during resolve finish
   //assert(dir->get_version() == 0 || version <= dir->get_version());  // hmm?
 
-  // state+pin
-  state_clear(STATE_DIRTY);
+  state_clear(STATE_DIRTY|STATE_NEW);
   dir->dec_num_dirty();
-  put(PIN_DIRTY);
-
+
+  item_dir_dirty.remove_myself();
   item_dirty.remove_myself();
 
-  clear_new();
-}    
+  put(PIN_DIRTY);
+}
 
 void CDentry::mark_new() 
 {

diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h
@@ -347,7 +347,7 @@ class CDentry : public MDSCacheObject, public LRUObject, public Counter<CDentry>
   __u32 hash;
   snapid_t first, last;
 
-  elist<CDentry*>::item item_dirty;
+  elist<CDentry*>::item item_dirty, item_dir_dirty;
   elist<CDentry*>::item item_stray;
 
   // lock

diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc
@@ -183,7 +183,9 @@ CDir::CDir(CInode *in, frag_t fg, MDCache *mdcache, bool auth) :
   cache(mdcache), inode(in), frag(fg),
   first(2),
   dirty_rstat_inodes(member_offset(CInode, dirty_rstat_item)),
-  projected_version(0),  item_dirty(this), item_new(this),
+  projected_version(0),
+  dirty_dentries(member_offset(CDentry, item_dir_dirty)),
+  item_dirty(this), item_new(this),
   num_head_items(0), num_head_null(0),
   num_snap_items(0), num_snap_null(0),
   num_dirty(0), committing_version(0), committed_version(0),
@@ -863,8 +865,10 @@ void CDir::steal_dentry(CDentry *dn)
     dn->dir->adjust_nested_auth_pins(-ap, -dap, NULL);
   }
 
-  if (dn->is_dirty()) 
+  if (dn->is_dirty()) {
+    dirty_dentries.push_back(&dn->item_dir_dirty);
     num_dirty++;
+  }
 
   dn->dir = this;
 }
@@ -2121,11 +2125,7 @@ void CDir::_omap_commit(int op_prio)
     stale_items.clear();
   }
 
-  for (map_t::iterator p = items.begin();
-      p != items.end(); ) {
-    CDentry *dn = p->second;
-    ++p;
-
+  auto write_one = [&](CDentry *dn) {
     string key;
     dn->key().encode(key);
 
@@ -2134,13 +2134,9 @@ void CDir::_omap_commit(int op_prio)
       dout(10) << " rm " << key << dendl;
       write_size += key.length();
       to_remove.insert(key);
-      continue;
+      return;
     }
 
-    if (!dn->is_dirty() &&
-	(!dn->state_test(CDentry::STATE_FRAGMENTING) || dn->get_linkage()->is_null()))
-      continue;  // skip clean dentries
-
     if (dn->get_linkage()->is_null()) {
       dout(10) << " rm " << dn->name << " " << *dn << dendl;
       write_size += key.length();
@@ -2174,6 +2170,22 @@ void CDir::_omap_commit(int op_prio)
       to_set.clear();
       to_remove.clear();
     }
+  };
+
+  if (state_test(CDir::STATE_FRAGMENTING)) {
+    for (auto p = items.begin(); p != items.end(); ) {
+      CDentry *dn = p->second;
+      ++p;
+      if (!dn->is_dirty() && dn->get_linkage()->is_null())
+	continue;
+      write_one(dn);
+    }
+  } else {
+    for (auto p = dirty_dentries.begin(); !p.end(); ) {
+      CDentry *dn = *p;
+      ++p;
+      write_one(dn);
+    }
   }
 
   ObjectOperation op;
@@ -2342,10 +2354,9 @@ void CDir::_committed(int r, version_t v)
     mark_clean();
 
   // dentries clean?
-  for (map_t::iterator it = items.begin();
-       it != items.end(); ) {
-    CDentry *dn = it->second;
-    ++it;
+  for (auto p = dirty_dentries.begin(); !p.end(); ) {
+    CDentry *dn = *p;
+    ++p;
 
     // inode?
     if (dn->linkage.is_primary()) {
@@ -2366,19 +2377,18 @@ void CDir::_committed(int r, version_t v)
 
     // dentry
     if (committed_version >= dn->get_version()) {
-      if (dn->is_dirty()) {
-	dout(15) << " dir " << committed_version << " >= dn " << dn->get_version() << " now clean " << *dn << dendl;
-	dn->mark_clean();
+      dout(15) << " dir " << committed_version << " >= dn " << dn->get_version() << " now clean " << *dn << dendl;
+      dn->mark_clean();
 
-	// drop clean null stray dentries immediately
-	if (stray && 
-	    dn->get_num_ref() == 0 &&
-	    !dn->is_projected() &&
-	    dn->get_linkage()->is_null())
-	  remove_dentry(dn);
-      } 
+      // drop clean null stray dentries immediately
+      if (stray &&
+	  dn->get_num_ref() == 0 &&
+	  !dn->is_projected() &&
+	  dn->get_linkage()->is_null())
+	remove_dentry(dn);
     } else {
       dout(15) << " dir " << committed_version << " < dn " << dn->get_version() << " still dirty " << *dn << dendl;
+      assert(dn->is_dirty());
     }
   }
 

diff --git a/src/mds/CDir.h b/src/mds/CDir.h
@@ -174,9 +174,9 @@ class CDir : public MDSCacheObject, public Counter<CDir> {
   std::list<fnode_t*> projected_fnode;
 
 public:
+  elist<CDentry*> dirty_dentries;
   elist<CDir*>::item item_dirty, item_new;
 
-
 public:
   version_t get_version() const { return fnode.version; }
   void set_version(version_t v) {