Skip to content

Commit

Permalink
mds: fix issuing redundant reintegrate/migrate_stray requests
Browse files Browse the repository at this point in the history
Just in cause a CInode's nlink is 1, and then a unlink request comes
and then early replies and submits to the MDLogs, but just before
the MDlogs are flushed a link request comes, and the link request
also succeeds and early replies to client.

Later when the unlink/link requests' MDLog events are flushed and
the callbacks are called, which will fire a stray denty reintegration.
But it will pick the new dentry, which is from the link's request
and is a remote dentry, to do the reintegration. While in the
'rename' code when traversing the path it will trigger to call the
'dn->link_remote()', which later will fire a new stray dentry
reintegration.

The problem is if the first 'rename' request is retried several
times, and in each time it will fire a new reintegration, which
makes no sense and maybe blocked for a very long time dues to some
reasons and then will be reported as slow request warning.

Fixes: https://tracker.ceph.com/issues/62702
Signed-off-by: Xiubo Li <xiubli@redhat.com>
  • Loading branch information
lxbsz committed Sep 5, 2023
1 parent 27edb75 commit b2ab55d
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/include/ceph_fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ union ceph_mds_request_args_legacy {
#define CEPH_MDS_FLAG_REPLAY 1 /* this is a replayed op */
#define CEPH_MDS_FLAG_WANT_DENTRY 2 /* want dentry in reply */
#define CEPH_MDS_FLAG_ASYNC 4 /* request is async */
#define CEPH_MDS_FLAG_REINTEGRATE 8 /* request is reintegration */

struct ceph_mds_request_head_legacy {
__le64 oldest_client_tid;
Expand Down
6 changes: 4 additions & 2 deletions src/mds/CDentry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ void CDentry::make_path(filepath& fp, bool projected) const
* active (no longer projected). if the passed dnl is projected,
* don't link in, and do that work later in pop_projected_linkage().
*/
void CDentry::link_remote(CDentry::linkage_t *dnl, CInode *in)
void CDentry::link_remote(CDentry::linkage_t *dnl, CInode *in, bool reintegrating)
{
ceph_assert(dnl->is_remote());
ceph_assert(in->ino() == dnl->get_remote_ino());
Expand All @@ -277,7 +277,9 @@ void CDentry::link_remote(CDentry::linkage_t *dnl, CInode *in)
in->add_remote_parent(this);

// check for reintegration
dir->mdcache->eval_remote(this);
if (!reintegrating) {
dir->mdcache->eval_remote(this);
}
}

void CDentry::unlink_remote(CDentry::linkage_t *dnl)
Expand Down
2 changes: 1 addition & 1 deletion src/mds/CDentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ class CDentry : public MDSCacheObject, public LRUObject, public Counter<CDentry>
int get_num_dir_auth_pins() const;

// remote links
void link_remote(linkage_t *dnl, CInode *in);
void link_remote(linkage_t *dnl, CInode *in, bool reintegrating=false);
void unlink_remote(linkage_t *dnl);

// copy cons
Expand Down
4 changes: 2 additions & 2 deletions src/mds/MDCache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8495,7 +8495,7 @@ int MDCache::path_traverse(MDRequestRef& mdr, MDSContextFactory& cf,
in = get_inode(dnl->get_remote_ino());
if (in) {
dout(7) << "linking in remote in " << *in << dendl;
dn->link_remote(dnl, in);
dn->link_remote(dnl, in, mdr->client_request->is_reintegration());
} else {
dout(7) << "remote link to " << dnl->get_remote_ino() << ", which i don't have" << dendl;
ceph_assert(mdr); // we shouldn't hit non-primary dentries doing a non-mdr traversal!
Expand Down Expand Up @@ -8775,7 +8775,7 @@ CInode *MDCache::get_dentry_inode(CDentry *dn, MDRequestRef& mdr, bool projected
CInode *in = get_inode(dnl->get_remote_ino());
if (in) {
dout(7) << "get_dentry_inode linking in remote in " << *in << dendl;
dn->link_remote(dnl, in);
dn->link_remote(dnl, in, mdr->client_request->is_reintegration());
return in;
} else {
dout(10) << "get_dentry_inode on remote dn, opening inode for " << *dn << dendl;
Expand Down
2 changes: 2 additions & 0 deletions src/mds/StrayManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,7 @@ void StrayManager::reintegrate_stray(CDentry *straydn, CDentry *rdn)
ceph_tid_t tid = mds->issue_tid();

auto req = make_message<MClientRequest>(CEPH_MDS_OP_RENAME);
req->set_reintegrated_op();
req->set_filepath(dst);
req->set_filepath2(src);
req->set_tid(tid);
Expand Down Expand Up @@ -722,6 +723,7 @@ void StrayManager::migrate_stray(CDentry *dn, mds_rank_t to)
ceph_tid_t tid = mds->issue_tid();

auto req = make_message<MClientRequest>(CEPH_MDS_OP_RENAME);
req->set_reintegrated_op();
req->set_filepath(dst);
req->set_filepath2(src);
req->set_tid(tid);
Expand Down
8 changes: 8 additions & 0 deletions src/messages/MClientRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ class MClientRequest final : public MMDSOp {
return get_flags() & CEPH_MDS_FLAG_ASYNC;
}

bool is_reintegration() const {
return get_flags() & CEPH_MDS_FLAG_REINTEGRATE;
}

// normal fields
void set_stamp(utime_t t) { stamp = t; }
void set_oldest_client_tid(ceph_tid_t t) { head.oldest_client_tid = t; }
Expand All @@ -192,6 +196,10 @@ class MClientRequest final : public MMDSOp {
head.flags = head.flags | CEPH_MDS_FLAG_ASYNC;
}

void set_reintegrated_op() {
head.flags = head.flags | CEPH_MDS_FLAG_REINTEGRATE;
}

void set_alternate_name(std::string _alternate_name) {
alternate_name = std::move(_alternate_name);
}
Expand Down

0 comments on commit b2ab55d

Please sign in to comment.