mds: fix try_remove_unlinked_dn

Clear dentry NEW flag when we commit it.. pretty sure that was the
old bug.  And we can't cheat the directory commit because the fnode
also gets updated.

diff --git a/src/TODO b/src/TODO
index b895723c3924aa513a2a3859b427a45d38a43e91..76eea96d57d489ff8ed7f3f8b41ef48ffaaeb193 100644 (file)
--- a/src/TODO
+++ b/src/TODO
@@ -130,7 +130,6 @@ mds
 - add an up:shadow mode?
   - tail the mds log as it is written
   - periodically check head so that we trim, too
-- try_remove_unlinked_dn thing
 - rename: importing inode... also journal imported client map?
 - rerun destro trace against latest, with various journal lengths
 - cap/lease length heuristics

diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h
index 5c6ec5bd19094e8d8d1111940920e27c0425e4db..abe1973f81bbdfaa6367c4b75411e9b6f3d9691d 100644 (file)
--- a/src/mds/CDentry.h
+++ b/src/mds/CDentry.h
@@ -261,6 +261,7 @@ public:
 
   void mark_new();
   bool is_new() { return state_test(STATE_NEW); }
+  void clear_new() { state_clear(STATE_NEW); }
   
   // -- replication
   void encode_replica(int mds, bufferlist& bl) {

diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc
index fbcfdfb74525c770a5a32a2056c7cc077f4f7cff..caf7669bdace92a45c4c377893c9deb221d70b5f 100644 (file)
--- a/src/mds/CDir.cc
+++ b/src/mds/CDir.cc
@@ -60,7 +60,7 @@ ostream& operator<<(ostream& out, CDir& dir)
     out << " v=" << dir.get_version();
     out << " cv=" << dir.get_committing_version();
     out << "/" << dir.get_committed_version();
-    out << "/" << dir.get_committed_version_equivalent();
+    //out << "/" << dir.get_committed_version_equivalent();
   } else {
     pair<int,int> a = dir.authority();
     out << " rep@" << a.first;
@@ -158,7 +158,8 @@ CDir::CDir(CInode *in, frag_t fg, MDCache *mdcache, bool auth) :
   projected_version = 0;
 
   committing_version = 0;
-  committed_version_equivalent = committed_version = 0;
+  //committed_version_equivalent = 
+  committed_version = 0;
 
   // dir_auth
   dir_auth = CDIR_AUTH_DEFAULT;
@@ -443,36 +444,20 @@ void CDir::try_remove_unlinked_dn(CDentry *dn)
 {
   assert(dn->dir == this);
   assert(dn->get_linkage()->is_null());
-  assert(dn->is_dirty());
   
-  /* FIXME: there is a bug in this.  i think new dentries are properly
-     identified.. e.g. maybe a dentry exists, is committed, is removed, is now
-     dirty+null, then reused and mistakenly considered new.. then it is removed, 
-     we remove it here, the dir is fetched, and the dentry exists again.  
-     
-     somethign like that...
-  */
-  return;
-
-
   // no pins (besides dirty)?
-  if (dn->get_num_ref() != 1) 
+  if (dn->get_num_ref() != dn->is_dirty()) 
     return;
 
   // was the dn new?  or is the dir complete (i.e. we don't need negatives)? 
   if (dn->is_new() || is_complete()) {
     dout(10) << "try_remove_unlinked_dn " << *dn << " in " << *this << dendl;
-    dn->mark_clean();
+    if (dn->is_dirty())
+      dn->mark_clean();
     remove_dentry(dn);
 
-    if (!is_projected() &&
-       committing_version == committed_version &&
-       num_dirty == 0) {
-      dout(10) << "try_remove_unlinked_dn committed_equivalent now " << get_version() 
-              << " vs committed " << committed_version
-              << dendl;
-      committed_version_equivalent = committed_version;    
-    }
+    // NOTE: we may not have any more dirty dentries, but the fnode
+    // still changed, so the directory must remain dirty.
   }
 }
 
@@ -1430,6 +1415,9 @@ void CDir::_commit(version_t want)
     
     n++;
 
+    // clear dentry NEW flag, if any.  we can no longer silently drop it.
+    dn->clear_new();
+
     // primary or remote?
     if (dn->linkage.is_remote()) {
       inodeno_t ino = dn->linkage.get_remote_ino();
@@ -1588,7 +1576,7 @@ void CDir::encode_export(bufferlist& bl)
   ::encode(fnode, bl);
   ::encode(dirty_old_rstat, bl);
   ::encode(committed_version, bl);
-  ::encode(committed_version_equivalent, bl);
+  //::encode(committed_version_equivalent, bl);
 
   ::encode(state, bl);
   ::encode(dir_rep, bl);
@@ -1618,7 +1606,7 @@ void CDir::decode_import(bufferlist::iterator& blp)
   ::decode(dirty_old_rstat, blp);
   projected_version = fnode.version;
   ::decode(committed_version, blp);
-  ::decode(committed_version_equivalent, blp);
+  //::decode(committed_version_equivalent, blp);
   committing_version = committed_version;
 
   unsigned s;

diff --git a/src/mds/CDir.h b/src/mds/CDir.h
index 1bb1549d425fa4e519451f92220cb68103cc732d..078dc4490bda2e9fcf06d205a0f40fa6def6850b 100644 (file)
--- a/src/mds/CDir.h
+++ b/src/mds/CDir.h
@@ -211,7 +211,7 @@ protected:
   // state
   version_t committing_version;
   version_t committed_version;
-  version_t committed_version_equivalent;  // in case of, e.g., temporary file
+  //version_t committed_version_equivalent;  // in case of, e.g., temporary file
 
 
   // lock nesting, freeze
@@ -434,7 +434,7 @@ private:
   // -- dirtyness --
   version_t get_committing_version() { return committing_version; }
   version_t get_committed_version() { return committed_version; }
-  version_t get_committed_version_equivalent() { return committed_version_equivalent; }
+  //version_t get_committed_version_equivalent() { return committed_version_equivalent; }
   void set_committed_version(version_t v) { committed_version = v; }
 
   void mark_complete() { state_set(STATE_COMPLETE); }