From 1cafde22e208f93d1174cb71df5be19966e64004 Mon Sep 17 00:00:00 2001 From: Xiubo Li Date: Tue, 12 Sep 2023 10:25:35 +0800 Subject: [PATCH] Revert "mds: wait unlink to finish to avoid conflict when creating same dentries" This reverts commit d4b9431dfe23a97075766fa2a2b76d7554543b0c. Fixes: https://tracker.ceph.com/issues/61818 Signed-off-by: Xiubo Li (cherry picked from commit 8e240303f85c5faa27504899629a1b33669e5521) --- src/mds/CDentry.h | 15 ++--- src/mds/MDCache.cc | 82 +++--------------------- src/mds/MDCache.h | 3 +- src/mds/Server.cc | 117 +++-------------------------------- src/mds/Server.h | 3 - src/messages/MDentryUnlink.h | 60 ++---------------- src/msg/Message.cc | 3 - src/msg/Message.h | 1 - 8 files changed, 29 insertions(+), 255 deletions(-) diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h index 9b7e1846f7cd5..5473aa00a8972 100644 --- a/src/mds/CDentry.h +++ b/src/mds/CDentry.h @@ -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); } } diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc index 3b3fbc2aab427..72d559f410358 100644 --- a/src/mds/MDCache.cc +++ b/src/mds/MDCache.cc @@ -8235,10 +8235,6 @@ void MDCache::dispatch(const cref_t &m) case MSG_MDS_DENTRYUNLINK: handle_dentry_unlink(ref_cast(m)); break; - case MSG_MDS_DENTRYUNLINK_ACK: - handle_dentry_unlink_ack(ref_cast(m)); - break; - case MSG_MDS_FRAGMENTNOTIFY: handle_fragment_notify(ref_cast(m)); @@ -11227,8 +11223,7 @@ void MDCache::handle_dentry_link(const cref_t &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 @@ -11240,11 +11235,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::iterator it = replicas.begin(); it != replicas.end(); ++it) { @@ -11257,21 +11247,12 @@ void MDCache::send_dentry_unlink(CDentry *dn, CDentry *straydn, rejoin_gather.count(*it))) continue; - auto unlink = make_message(dn->get_dir()->dirfrag(), - dn->get_name(), unlinking); + auto unlink = make_message(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)); } } @@ -11280,40 +11261,23 @@ void MDCache::handle_dentry_unlink(const cref_t &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 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); @@ -11324,12 +11288,11 @@ void MDCache::handle_dentry_unlink(const cref_t &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) @@ -11340,7 +11303,7 @@ void MDCache::handle_dentry_unlink(const cref_t &m) if (in->is_any_caps() && !in->state_test(CInode::STATE_EXPORTINGCAPS)) migrator->export_caps(in); - + straydn = NULL; } else { ceph_assert(!straydn); @@ -11348,7 +11311,6 @@ void MDCache::handle_dentry_unlink(const cref_t &m) dn->dir->unlink_inode(dn); } ceph_assert(dnl->is_null()); - dn->state_clear(CDentry::STATE_UNLINKING); } } @@ -11360,36 +11322,8 @@ void MDCache::handle_dentry_unlink(const cref_t &m) trim_dentry(straydn, ex); send_expire_messages(ex); } - return; - -ack: - ack = make_message(m->get_dirfrag(), m->get_dn()); - mds->send_message(ack, m->get_connection()); } -void MDCache::handle_dentry_unlink_ack(const cref_t &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); - } - } -} diff --git a/src/mds/MDCache.h b/src/mds/MDCache.h index 0069852cf57f0..d9f17303884e7 100644 --- a/src/mds/MDCache.h +++ b/src/mds/MDCache.h @@ -913,7 +913,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); @@ -1156,7 +1156,6 @@ class MDCache { void handle_discover_reply(const cref_t &m); void handle_dentry_link(const cref_t &m); void handle_dentry_unlink(const cref_t &m); - void handle_dentry_unlink_ack(const cref_t &m); int dump_cache(std::string_view fn, Formatter *f, double timeout); diff --git a/src/mds/Server.cc b/src/mds/Server.cc index d70da56219c2a..d585b39380514 100644 --- a/src/mds/Server.cc +++ b/src/mds/Server.cc @@ -1978,23 +1978,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, @@ -4524,11 +4521,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. @@ -6780,44 +6772,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 { @@ -6884,11 +6838,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)) @@ -6987,11 +6936,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(); @@ -7087,11 +7031,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(); @@ -7203,11 +7142,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; @@ -7492,17 +7426,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); @@ -7876,20 +7804,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(); @@ -8164,14 +8082,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()) @@ -8186,7 +8099,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); @@ -8645,16 +8558,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()); diff --git a/src/mds/Server.h b/src/mds/Server.h index a7965a90a8e50..546cecf0e03fe 100644 --- a/src/mds/Server.h +++ b/src/mds/Server.h @@ -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. diff --git a/src/messages/MDentryUnlink.h b/src/messages/MDentryUnlink.h index fc52284416659..210fa033c5219 100644 --- a/src/messages/MDentryUnlink.h +++ b/src/messages/MDentryUnlink.h @@ -22,17 +22,15 @@ 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,66 +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 - friend boost::intrusive_ptr ceph::make_message(Args&&... args); - template - friend MURef crimson::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 diff --git a/src/msg/Message.cc b/src/msg/Message.cc index fdf32b5e09f2b..348546abdf011 100644 --- a/src/msg/Message.cc +++ b/src/msg/Message.cc @@ -822,9 +822,6 @@ Message *decode_message(CephContext *cct, break; - case MSG_MDS_DENTRYUNLINK_ACK: - m = make_message(); - break; case MSG_MDS_DENTRYUNLINK: m = make_message(); break; diff --git a/src/msg/Message.h b/src/msg/Message.h index 9eec1c5bb8385..f27c5448ea2da 100644 --- a/src/msg/Message.h +++ b/src/msg/Message.h @@ -178,7 +178,6 @@ #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 -- 2.39.5