From fcdee486697ca98a44960e6150bfd36dc34e5fe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=83=A1=E7=8E=AE=E6=96=87?= Date: Thu, 16 Dec 2021 22:31:17 +0800 Subject: [PATCH 1/2] mds: fix crash when exporting unlinked dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When fetch() an unlinked dir, we set fnode->version = 1, but leave projected_version = 0. This will trigger the assert `dir->get_projected_version() == dir->get_version()` in Migrator::encode_export_dir(). projected_version should equal to `fnode->version` unless this dir is projected. Fix this by introducing a new helper CDir::set_fresh_fnode(), which will ensure versions are correctly set. Fixes: https://tracker.ceph.com/issues/53597 Signed-off-by: 胡玮文 --- src/mds/CDir.cc | 35 ++++++++++++++++------------------- src/mds/CDir.h | 1 + 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc index aee737f82b69b..f53e44e6f1892 100644 --- a/src/mds/CDir.cc +++ b/src/mds/CDir.cc @@ -1475,6 +1475,20 @@ void CDir::mark_new(LogSegment *ls) mdcache->mds->queue_waiters(waiters); } +void CDir::set_fresh_fnode(fnode_const_ptr&& ptr) { + ceph_assert(inode->is_auth()); + ceph_assert(!is_projected()); + ceph_assert(!state_test(STATE_COMMITTING)); + reset_fnode(std::move(ptr)); + projected_version = committing_version = committed_version = get_version(); + + if (state_test(STATE_REJOINUNDEF)) { + ceph_assert(mdcache->mds->is_rejoin()); + state_clear(STATE_REJOINUNDEF); + mdcache->opened_undef_dirfrag(this); + } +} + void CDir::mark_clean() { dout(10) << __func__ << " " << *this << " version " << get_version() << dendl; @@ -1547,16 +1561,9 @@ void CDir::fetch(std::string_view dname, snapid_t last, pdir && pdir->inode->is_stray() && !inode->snaprealm) { dout(7) << "fetch dirfrag for unlinked directory, mark complete" << dendl; if (get_version() == 0) { - ceph_assert(inode->is_auth()); auto _fnode = allocate_fnode(); _fnode->version = 1; - reset_fnode(std::move(_fnode)); - - if (state_test(STATE_REJOINUNDEF)) { - ceph_assert(mdcache->mds->is_rejoin()); - state_clear(STATE_REJOINUNDEF); - mdcache->opened_undef_dirfrag(this); - } + set_fresh_fnode(std::move(_fnode)); } mark_complete(); @@ -2043,17 +2050,7 @@ void CDir::_omap_fetched(bufferlist& hdrbl, map& omap, // take the loaded fnode? // only if we are a fresh CDir* with no prior state. if (get_version() == 0) { - ceph_assert(!is_projected()); - ceph_assert(!state_test(STATE_COMMITTING)); - auto _fnode = allocate_fnode(got_fnode); - reset_fnode(std::move(_fnode)); - projected_version = committing_version = committed_version = get_version(); - - if (state_test(STATE_REJOINUNDEF)) { - ceph_assert(mdcache->mds->is_rejoin()); - state_clear(STATE_REJOINUNDEF); - mdcache->opened_undef_dirfrag(this); - } + set_fresh_fnode(allocate_fnode(got_fnode)); } list undef_inodes; diff --git a/src/mds/CDir.h b/src/mds/CDir.h index 1d87f314dcb74..852f86ba826e4 100644 --- a/src/mds/CDir.h +++ b/src/mds/CDir.h @@ -247,6 +247,7 @@ class CDir : public MDSCacheObject, public Counter { void reset_fnode(fnode_const_ptr&& ptr) { fnode = std::move(ptr); } + void set_fresh_fnode(fnode_const_ptr&& ptr); const fnode_const_ptr& get_fnode() const { return fnode; From 9558a6ada253db889bad9d0d0236b00de662eae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=83=A1=E7=8E=AE=E6=96=87?= Date: Tue, 18 Jan 2022 18:02:10 +0800 Subject: [PATCH 2/2] qa: reproduce the crash when exporting unlinked dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 胡玮文 --- qa/tasks/cephfs/mount.py | 25 +++++++++++++++++++++ qa/tasks/cephfs/test_strays.py | 41 +++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/qa/tasks/cephfs/mount.py b/qa/tasks/cephfs/mount.py index bb27b61d687f5..75efeb64ef724 100644 --- a/qa/tasks/cephfs/mount.py +++ b/qa/tasks/cephfs/mount.py @@ -846,6 +846,31 @@ def open_background(self, basename="background_file", write=True): return rproc + def open_dir_background(self, basename): + """ + Create and hold a capability to a directory. + """ + assert(self.is_mounted()) + + path = os.path.join(self.hostfs_mntpt, basename) + + pyscript = dedent(""" + import time + import os + + os.mkdir("{path}") + fd = os.open("{path}", os.O_RDONLY) + while True: + time.sleep(1) + """).format(path=path) + + rproc = self._run_python(pyscript) + self.background_procs.append(rproc) + + self.wait_for_visible(basename) + + return rproc + def wait_for_dir_empty(self, dirname, timeout=30): dirpath = os.path.join(self.hostfs_mntpt, dirname) with safe_while(sleep=5, tries=(timeout//5)) as proceed: diff --git a/qa/tasks/cephfs/test_strays.py b/qa/tasks/cephfs/test_strays.py index 582f1a81bcd4a..8bdc126e2b647 100644 --- a/qa/tasks/cephfs/test_strays.py +++ b/qa/tasks/cephfs/test_strays.py @@ -602,7 +602,6 @@ def _force_migrate(self, path, rank=1): """ :param to_id: MDS id to move it to :param path: Filesystem path (string) to move - :param watch_ino: Inode number to look for at destination to confirm move :return: None """ self.mount_a.run_shell(["setfattr", "-n", "ceph.dir.pin", "-v", str(rank), path]) @@ -700,6 +699,46 @@ def test_migration_on_shutdown(self): # See that the stray counter on rank 0 has incremented self.assertEqual(self.get_mdc_stat("strays_created", rank_0_id), 1) + def test_migrate_unlinked_dir(self): + """ + Reproduce https://tracker.ceph.com/issues/53597 + """ + rank_0_id, rank_1_id = self._setup_two_ranks() + + self.mount_a.run_shell_payload(""" +mkdir pin +touch pin/placeholder +""") + + self._force_migrate("pin") + + # Hold the dir open so it cannot be purged + p = self.mount_a.open_dir_background("pin/to-be-unlinked") + + # Unlink the dentry + self.mount_a.run_shell(["rmdir", "pin/to-be-unlinked"]) + + # Wait to see the stray count increment + self.wait_until_equal( + lambda: self.get_mdc_stat("num_strays", mds_id=rank_1_id), + expect_val=1, timeout=60, reject_fn=lambda x: x > 1) + # but not purged + self.assertEqual(self.get_mdc_stat("strays_created", mds_id=rank_1_id), 1) + self.assertEqual(self.get_mdc_stat("strays_enqueued", mds_id=rank_1_id), 0) + + # Test loading unlinked dir into cache + self.fs.mds_asok(['flush', 'journal'], rank_1_id) + self.fs.mds_asok(['cache', 'drop'], rank_1_id) + + # Shut down rank 1 + self.fs.set_max_mds(1) + self.fs.wait_for_daemons(timeout=120) + # Now the stray should be migrated to rank 0 + # self.assertEqual(self.get_mdc_stat("strays_created", mds_id=rank_0_id), 1) + # https://github.com/ceph/ceph/pull/44335#issuecomment-1125940158 + + self.mount_a.kill_background(p) + def assert_backtrace(self, ino, expected_path): """ Assert that the backtrace in the data pool for an inode matches