mds: distribute dirfrags for ephemeral distributed directory #36537

ukernel · 2020-08-10T06:25:45Z

Instead of distribute individual dir inodes inside the ephemeral
distributed dir. Distributing dirfrags can limit number of subtrees
created by the ephemeral dist pin.

This patch also unifies codes that handle export pin and ephemeral pin.

Fixes: https://tracker.ceph.com/issues/46696
Signed-off-by: "Yan, Zheng" zyan@redhat.com

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

batrick

This PR will also need to update the tests in test_exports.py.

markhpc · 2020-08-10T15:03:41Z

@ukernel I will try giving this a spin on on the io500 cluster setup.

ukernel · 2020-08-12T12:24:15Z

https://pulpito.ceph.com/zyan-2020-08-12_08:48:43-multimds-wip-46696-distro-basic-smithi/

batrick · 2020-08-19T21:44:45Z

retest this please

batrick · 2020-08-20T02:59:17Z

@ukernel there are some qa tox failures to fix.

batrick

I'm mostly focusing on the tests to identify changes in behavior. I'll glance at the code once I grok that.

Still not done with the tests though. Submitting my comments so far...

batrick · 2020-08-20T03:08:35Z

qa/tasks/cephfs/test_exports.py

-        self.assertIsNotNone(which)
+        self._setup_tree(distributed=True)
+        subtrees = self._wait_distributed_subtrees(3 * 2, status=self.status, rank="all")
+        self.mount_a.setfattr("tree", "ceph.dir.pin", "0")


You've changed what the test was checking. We want to check that the immediate child directory (like /tree/3) which is ephemerally pinned to rank 1 can be moved to rank 0 via an export pin.

I realize the directory is fragmented now... can we load the MDS cache to find a sub-directory is pinned to rank 1 and then export pin that?

test_ephemeral_pin_dist_override_before does that

Okay, so what is the purpose of this test? Now the test is pinning /tree instead of /tree/<N>. We expect the fragments to stay pinned now?

Then the test below should just be:

self._wait_subtrees([('/tree', 0)], status=self.status)

self._wait_subtrees([('/tree', 0)], ...) does not work. we don't know how many frags are there

Maybe I'm not understanding the new behavior but we expect 3*2 fragments to be created and distributed after setting the distributed epin on /tree. Then you're pinning /tree to 0. That should cause all of the fragments to lose their ephemeral pins and move back to rank 0, right? Then I would expect the subtrees to merge and /tree should be the only subtree, pinned to rank 0?

merging dirfrags needs a trigger. this is no one

batrick · 2020-08-20T03:14:58Z

qa/tasks/cephfs/test_exports.py

        """
        That turning off ephemeral distributed pin config merges subtrees.
+        FIXME: who triggers the merge?


the test relies on bug in CInode::maybe_export_pin.

batrick · 2020-09-03T20:17:05Z

retest this please

batrick · 2020-09-22T20:20:43Z

retest this please

batrick

Otherwise LGTM

batrick · 2020-09-22T20:43:14Z

src/mds/MDCache.cc

-  if (dir->is_auth()) {
-    /* do this now that we are auth for the CDir */
-    dir->inode->maybe_pin();
-  }


This was part of 306003c. Why is this no longer necessary?

MDBalancer::handle_export_pins() already checks auth subtrees. no reason do the check here

src/mds/MDSRank.cc

batrick · 2020-09-23T18:31:28Z

src/mds/CInode.cc

  }
 }

 void CInode::maybe_export_pin(bool update)
 {
  if (!g_conf()->mds_bal_export_pin)
    return;
-  if (!is_dir() || !is_normal())
+  if (!is_dir())


why remove this check?

because '!is_dir() || !is_normal()' is always true

because '!is_dir() || !is_normal()' is always true

uh? No it's not? The main point of that check was to exclude root.

you are right

ukernel · 2020-09-24T09:46:18Z

retest this please

ukernel · 2020-09-24T12:59:53Z

retest this please

batrick

otherwise LGTM

src/mds/MDSRank.cc

batrick · 2020-09-24T21:29:46Z

src/mds/CInode.cc

  }
 }

 void CInode::maybe_export_pin(bool update)
 {
  if (!g_conf()->mds_bal_export_pin)
    return;
-  if (!is_dir() || !is_normal())
+  if (!is_dir())


because '!is_dir() || !is_normal()' is always true

uh? No it's not? The main point of that check was to exclude root.

batrick

https://pulpito.ceph.com/pdonnell-2020-09-26_05:47:11-fs-wip-pdonnell-testing-20200926.000717-distro-basic-smithi/5471253/

ukernel · 2020-09-29T13:36:02Z

Fixed
https://pulpito.ceph.com/zyan-2020-09-29_08:44:37-fs-wip-46696-testing-basic-smithi/

batrick · 2020-10-09T01:23:30Z

jenkins test make check

batrick · 2020-10-10T01:43:47Z

jenkins test make check

batrick

@ukernel this needs rebased. Testing done here:

https://pulpito.ceph.com/?branch=wip-pdonnell-testing-20201009.012350

I didn't see any failures that looked to be caused by this PR. Please have a glance yourself. Let's merge if you're satisfied.

Instead of distribute individual dir inodes inside the ephemeral distributed dir. Distributing dirfrags can limit number of subtrees created by the ephemeral dist pin. This patch also unifies codes that handle export pin and ephemeral pin. Fixes: https://tracker.ceph.com/issues/46696 Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

ukernel added feature cephfs Ceph File System labels Aug 10, 2020

ukernel requested a review from batrick August 10, 2020 06:25

ukernel force-pushed the wip-46696 branch from 066c33b to 5e25b6f Compare August 10, 2020 13:28

batrick requested changes Aug 10, 2020

View reviewed changes

ukernel force-pushed the wip-46696 branch from 5e25b6f to f1c3b44 Compare August 12, 2020 06:54

markhpc added the performance label Aug 13, 2020

batrick self-requested a review August 13, 2020 23:54

batrick added the needs-review label Aug 13, 2020

ukernel force-pushed the wip-46696 branch from f1c3b44 to 1bfbdbb Compare August 18, 2020 03:40

batrick requested changes Aug 20, 2020

View reviewed changes

ukernel force-pushed the wip-46696 branch from 1bfbdbb to 5fe3321 Compare September 4, 2020 03:21

ukernel force-pushed the wip-46696 branch 2 times, most recently from 5cacb81 to 98846ae Compare September 23, 2020 08:05

batrick reviewed Sep 23, 2020

View reviewed changes

ukernel force-pushed the wip-46696 branch from 98846ae to dbf5020 Compare September 24, 2020 06:53

ukernel force-pushed the wip-46696 branch from dbf5020 to 5f1ebde Compare September 24, 2020 09:48

batrick requested changes Sep 24, 2020

View reviewed changes

ukernel force-pushed the wip-46696 branch from 5f1ebde to b91f8f3 Compare September 25, 2020 00:45

batrick approved these changes Sep 26, 2020

View reviewed changes

batrick added needs-qa and removed needs-review labels Sep 26, 2020

batrick added wip-pdonnell-testing wip-pdonnell-testing2 and removed wip-pdonnell-testing labels Sep 26, 2020

batrick requested changes Sep 28, 2020

View reviewed changes

batrick removed needs-qa wip-pdonnell-testing2 labels Sep 28, 2020

batrick assigned ukernel Sep 28, 2020

ukernel force-pushed the wip-46696 branch from b91f8f3 to 00ee8fe Compare September 29, 2020 13:34

batrick unassigned ukernel Oct 9, 2020

batrick added the wip-pdonnell-testing2 label Oct 9, 2020

batrick approved these changes Oct 13, 2020

View reviewed changes

ukernel added 2 commits October 13, 2020 20:51

qa/cephfs: update ephemeral pin tests

e76abf5

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

ukernel force-pushed the wip-46696 branch from 5e53bcd to e76abf5 Compare October 13, 2020 12:53

batrick merged commit c2be93c into ceph:master Oct 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mds: distribute dirfrags for ephemeral distributed directory #36537

mds: distribute dirfrags for ephemeral distributed directory #36537

ukernel commented Aug 10, 2020

batrick left a comment

markhpc commented Aug 10, 2020

ukernel commented Aug 12, 2020

batrick commented Aug 19, 2020

batrick commented Aug 20, 2020

batrick left a comment

batrick Aug 20, 2020

batrick Aug 20, 2020

ukernel Aug 20, 2020

batrick Aug 20, 2020

ukernel Aug 20, 2020

batrick Aug 20, 2020

ukernel Aug 21, 2020

batrick Aug 20, 2020

ukernel Aug 20, 2020

batrick commented Sep 3, 2020

batrick commented Sep 22, 2020

batrick left a comment

batrick Sep 22, 2020

ukernel Sep 24, 2020

batrick Sep 23, 2020

ukernel Sep 24, 2020

batrick Sep 24, 2020

ukernel Sep 25, 2020

ukernel commented Sep 24, 2020

ukernel commented Sep 24, 2020

batrick left a comment

batrick Sep 24, 2020

batrick left a comment

ukernel commented Sep 29, 2020

batrick commented Oct 9, 2020

batrick commented Oct 10, 2020

batrick left a comment

mds: distribute dirfrags for ephemeral distributed directory #36537

mds: distribute dirfrags for ephemeral distributed directory #36537

Conversation

ukernel commented Aug 10, 2020

Checklist

batrick left a comment

Choose a reason for hiding this comment

markhpc commented Aug 10, 2020

ukernel commented Aug 12, 2020

batrick commented Aug 19, 2020

batrick commented Aug 20, 2020

batrick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

batrick commented Sep 3, 2020

batrick commented Sep 22, 2020

batrick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ukernel commented Sep 24, 2020

ukernel commented Sep 24, 2020

batrick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

batrick left a comment

Choose a reason for hiding this comment

ukernel commented Sep 29, 2020

batrick commented Oct 9, 2020

batrick commented Oct 10, 2020

batrick left a comment

Choose a reason for hiding this comment