]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
Revert "mds: wait unlink to finish to avoid conflict when creating same dentries"
authorXiubo Li <xiubli@redhat.com>
Mon, 18 Sep 2023 02:02:32 +0000 (10:02 +0800)
committerXiubo Li <xiubli@redhat.com>
Mon, 18 Sep 2023 02:08:04 +0000 (10:08 +0800)
This reverts commit 76b099b29cefbee770aba1fafea678e6891795c0.

Fixes: https://tracker.ceph.com/issues/61818
Signed-off-by: Xiubo Li <xiubli@redhat.com>
(cherry picked from commit 8e240303f85c5faa27504899629a1b33669e5521)
Conflicts:
    src/messages/MDentryUnlink.h

src/mds/CDentry.h
src/mds/MDCache.cc
src/mds/MDCache.h
src/mds/Server.cc
src/mds/Server.h
src/messages/MDentryUnlink.h
src/msg/Message.cc
src/msg/Message.h

index 03932285cf7614dcb7851424fa516c851cc3b9a8..6897f1749a2bb38fa093b24bce3bb10180c8db6a 100644 (file)
@@ -29,7 +29,6 @@
 #include "BatchOp.h"
 #include "MDSCacheObject.h"
 #include "MDSContext.h"
-#include "Mutation.h"
 #include "SimpleLock.h"
 #include "LocalLockC.h"
 #include "ScrubHeader.h"
@@ -87,23 +86,18 @@ public:
   static const int STATE_EVALUATINGSTRAY = (1<<4);
   static const int STATE_PURGINGPINNED =  (1<<5);
   static const int STATE_BOTTOMLRU =    (1<<6);
-  static const int STATE_UNLINKING =    (1<<7);
   // stray dentry needs notification of releasing reference
   static const int STATE_STRAY =       STATE_NOTIFYREF;
   static const int MASK_STATE_IMPORT_KEPT = STATE_BOTTOMLRU;
 
   // -- pins --
-  static const int PIN_INODEPIN =         1;  // linked inode is pinned
-  static const int PIN_FRAGMENTING =     -2;  // containing dir is refragmenting
-  static const int PIN_PURGING =          3;
-  static const int PIN_SCRUBPARENT =      4;
-  static const int PIN_WAITUNLINKSTATE  = 5;
+  static const int PIN_INODEPIN =     1;  // linked inode is pinned
+  static const int PIN_FRAGMENTING = -2;  // containing dir is refragmenting
+  static const int PIN_PURGING =      3;
+  static const int PIN_SCRUBPARENT =  4;
 
   static const unsigned EXPORT_NONCE = 1;
 
-  const static uint64_t WAIT_UNLINK_STATE  = (1<<0);
-  const static uint64_t WAIT_UNLINK_FINISH = (1<<1);
-  uint32_t replica_unlinking_ref = 0;
 
   CDentry(std::string_view n, __u32 h,
           mempool::mds_co::string alternate_name,
@@ -142,7 +136,6 @@ public:
     case PIN_FRAGMENTING: return "fragmenting";
     case PIN_PURGING: return "purging";
     case PIN_SCRUBPARENT: return "scrubparent";
-    case PIN_WAITUNLINKSTATE: return "waitunlinkstate";
     default: return generic_pin_name(p);
     }
   }
index d271bceaab8de271868ea692589b622cc94a71bf..2244437e45289c070da942859fdcc0754988546f 100644 (file)
@@ -8107,10 +8107,6 @@ void MDCache::dispatch(const cref_t<Message> &m)
   case MSG_MDS_DENTRYUNLINK:
     handle_dentry_unlink(ref_cast<MDentryUnlink>(m));
     break;
-  case MSG_MDS_DENTRYUNLINK_ACK:
-    handle_dentry_unlink_ack(ref_cast<MDentryUnlinkAck>(m));
-    break;
-
 
   case MSG_MDS_FRAGMENTNOTIFY:
     handle_fragment_notify(ref_cast<MMDSFragmentNotify>(m));
@@ -11051,8 +11047,7 @@ void MDCache::handle_dentry_link(const cref_t<MDentryLink> &m)
 
 // UNLINK
 
-void MDCache::send_dentry_unlink(CDentry *dn, CDentry *straydn,
-                                 MDRequestRef& mdr, bool unlinking)
+void MDCache::send_dentry_unlink(CDentry *dn, CDentry *straydn, MDRequestRef& mdr)
 {
   dout(10) << __func__ << " " << *dn << dendl;
   // share unlink news with replicas
@@ -11064,11 +11059,6 @@ void MDCache::send_dentry_unlink(CDentry *dn, CDentry *straydn,
     CInode *strayin = straydn->get_linkage()->get_inode();
     strayin->encode_snap_blob(snapbl);
   }
-
-  if (unlinking) {
-    ceph_assert(!straydn);
-    dn->replica_unlinking_ref = 0;
-  }
   for (set<mds_rank_t>::iterator it = replicas.begin();
        it != replicas.end();
        ++it) {
@@ -11081,21 +11071,12 @@ void MDCache::send_dentry_unlink(CDentry *dn, CDentry *straydn,
         rejoin_gather.count(*it)))
       continue;
 
-    auto unlink = make_message<MDentryUnlink>(dn->get_dir()->dirfrag(),
-                                              dn->get_name(), unlinking);
+    auto unlink = make_message<MDentryUnlink>(dn->get_dir()->dirfrag(), dn->get_name());
     if (straydn) {
       encode_replica_stray(straydn, *it, unlink->straybl);
       unlink->snapbl = snapbl;
     }
     mds->send_message_mds(unlink, *it);
-    if (unlinking) {
-      dn->replica_unlinking_ref++;
-      dn->get(CDentry::PIN_WAITUNLINKSTATE);
-    }
-  }
-
-  if (unlinking && dn->replica_unlinking_ref) {
-    dn->add_waiter(CDentry::WAIT_UNLINK_STATE, new C_MDS_RetryRequest(this, mdr));
   }
 }
 
@@ -11104,40 +11085,23 @@ void MDCache::handle_dentry_unlink(const cref_t<MDentryUnlink> &m)
   // straydn
   CDentry *straydn = nullptr;
   CInode *strayin = nullptr;
-
   if (m->straybl.length())
     decode_replica_stray(straydn, &strayin, m->straybl, mds_rank_t(m->get_source().num()));
 
-  boost::intrusive_ptr<MDentryUnlinkAck> ack;
-  CDentry::linkage_t *dnl;
-  CDentry *dn;
-  CInode *in;
-  bool hadrealm;
-
   CDir *dir = get_dirfrag(m->get_dirfrag());
   if (!dir) {
     dout(7) << __func__ << " don't have dirfrag " << m->get_dirfrag() << dendl;
-    if (m->is_unlinking())
-      goto ack;
   } else {
-    dn = dir->lookup(m->get_dn());
+    CDentry *dn = dir->lookup(m->get_dn());
     if (!dn) {
       dout(7) << __func__ << " don't have dentry " << *dir << " dn " << m->get_dn() << dendl;
-      if (m->is_unlinking())
-        goto ack;
     } else {
       dout(7) << __func__ << " on " << *dn << dendl;
-
-      if (m->is_unlinking()) {
-        dn->state_set(CDentry::STATE_UNLINKING);
-        goto ack;
-      }
-
-      dnl = dn->get_linkage();
+      CDentry::linkage_t *dnl = dn->get_linkage();
 
       // open inode?
       if (dnl->is_primary()) {
-       in = dnl->get_inode();
+       CInode *in = dnl->get_inode();
        dn->dir->unlink_inode(dn);
        ceph_assert(straydn);
        straydn->dir->link_primary_inode(straydn, in);
@@ -11148,12 +11112,11 @@ void MDCache::handle_dentry_unlink(const cref_t<MDentryUnlink> &m)
        in->first = straydn->first;
 
        // update subtree map?
-       if (in->is_dir()) {
+       if (in->is_dir()) 
          adjust_subtree_after_rename(in, dir, false);
-       }
 
        if (m->snapbl.length()) {
-         hadrealm = (in->snaprealm ? true : false);
+         bool hadrealm = (in->snaprealm ? true : false);
          in->decode_snap_blob(m->snapbl);
          ceph_assert(in->snaprealm);
          if (!hadrealm)
@@ -11164,7 +11127,7 @@ void MDCache::handle_dentry_unlink(const cref_t<MDentryUnlink> &m)
        if (in->is_any_caps() &&
            !in->state_test(CInode::STATE_EXPORTINGCAPS))
          migrator->export_caps(in);
-
+       
        straydn = NULL;
       } else {
        ceph_assert(!straydn);
@@ -11172,7 +11135,6 @@ void MDCache::handle_dentry_unlink(const cref_t<MDentryUnlink> &m)
        dn->dir->unlink_inode(dn);
       }
       ceph_assert(dnl->is_null());
-      dn->state_clear(CDentry::STATE_UNLINKING);
     }
   }
 
@@ -11184,36 +11146,8 @@ void MDCache::handle_dentry_unlink(const cref_t<MDentryUnlink> &m)
     trim_dentry(straydn, ex);
     send_expire_messages(ex);
   }
-  return;
-
-ack:
-  ack = make_message<MDentryUnlinkAck>(m->get_dirfrag(), m->get_dn());
-  mds->send_message(ack, m->get_connection());
 }
 
-void MDCache::handle_dentry_unlink_ack(const cref_t<MDentryUnlinkAck> &m)
-{
-  CDir *dir = get_dirfrag(m->get_dirfrag());
-  if (!dir) {
-    dout(7) << __func__ << " don't have dirfrag " << m->get_dirfrag() << dendl;
-  } else {
-    CDentry *dn = dir->lookup(m->get_dn());
-    if (!dn) {
-      dout(7) << __func__ << " don't have dentry " << *dir << " dn " << m->get_dn() << dendl;
-    } else {
-      dout(7) << __func__ << " on " << *dn << " ref "
-             << dn->replica_unlinking_ref << " -> "
-             << dn->replica_unlinking_ref - 1 << dendl;
-      dn->replica_unlinking_ref--;
-      if (!dn->replica_unlinking_ref) {
-        MDSContext::vec finished;
-        dn->take_waiting(CDentry::WAIT_UNLINK_STATE, finished);
-        mds->queue_waiters(finished);
-      }
-      dn->put(CDentry::PIN_WAITUNLINKSTATE);
-    }
-  }
-}
 
 
 
index 7707179b90a9e4fe00ea15bae358d97404932147..9bfab20e8b722415535b1bce609467d836ef161d 100644 (file)
@@ -897,7 +897,7 @@ class MDCache {
   void encode_remote_dentry_link(CDentry::linkage_t *dnl, bufferlist& bl);
   void decode_remote_dentry_link(CDir *dir, CDentry *dn, bufferlist::const_iterator& p);
   void send_dentry_link(CDentry *dn, MDRequestRef& mdr);
-  void send_dentry_unlink(CDentry *dn, CDentry *straydn, MDRequestRef& mdr, bool unlinking=false);
+  void send_dentry_unlink(CDentry *dn, CDentry *straydn, MDRequestRef& mdr);
 
   void wait_for_uncommitted_fragment(dirfrag_t dirfrag, MDSContext *c) {
     uncommitted_fragments.at(dirfrag).waiters.push_back(c);
@@ -1133,7 +1133,6 @@ class MDCache {
   void handle_discover_reply(const cref_t<MDiscoverReply> &m);
   void handle_dentry_link(const cref_t<MDentryLink> &m);
   void handle_dentry_unlink(const cref_t<MDentryUnlink> &m);
-  void handle_dentry_unlink_ack(const cref_t<MDentryUnlinkAck> &m);
 
   int dump_cache(std::string_view fn, Formatter *f);
 
index cfd43f350a472c55717ffd7fe0bd348d72c4fc34..5b115144731fd0d849be62808d03781f249f6d44 100644 (file)
@@ -1970,23 +1970,20 @@ void Server::journal_and_reply(MDRequestRef& mdr, CInode *in, CDentry *dn, LogEv
     mdr->pin(dn);
 
   early_reply(mdr, in, dn);
-
+  
   mdr->committing = true;
   submit_mdlog_entry(le, fin, mdr, __func__);
-
+  
   if (mdr->client_request && mdr->client_request->is_queued_for_replay()) {
     if (mds->queue_one_replay()) {
       dout(10) << " queued next replay op" << dendl;
     } else {
       dout(10) << " journaled last replay op" << dendl;
     }
-  } else if (mdr->did_early_reply) {
+  } else if (mdr->did_early_reply)
     mds->locker->drop_rdlocks_for_early_reply(mdr.get());
-    if (dn && dn->is_waiter_for(CDentry::WAIT_UNLINK_FINISH))
-      mdlog->flush();
-  } else {
+  else
     mdlog->flush();
-  }
 }
 
 void Server::submit_mdlog_entry(LogEvent *le, MDSLogContextBase *fin, MDRequestRef& mdr,
@@ -4533,11 +4530,6 @@ void Server::handle_client_openc(MDRequestRef& mdr)
   if (!dn)
     return;
 
-  if (is_unlink_pending(dn)) {
-    wait_for_pending_unlink(dn, mdr);
-    return;
-  }
-
   CDentry::linkage_t *dnl = dn->get_projected_linkage();
   if (!excl && !dnl->is_null()) {
     // it existed.
@@ -6710,44 +6702,6 @@ void Server::handle_client_getvxattr(MDRequestRef& mdr)
 
 // ------------------------------------------------
 
-struct C_WaitUnlinkToFinish : public MDSContext {
-protected:
-  MDCache *mdcache;
-  CDentry *dn;
-  MDSContext *fin;
-
-  MDSRank *get_mds() override
-  {
-    ceph_assert(mdcache != NULL);
-    return mdcache->mds;
-  }
-
-public:
-  C_WaitUnlinkToFinish(MDCache *m, CDentry *d, MDSContext *f) :
-    mdcache(m), dn(d), fin(f) {}
-  void finish(int r) override {
-    fin->complete(r);
-    dn->put(CDentry::PIN_PURGING);
-  }
-};
-
-bool Server::is_unlink_pending(CDentry *dn)
-{
-  CDentry::linkage_t *dnl = dn->get_projected_linkage();
-  if (!dnl->is_null() && dn->state_test(CDentry::STATE_UNLINKING)) {
-      return true;
-  }
-  return false;
-}
-
-void Server::wait_for_pending_unlink(CDentry *dn, MDRequestRef& mdr)
-{
-  mds->locker->drop_locks(mdr.get());
-  auto fin = new C_MDS_RetryRequest(mdcache, mdr);
-  dn->get(CDentry::PIN_PURGING);
-  dn->add_waiter(CDentry::WAIT_UNLINK_FINISH, new C_WaitUnlinkToFinish(mdcache, dn, fin));
-}
-
 // MKNOD
 
 class C_MDS_mknod_finish : public ServerLogContext {
@@ -6814,11 +6768,6 @@ void Server::handle_client_mknod(MDRequestRef& mdr)
   if (!dn)
     return;
 
-  if (is_unlink_pending(dn)) {
-    wait_for_pending_unlink(dn, mdr);
-    return;
-  }
-
   CDir *dir = dn->get_dir();
   CInode *diri = dir->get_inode();
   if (!check_access(mdr, diri, MAY_WRITE))
@@ -6917,11 +6866,6 @@ void Server::handle_client_mkdir(MDRequestRef& mdr)
   if (!dn)
     return;
 
-  if (is_unlink_pending(dn)) {
-    wait_for_pending_unlink(dn, mdr);
-    return;
-  }
-
   CDir *dir = dn->get_dir();
   CInode *diri = dir->get_inode();
 
@@ -7017,11 +6961,6 @@ void Server::handle_client_symlink(MDRequestRef& mdr)
   if (!dn)
     return;
 
-  if (is_unlink_pending(dn)) {
-    wait_for_pending_unlink(dn, mdr);
-    return;
-  }
-
   CDir *dir = dn->get_dir();
   CInode *diri = dir->get_inode();
 
@@ -7133,11 +7072,6 @@ void Server::handle_client_link(MDRequestRef& mdr)
     targeti = ret.second->get_projected_linkage()->get_inode();
   }
 
-  if (is_unlink_pending(destdn)) {
-    wait_for_pending_unlink(destdn, mdr);
-    return;
-  }
-
   ceph_assert(destdn->get_projected_linkage()->is_null());
   if (req->get_alternate_name().size() > alternate_name_max) {
     dout(10) << " alternate_name longer than " << alternate_name_max << dendl;
@@ -7422,17 +7356,11 @@ void Server::_link_remote_finish(MDRequestRef& mdr, bool inc,
   mdr->apply();
 
   MDRequestRef null_ref;
-  if (inc) {
+  if (inc)
     mdcache->send_dentry_link(dn, null_ref);
-  } else {
-    dn->state_clear(CDentry::STATE_UNLINKING);
+  else
     mdcache->send_dentry_unlink(dn, NULL, null_ref);
-
-    MDSContext::vec finished;
-    dn->take_waiting(CDentry::WAIT_UNLINK_FINISH, finished);
-    mdcache->mds->queue_waiters(finished);
-  }
-
+  
   // bump target popularity
   mds->balancer->hit_inode(targeti, META_POP_IWR);
   mds->balancer->hit_dir(dn->get_dir(), META_POP_IWR);
@@ -7806,20 +7734,10 @@ void Server::handle_client_unlink(MDRequestRef& mdr)
 
   if (rmdir)
     mdr->disable_lock_cache();
-
   CDentry *dn = rdlock_path_xlock_dentry(mdr, false, true);
   if (!dn)
     return;
 
-  // notify replica MDSes the dentry is under unlink
-  if (!dn->state_test(CDentry::STATE_UNLINKING)) {
-    dn->state_set(CDentry::STATE_UNLINKING);
-    mdcache->send_dentry_unlink(dn, nullptr, mdr, true);
-    if (dn->replica_unlinking_ref) {
-      return;
-    }
-  }
-
   CDentry::linkage_t *dnl = dn->get_linkage(client, mdr);
   ceph_assert(!dnl->is_null());
   CInode *in = dnl->get_inode();
@@ -8094,14 +8012,9 @@ void Server::_unlink_local_finish(MDRequestRef& mdr,
   }
 
   mdr->apply();
-
-  dn->state_clear(CDentry::STATE_UNLINKING);
+  
   mdcache->send_dentry_unlink(dn, straydn, mdr);
-
-  MDSContext::vec finished;
-  dn->take_waiting(CDentry::WAIT_UNLINK_FINISH, finished);
-  mdcache->mds->queue_waiters(finished);
-
+  
   if (straydn) {
     // update subtree map?
     if (strayin->is_dir())
@@ -8116,7 +8029,7 @@ void Server::_unlink_local_finish(MDRequestRef& mdr,
 
   // reply
   respond_to_request(mdr, 0);
-
+  
   // removing a new dn?
   dn->get_dir()->try_remove_unlinked_dn(dn);
 
@@ -8575,16 +8488,6 @@ void Server::handle_client_rename(MDRequestRef& mdr)
   if (!destdn)
     return;
 
-  if (is_unlink_pending(destdn)) {
-    wait_for_pending_unlink(destdn, mdr);
-    return;
-  }
-
-  if (is_unlink_pending(srcdn)) {
-    wait_for_pending_unlink(srcdn, mdr);
-    return;
-  }
-
   dout(10) << " destdn " << *destdn << dendl;
   CDir *destdir = destdn->get_dir();
   ceph_assert(destdir->is_auth());
index 8e35ec9ad78dd23f7bd8edb5d3f614b5a33fa99f..efe9e0636d575c93b271c84430e2ef9c765f0a04 100644 (file)
@@ -235,9 +235,6 @@ public:
 
   void handle_client_fsync(MDRequestRef& mdr);
 
-  bool is_unlink_pending(CDentry *dn);
-  void wait_for_pending_unlink(CDentry *dn, MDRequestRef& mdr);
-
   // open
   void handle_client_open(MDRequestRef& mdr);
   void handle_client_openc(MDRequestRef& mdr);  // O_CREAT variant.
index 01240f5dccecf72da2b8bb4a7d85b2599c556149..02edb92fb3054a88e77cd8fe5aa73151cc8c472f 100644 (file)
 
 class MDentryUnlink final : public MMDSOp {
 private:
-  static constexpr int HEAD_VERSION = 2;
+  static constexpr int HEAD_VERSION = 1;
   static constexpr int COMPAT_VERSION = 1;
-
+  
   dirfrag_t dirfrag;
   std::string dn;
-  bool unlinking = false;
 
  public:
   dirfrag_t get_dirfrag() const { return dirfrag; }
   const std::string& get_dn() const { return dn; }
-  bool is_unlinking() const { return unlinking; }
 
   ceph::buffer::list straybl;
   ceph::buffer::list snapbl;
@@ -40,9 +38,10 @@ private:
 protected:
   MDentryUnlink() :
     MMDSOp(MSG_MDS_DENTRYUNLINK, HEAD_VERSION, COMPAT_VERSION) { }
-  MDentryUnlink(dirfrag_t df, std::string_view n, bool u=false) :
+  MDentryUnlink(dirfrag_t df, std::string_view n) :
     MMDSOp(MSG_MDS_DENTRYUNLINK, HEAD_VERSION, COMPAT_VERSION),
-    dirfrag(df), dn(n), unlinking(u) {}
+    dirfrag(df),
+    dn(n) {}
   ~MDentryUnlink() final {}
 
 public:
@@ -50,64 +49,19 @@ public:
   void print(std::ostream& o) const override {
     o << "dentry_unlink(" << dirfrag << " " << dn << ")";
   }
-
+  
   void decode_payload() override {
     using ceph::decode;
     auto p = payload.cbegin();
     decode(dirfrag, p);
     decode(dn, p);
     decode(straybl, p);
-    if (header.version >= 2)
-      decode(unlinking, p);
   }
   void encode_payload(uint64_t features) override {
     using ceph::encode;
     encode(dirfrag, payload);
     encode(dn, payload);
     encode(straybl, payload);
-    encode(unlinking, payload);
-  }
-private:
-  template<class T, typename... Args>
-  friend boost::intrusive_ptr<T> ceph::make_message(Args&&... args);
-};
-
-class MDentryUnlinkAck final : public MMDSOp {
-private:
-  static constexpr int HEAD_VERSION = 1;
-  static constexpr int COMPAT_VERSION = 1;
-
-  dirfrag_t dirfrag;
-  std::string dn;
-
- public:
-  dirfrag_t get_dirfrag() const { return dirfrag; }
-  const std::string& get_dn() const { return dn; }
-
-protected:
-  MDentryUnlinkAck() :
-    MMDSOp(MSG_MDS_DENTRYUNLINK_ACK, HEAD_VERSION, COMPAT_VERSION) { }
-  MDentryUnlinkAck(dirfrag_t df, std::string_view n) :
-    MMDSOp(MSG_MDS_DENTRYUNLINK_ACK, HEAD_VERSION, COMPAT_VERSION),
-    dirfrag(df), dn(n) {}
-  ~MDentryUnlinkAck() final {}
-
-public:
-  std::string_view get_type_name() const override { return "dentry_unlink_ack";}
-  void print(std::ostream& o) const override {
-    o << "dentry_unlink_ack(" << dirfrag << " " << dn << ")";
-  }
-
-  void decode_payload() override {
-    using ceph::decode;
-    auto p = payload.cbegin();
-    decode(dirfrag, p);
-    decode(dn, p);
-  }
-  void encode_payload(uint64_t features) override {
-    using ceph::encode;
-    encode(dirfrag, payload);
-    encode(dn, payload);
   }
 private:
   template<class T, typename... Args>
index 3f4405a50912ae6b0863eebc192ea1013edb05ff..266eb7676786b85ea7481855fa6d8bd94c600a56 100644 (file)
@@ -822,9 +822,6 @@ Message *decode_message(CephContext *cct,
     break;
 
 
-  case MSG_MDS_DENTRYUNLINK_ACK:
-    m = make_message<MDentryUnlinkAck>();
-    break;
   case MSG_MDS_DENTRYUNLINK:
     m = make_message<MDentryUnlink>();
     break;
index b211474c3affbf98af496bef3e9a2e0b14082a97..5ea78c22d9543f8108bc61d9ef75a64156f43def 100644 (file)
 #define MSG_MDS_OPENINOREPLY       0x210
 #define MSG_MDS_SNAPUPDATE         0x211
 #define MSG_MDS_FRAGMENTNOTIFYACK  0x212
-#define MSG_MDS_DENTRYUNLINK_ACK   0x213
 #define MSG_MDS_LOCK               0x300 // 0x3xx are for locker of mds
 #define MSG_MDS_INODEFILECAPS      0x301