mds: fix remote auth pin race

When removing auth unpinned objects from mdr->remote_auth_pins,
Server::handle_slave_auth_pin() checks object's authority to decide
if the object was auth pinned by a given MDS. This method isn't
reliable because when object isn't auth pinned, its authority may
change.

The fix is remember from which MDS an objects was auth pinned.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>

diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index 38ccff06f0ac1e02daec6e089ec2e9c325d69747..3ffe709de9c7b2e2ab8de7067b35ac0574137bb7 100644 (file)
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -350,14 +350,14 @@ bool Locker::acquire_locks(MDRequestRef& mdr,
   // request remote auth_pins
   if (!mustpin_remote.empty()) {
     marker.message = "requesting remote authpins";
-    for (set<MDSCacheObject*>::iterator p = mdr->remote_auth_pins.begin();
+    for (map<MDSCacheObject*,int>::iterator p = mdr->remote_auth_pins.begin();
         p != mdr->remote_auth_pins.end();
         ++p) {
-      if (mustpin.count(*p)) {
-       int auth = (*p)->authority().first;
-       map<int, set<MDSCacheObject*> >::iterator q = mustpin_remote.find(auth);
+      if (mustpin.count(p->first)) {
+       assert(p->second == p->first->authority().first);
+       map<int, set<MDSCacheObject*> >::iterator q = mustpin_remote.find(p->second);
        if (q != mustpin_remote.end())
-         q->second.insert(*p);
+         q->second.insert(p->first);
       }
     }
     for (map<int, set<MDSCacheObject*> >::iterator p = mustpin_remote.begin();

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 1605fe1475841b18b4d97e2946b7a2aa95aa7526..8cac24b8729f6a897f0404ea4411f6a2ef3cf96f 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -3732,24 +3732,24 @@ void MDCache::rejoin_send_rejoins()
       if (mdr->is_slave())
        continue;
       // auth pins
-      for (set<MDSCacheObject*>::iterator q = mdr->remote_auth_pins.begin();
+      for (map<MDSCacheObject*,int>::iterator q = mdr->remote_auth_pins.begin();
           q != mdr->remote_auth_pins.end();
           ++q) {
-       if (!(*q)->is_auth()) {
-         int who = (*q)->authority().first;
-         if (rejoins.count(who) == 0) continue;
-         MMDSCacheRejoin *rejoin = rejoins[who];
+       if (!q->first->is_auth()) {
+         assert(q->second == q->first->authority().first);
+         if (rejoins.count(q->second) == 0) continue;
+         MMDSCacheRejoin *rejoin = rejoins[q->second];
          
-         dout(15) << " " << *mdr << " authpin on " << **q << dendl;
+         dout(15) << " " << *mdr << " authpin on " << *q->first << dendl;
          MDSCacheObjectInfo i;
-         (*q)->set_object_info(i);
+         q->first->set_object_info(i);
          if (i.ino)
            rejoin->add_inode_authpin(vinodeno_t(i.ino, i.snapid), mdr->reqid, mdr->attempt);
          else
            rejoin->add_dentry_authpin(i.dirfrag, i.dname, i.snapid, mdr->reqid, mdr->attempt);
 
          if (mdr->has_more() && mdr->more()->is_remote_frozen_authpin &&
-             mdr->more()->rename_inode == (*q))
+             mdr->more()->rename_inode == q->first)
            rejoin->add_inode_frozen_authpin(vinodeno_t(i.ino, i.snapid),
                                             mdr->reqid, mdr->attempt);
        }

diff --git a/src/mds/Mutation.h b/src/mds/Mutation.h
index 2a704235d5e2e7947035d554c4f2897b37055aa8..fdabe29683e16bfcde0f3df7f06436c0258f01a6 100644 (file)
--- a/src/mds/Mutation.h
+++ b/src/mds/Mutation.h
@@ -50,7 +50,7 @@ struct MutationImpl {
   set<CInode*> stickydirs;
 
   // auth pins
-  set< MDSCacheObject* > remote_auth_pins;
+  map<MDSCacheObject*,int> remote_auth_pins;
   set< MDSCacheObject* > auth_pins;
   
   // held locks

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 2172f5cabc0f2ee440115399b0602b02f41b192f..642d38f2a0601163d368a88316973ea7b080b563 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -1660,6 +1660,8 @@ void Server::handle_slave_auth_pin(MDRequestRef& mdr)
        fail = true;
        break;
       }
+      if (mdr->is_auth_pinned(*p))
+       continue;
       if (!mdr->can_auth_pin(*p)) {
        if (mdr->slave_request->is_nonblock()) {
          dout(10) << " can't auth_pin (freezing?) " << **p << " nonblocking" << dendl;
@@ -1769,21 +1771,19 @@ void Server::handle_slave_auth_pin_ack(MDRequestRef& mdr, MMDSSlaveRequest *ack)
     assert(object);  // we pinned it
     dout(10) << " remote has pinned " << *object << dendl;
     if (!mdr->is_auth_pinned(object))
-      mdr->remote_auth_pins.insert(object);
+      mdr->remote_auth_pins[object] = from;
     if (*p == ack->get_authpin_freeze())
       mdr->set_remote_frozen_auth_pin(static_cast<CInode *>(object));
     pinned.insert(object);
   }
 
   // removed auth pins?
-  set<MDSCacheObject*>::iterator p = mdr->remote_auth_pins.begin();
+  map<MDSCacheObject*,int>::iterator p = mdr->remote_auth_pins.begin();
   while (p != mdr->remote_auth_pins.end()) {
-    if ((*p)->authority().first == from &&
-       pinned.count(*p) == 0) {
-      dout(10) << " remote has unpinned " << **p << dendl;
-      set<MDSCacheObject*>::iterator o = p;
-      ++p;
-      mdr->remote_auth_pins.erase(o);
+    MDSCacheObject* object = p->first;
+    if (p->second == from && pinned.count(object) == 0) {
+      dout(10) << " remote has unpinned " << *object << dendl;
+      mdr->remote_auth_pins.erase(p++);
     } else {
       ++p;
     }