mds: avoid submitting log entry while adjusting subtree map

MDCache::eval_subtree_root() may tigger scatter-gather process, which
submits log entry. Submitting log entry while adjusting subtree map is
bad, because subtree map in intermediate state may get used/logged.

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 3694e1b2b2d9296fdf5d8bd23f8e7f83472f2664..7208ec8e95b9df9c60da3cb3b4c0dfbcdd630099 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -798,14 +798,11 @@ void MDCache::list_subtrees(list<CDir*>& ls)
  * merge with parent and/or child subtrees, if is it appropriate.
  * merge can ONLY happen if both parent and child have unambiguous auth.
  */
-void MDCache::adjust_subtree_auth(CDir *dir, mds_authority_t auth, bool do_eval)
+void MDCache::adjust_subtree_auth(CDir *dir, mds_authority_t auth)
 {
   dout(7) << "adjust_subtree_auth " << dir->get_dir_auth() << " -> " << auth
          << " on " << *dir << dendl;
 
-  if (mds->is_any_replay() || mds->is_resolve())
-    do_eval = false;
-
   show_subtrees();
 
   CDir *root;
@@ -865,9 +862,6 @@ void MDCache::adjust_subtree_auth(CDir *dir, mds_authority_t auth, bool do_eval)
        p = p->inode->get_parent_dir();
       }
     }
-
-    if (do_eval)
-      eval_subtree_root(dir->get_inode());
   }
 
   show_subtrees();
@@ -880,14 +874,18 @@ void MDCache::try_subtree_merge(CDir *dir)
   assert(subtrees.count(dir));
   set<CDir*> oldbounds = subtrees[dir];
 
+  set<CInode*> to_eval;
   // try merge at my root
-  try_subtree_merge_at(dir);
+  try_subtree_merge_at(dir, &to_eval);
 
   // try merge at my old bounds
-  for (set<CDir*>::iterator p = oldbounds.begin();
-       p != oldbounds.end();
-       ++p) 
-    try_subtree_merge_at(*p);
+  for (auto bound : oldbounds)
+    try_subtree_merge_at(bound, &to_eval);
+
+  if (!(mds->is_any_replay() || mds->is_resolve())) {
+    for(auto in : to_eval)
+      eval_subtree_root(in);
+  }
 }
 
 class C_MDC_SubtreeMergeWB : public MDCacheLogContext {
@@ -900,14 +898,11 @@ public:
   }
 };
 
-void MDCache::try_subtree_merge_at(CDir *dir, bool do_eval)
+void MDCache::try_subtree_merge_at(CDir *dir, set<CInode*> *to_eval)
 {
   dout(10) << "try_subtree_merge_at " << *dir << dendl;
   assert(subtrees.count(dir));
 
-  if (mds->is_any_replay() || mds->is_resolve())
-    do_eval = false;
-
   // merge with parent?
   CDir *parent = dir;  
   if (!dir->inode->is_base())
@@ -944,8 +939,8 @@ void MDCache::try_subtree_merge_at(CDir *dir, bool do_eval)
       }
     }
 
-    if (do_eval)
-      eval_subtree_root(dir->get_inode());
+    if (to_eval && dir->get_inode()->is_auth())
+      to_eval->insert(dir->get_inode());
   } 
 
   show_subtrees(15);
@@ -967,8 +962,8 @@ void MDCache::eval_subtree_root(CInode *diri)
 {
   // evaluate subtree inode filelock?
   //  (we should scatter the filelock on subtree bounds)
-  if (diri->is_auth())
-    mds->locker->try_eval(diri, CEPH_LOCK_IFILE | CEPH_LOCK_INEST);
+  assert(diri->is_auth());
+  mds->locker->try_eval(diri, CEPH_LOCK_IFILE | CEPH_LOCK_INEST);
 }
 
 
@@ -1031,6 +1026,8 @@ void MDCache::adjust_bounded_subtree_auth(CDir *dir, set<CDir*>& bounds, mds_aut
     root = dir;
   }
 
+  set<CInode*> to_eval;
+
   // verify/adjust bounds.
   // - these may be new, or
   // - beneath existing ambiguous bounds (which will be collapsed),
@@ -1060,7 +1057,7 @@ void MDCache::adjust_bounded_subtree_auth(CDir *dir, set<CDir*>& bounds, mds_aut
            t = get_subtree_root(t->get_parent_dir());
          dout(10) << "  swallowing intervening subtree at " << *t << dendl;
          adjust_subtree_auth(t, auth);
-         try_subtree_merge_at(t);
+         try_subtree_merge_at(t, &to_eval);
          t = get_subtree_root(bound->get_parent_dir());
          if (t == dir) break;
        }
@@ -1078,7 +1075,7 @@ void MDCache::adjust_bounded_subtree_auth(CDir *dir, set<CDir*>& bounds, mds_aut
        CDir *stray = *p;
        dout(10) << "  swallowing extra subtree at " << *stray << dendl;
        adjust_subtree_auth(stray, auth);
-       try_subtree_merge_at(stray);
+       try_subtree_merge_at(stray, &to_eval);
       }
     }
     // swallowing subtree may add new subtree bounds
@@ -1090,6 +1087,11 @@ void MDCache::adjust_bounded_subtree_auth(CDir *dir, set<CDir*>& bounds, mds_aut
   verify_subtree_bounds(dir, bounds);
 
   show_subtrees();
+
+  if (!(mds->is_any_replay() || mds->is_resolve())) {
+    for(auto in : to_eval)
+      eval_subtree_root(in);
+  }
 }
 
 
@@ -1306,8 +1308,7 @@ void MDCache::project_subtree_rename(CInode *diri, CDir *olddir, CDir *newdir)
   projected_subtree_renames[diri].push_back(pair<CDir*,CDir*>(olddir, newdir));
 }
 
-void MDCache::adjust_subtree_after_rename(CInode *diri, CDir *olddir,
-                                          bool pop, bool imported)
+void MDCache::adjust_subtree_after_rename(CInode *diri, CDir *olddir, bool pop)
 {
   dout(10) << "adjust_subtree_after_rename " << *diri << " from " << *olddir << dendl;
 
@@ -1352,7 +1353,8 @@ void MDCache::adjust_subtree_after_rename(CInode *diri, CDir *olddir,
       subtrees[oldparent].erase(dir);
       assert(subtrees.count(newparent));
       subtrees[newparent].insert(dir);
-      try_subtree_merge_at(dir, !imported);
+      // caller is responsible for 'eval diri'
+      try_subtree_merge_at(dir, NULL);
     } else {
       // mid-subtree.
 
@@ -1377,8 +1379,9 @@ void MDCache::adjust_subtree_after_rename(CInode *diri, CDir *olddir,
 
       // did auth change?
       if (oldparent->authority() != newparent->authority()) {
-       adjust_subtree_auth(dir, oldparent->authority(), !imported);  // caller is responsible for *diri.
-       try_subtree_merge_at(dir, !imported);
+       adjust_subtree_auth(dir, oldparent->authority());
+       // caller is responsible for 'eval diri'
+       try_subtree_merge_at(dir, NULL);
       }
     }
   }

diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h
index 768f4cc9277e88d0ba5cfe374e5879fde5c325e7..1d90fb8ce6697958be59deb0414b38887a0d78b5 100644 (file)
--- a/src/mds/MDCache.h
+++ b/src/mds/MDCache.h
@@ -269,9 +269,9 @@ protected:
 public:
   bool is_subtrees() { return !subtrees.empty(); }
   void list_subtrees(list<CDir*>& ls);
-  void adjust_subtree_auth(CDir *root, mds_authority_t auth, bool do_eval=true);
-  void adjust_subtree_auth(CDir *root, mds_rank_t a, mds_rank_t b=CDIR_AUTH_UNKNOWN, bool do_eval=true) {
-    adjust_subtree_auth(root, mds_authority_t(a,b), do_eval); 
+  void adjust_subtree_auth(CDir *root, mds_authority_t auth);
+  void adjust_subtree_auth(CDir *root, mds_rank_t a, mds_rank_t b=CDIR_AUTH_UNKNOWN) {
+    adjust_subtree_auth(root, mds_authority_t(a,b));
   }
   void adjust_bounded_subtree_auth(CDir *dir, set<CDir*>& bounds, mds_authority_t auth);
   void adjust_bounded_subtree_auth(CDir *dir, set<CDir*>& bounds, mds_rank_t a) {
@@ -283,7 +283,7 @@ public:
   }
   void map_dirfrag_set(list<dirfrag_t>& dfs, set<CDir*>& result);
   void try_subtree_merge(CDir *root);
-  void try_subtree_merge_at(CDir *root, bool do_eval=true);
+  void try_subtree_merge_at(CDir *root, set<CInode*> *to_eval);
   void subtree_merge_writebehind_finish(CInode *in, MutationRef& mut);
   void eval_subtree_root(CInode *diri);
   CDir *get_subtree_root(CDir *dir);
@@ -302,8 +302,7 @@ public:
   void verify_subtree_bounds(CDir *root, const list<dirfrag_t>& bounds);
 
   void project_subtree_rename(CInode *diri, CDir *olddir, CDir *newdir);
-  void adjust_subtree_after_rename(CInode *diri, CDir *olddir,
-                                   bool pop, bool imported = false);
+  void adjust_subtree_after_rename(CInode *diri, CDir *olddir, bool pop);
 
   void get_auth_subtrees(set<CDir*>& s);
   void get_fullauth_subtrees(set<CDir*>& s);

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 2b403400652fdc8f67885affec76405825f3f0c9..d65b3c9d9aa1f1834d28e1b84e29a9780f33cbc1 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -7301,8 +7301,6 @@ void Server::_rename_apply(MDRequestRef& mdr, CDentry *srcdn, CDentry *destdn, C
   CDentry::linkage_t *destdnl = destdn->get_linkage();
 
   CInode *oldin = destdnl->get_inode();
-  
-  bool imported_inode = false;
 
   // primary+remote link merge?
   bool linkmerge = (srcdnl->get_inode() == destdnl->get_inode() &&
@@ -7402,7 +7400,6 @@ void Server::_rename_apply(MDRequestRef& mdr, CDentry *srcdn, CDentry *destdn, C
       
       // hack: fix auth bit
       in->state_set(CInode::STATE_AUTH);
-      imported_inode = true;
 
       mdr->clear_ambiguous_auth();
     }
@@ -7427,7 +7424,7 @@ void Server::_rename_apply(MDRequestRef& mdr, CDentry *srcdn, CDentry *destdn, C
 
   // update subtree map?
   if (destdnl->is_primary() && in->is_dir()) 
-    mdcache->adjust_subtree_after_rename(in, srcdn->get_dir(), true, imported_inode);
+    mdcache->adjust_subtree_after_rename(in, srcdn->get_dir(), true);
 
   if (straydn && oldin->is_dir())
     mdcache->adjust_subtree_after_rename(oldin, destdn->get_dir(), true);

diff --git a/src/mds/journal.cc b/src/mds/journal.cc
index 5d8088234cd0f6c0d3af0cd96a56606c62ac09a2..b63fbd980c8b0808518fc825281a2aea12809a3b 100644 (file)
--- a/src/mds/journal.cc
+++ b/src/mds/journal.cc
@@ -1488,7 +1488,7 @@ void EMetaBlob::replay(MDSRank *mds, LogSegment *logseg, MDSlaveUpdate *slaveup)
        dir = renamed_diri->get_or_open_dirfrag(mds->mdcache, *p);
        dout(10) << " creating new rename import bound " << *dir << dendl;
        dir->state_clear(CDir::STATE_AUTH);
-       mds->mdcache->adjust_subtree_auth(dir, CDIR_AUTH_UNDEF, false);
+       mds->mdcache->adjust_subtree_auth(dir, CDIR_AUTH_UNDEF);
       }
     }