client,mds: case-insensitive directory trees by batrick · Pull Request #60746 · ceph/ceph

batrick · 2024-11-15T02:23:40Z

This is the PR of the case-insensitive tree work in CephFS. Comments welcome.

TODO:

resolve remaining TODO/FIXME in patches

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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 dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

batrick · 2024-11-15T02:56:58Z

I'm aware there are some clang build failures. Will address tomorrow.

doc/cephfs/charmap.rst

batrick · 2025-01-07T00:20:36Z

Docs: https://ceph--60746.org.readthedocs.build/en/60746/cephfs/charmap/

This acquires a lock which is a no-no in the messenger. Signed-off-by: Patrick Donnelly <[email protected]>

Many dispatchers return false to allow other dispatchers also common messages like MOSDMap or MFSMap. They implicitly depend on some dispatcher which is always at the "tail" of the dispatcher queue to return "true" indicating the msg was processed to avoid messages like: 2025-02-18T05:31:17.738+0000 7f5206546640 0 ms_deliver_dispatch: unhandled message 0x5632d05f0700 fsmap(e 9) from mon.0 v2:172.21.3.230:40412/0 but this cannot always happen when some libraries like the RadosClient used standalone. So, add a variant for encapsulating other indications for how the message was processed by dispatch2. For example, a message may be "acknowledged" but explicitly allow other dispatchers to try processing the message. Note: we're using a variant to avoid updating all of the ms_dispatch code to use the sentinel classes. Signed-off-by: Patrick Donnelly <[email protected]>

Ancillary change: do not do client upkeep after map processing. Signed-off-by: Patrick Donnelly <[email protected]>

Convert ms_dispatch to ms_dispatch2 to enable indicating that a map message is acknowledged and instead of processed (or deliberately not processed). Signed-off-by: Patrick Donnelly <[email protected]>

This avoids messages like: 2025-02-18T05:31:17.738+0000 7f5206546640 0 ms_deliver_dispatch: unhandled message 0x5632d05f0700 fsmap(e 9) from mon.0 v2:172.21.3.230:40412/0 Signed-off-by: Patrick Donnelly <[email protected]>

Instead of marking the message as handled, give another component (or Client) a chance to process. Signed-off-by: Patrick Donnelly <[email protected]>

batrick · 2025-02-28T00:57:16Z

Adding the QA test changes added for https://tracker.ceph.com/issues/70197#note-5

batrick · 2025-02-28T13:49:10Z

jenkins test api

vshankar · 2025-03-03T11:51:49Z

Approved on the basic of QA runs (mostly) and implementation which I went through a while back. This is good to merge.

batrick · 2025-03-03T13:31:23Z

Approved on the basic of QA runs (mostly) and implementation which I went through a while back. This is good to merge.

Thanks Venky!

cbodley · 2025-03-04T17:06:53Z

CMakeLists.txt


+if(WITH_LIBCEPHFS)
+  find_package(ICU REQUIRED COMPONENTS uc i18n)
+  list(APPEND BOOST_COMPONENTS locale)


the client target depends on Boost::locale, but is not conditional on WITH_LIBCEPHFS. i raised #62108 to fix compilation when WITH_LIBCEPHFS=OFF

cbodley · 2025-03-04T17:18:10Z

src/pybind/cephfs/cephfs.pyx

+            with nogil:
+                ret = ceph_mds_command2(self.cluster, _mds_spec,


any idea why this doesn't build on fedora with python 3.13?

performance hint: cephfs.pyx:114:5: Exception check on 'completion_callback' will always require the GIL to be acquired. Possible solutions: 1. Declare 'completion_callback' as 'noexcept' if you control the definition and you're sure you don't want the function to raise exceptions. 2. Use an 'int' return type on 'completion_callback' to allow an error code to be returned. Error compiling Cython file: ------------------------------------------------------------ ... with nogil: ret = ceph_mds_command2(self.cluster, _mds_spec, <const char **>_cmd, _cmdlen, <const char*>_inbuf, _inbuf_len, _one_shot, completion_callback, ^ ------------------------------------------------------------ cephfs.pyx:2324:40: Cannot assign type 'void (int, const void *, size_t, const void *, size_t, void *) except * nogil' to 'libcephfs_c_completion_t' (alias of 'void (*)(int, const void *, size_t, const void *, size_t, void *) noexcept nogil'). Exception values are incompatible. Suggest adding 'noexcept' to the type of 'completion_callback'.

Cython version perhaps. See: #62099

@cbodley can you please try #62107

man fchownat(2)[1] says the following: . . . AT_EMPTY_PATH (since Linux 2.6.39) If pathname is an empty string, operate on the file referred to by dirfd (which may have been obtained using the open(2) O_PATH flag). In this case, dirfd can refer to any type of file, not just a directory. If dirfd is AT_FDCWD, the call operates on the current working directory. . . . Look out for an empty pathname and use the relative fd's inode in the presence of AT_EMPTY_PATH flag before calling internal _setattr(). Fixes: https://tracker.ceph.com/issues/68189 Review with: git show -w Note: path_walk() refactor from ceph#60746 incorporates the core changes required here and thus src/libcephfs.cc changes have been removed from the cherry-pick as they conflict at the same time. [1] https://www.man7.org/linux/man-pages/man2/fchownat.2.html Signed-off-by: Anoop C S <[email protected]> (cherry picked from commit 829f388)

man statx(2)[1] says the following: . . . AT_EMPTY_PATH If pathname is an empty string, operate on the file referred to by dirfd (which may have been obtained using the open(2) O_PATH flag). In this case, dirfd can refer to any type of file, not just a directory. If dirfd is AT_FDCWD, the call operates on the current working directory. . . . Look out for an empty pathname and use the relative fd's inode in the presence of AT_EMPTY_PATH flag before calling internal _getattr(). Fixes: https://tracker.ceph.com/issues/68189 Review with: git show -w Note: path_walk() refactor from ceph#60746 incorporates the core changes required here and thus src/libcephfs.cc changes have been removed from the cherry-pick as they conflict at the same time. [1] https://www.man7.org/linux/man-pages/man2/statx.2.html Signed-off-by: Anoop C S <[email protected]> (cherry picked from commit edd7fe7)

man fchownat(2)[1] says the following: . . . AT_EMPTY_PATH (since Linux 2.6.39) If pathname is an empty string, operate on the file referred to by dirfd (which may have been obtained using the open(2) O_PATH flag). In this case, dirfd can refer to any type of file, not just a directory. If dirfd is AT_FDCWD, the call operates on the current working directory. . . . Look out for an empty pathname and use the relative fd's inode in the presence of AT_EMPTY_PATH flag before calling internal _setattr(). Fixes: https://tracker.ceph.com/issues/68189 Review with: git show -w Note: path_walk() refactor from ceph#60746 included the core changes required here and thus src/client/Client.cc changes have been removed from the cherry-pick as they conflict at the same time. [1] https://www.man7.org/linux/man-pages/man2/fchownat.2.html Signed-off-by: Anoop C S <[email protected]> (cherry picked from commit 829f388)

man statx(2)[1] says the following: . . . AT_EMPTY_PATH If pathname is an empty string, operate on the file referred to by dirfd (which may have been obtained using the open(2) O_PATH flag). In this case, dirfd can refer to any type of file, not just a directory. If dirfd is AT_FDCWD, the call operates on the current working directory. . . . Look out for an empty pathname and use the relative fd's inode in the presence of AT_EMPTY_PATH flag before calling internal _getattr(). Fixes: https://tracker.ceph.com/issues/68189 Review with: git show -w Note: path_walk() refactor from ceph#60746 included the core changes required here and thus src/client/Client.cc changes have been removed from the cherry-pick as they conflict at the same time. [1] https://www.man7.org/linux/man-pages/man2/statx.2.html Signed-off-by: Anoop C S <[email protected]> (cherry picked from commit edd7fe7)

mchangir · 2025-09-09T04:54:36Z

src/messages/MClientReply.h

+      if (struct_v >= 8) {
+        decode(optmetadata, p);
+      }


@batrick now that the struct version is bumped up to 8, shouldn't the decode macro at the top also reflect the bumped up version, i.e. DECODE_START(8, p);

Yes, I think so. In this case it's a harmless omission because the compatv did not increase.

Since you found it, would you like to make the change?

batrick added cephfs Ceph File System DNM labels Nov 15, 2024

github-actions bot added common tests labels Nov 15, 2024

batrick force-pushed the i66373-wip branch from 9c6bf3c to 378cbcd Compare November 16, 2024 00:18

github-actions bot added needs-rebase labels Dec 17, 2024

ceph deleted a comment from github-actions bot Dec 18, 2024

batrick force-pushed the i66373-wip branch from 378cbcd to 5a9b587 Compare December 31, 2024 19:09

github-actions bot added build/ops and removed needs-rebase labels Dec 31, 2024

batrick force-pushed the i66373-wip branch from 5a9b587 to c55484d Compare January 3, 2025 21:20

batrick marked this pull request as ready for review January 3, 2025 21:20

batrick requested a review from a team as a code owner January 3, 2025 21:20

github-actions bot added the documentation label Jan 3, 2025

batrick removed the DNM label Jan 3, 2025

batrick changed the title ~~[WIP] mds: case-insensitive directory trees~~ client,mds: case-insensitive directory trees Jan 3, 2025

anthonyeleven reviewed Jan 3, 2025

View reviewed changes

doc/cephfs/charmap.rst Outdated Show resolved Hide resolved

doc/cephfs/charmap.rst Outdated Show resolved Hide resolved

doc/cephfs/charmap.rst Outdated Show resolved Hide resolved

batrick force-pushed the i66373-wip branch 2 times, most recently from 6f9e321 to fc5ff2e Compare January 6, 2025 15:27

adamemerson self-assigned this Jan 6, 2025

batrick force-pushed the i66373-wip branch 5 times, most recently from 5a78761 to 3b7e55e Compare January 6, 2025 19:40

batrick added the needs-review label Jan 7, 2025

batrick added 6 commits February 27, 2025 19:55

tools/cephfs_mirror: do not process maps with fast dispatch

3b79bdb

This acquires a lock which is a no-no in the messenger. Signed-off-by: Patrick Donnelly <[email protected]>

client: indicate maps are acked not processed

2810ed0

Ancillary change: do not do client upkeep after map processing. Signed-off-by: Patrick Donnelly <[email protected]>

osdc/Objecter: convert to ms_dispatch2 for ack

f060ee8

Convert ms_dispatch to ms_dispatch2 to enable indicating that a map message is acknowledged and instead of processed (or deliberately not processed). Signed-off-by: Patrick Donnelly <[email protected]>

mgr: indicate map message is acked instead of unhandled

56cb057

This avoids messages like: 2025-02-18T05:31:17.738+0000 7f5206546640 0 ms_deliver_dispatch: unhandled message 0x5632d05f0700 fsmap(e 9) from mon.0 v2:172.21.3.230:40412/0 Signed-off-by: Patrick Donnelly <[email protected]>

client: skip unexpected command replies

616fdd1

Instead of marking the message as handled, give another component (or Client) a chance to process. Signed-off-by: Patrick Donnelly <[email protected]>

batrick force-pushed the i66373-wip branch from 82cbf95 to 616fdd1 Compare February 28, 2025 00:56

vshankar approved these changes Mar 3, 2025

View reviewed changes

github-project-automation bot moved this from New to Reviewer approved in Ceph-Dashboard Mar 3, 2025

batrick merged commit 048fc68 into ceph:main Mar 3, 2025
11 of 13 checks passed

batrick deleted the i66373-wip branch March 3, 2025 13:31

github-project-automation bot moved this from Reviewer approved to Done in Ceph-Dashboard Mar 3, 2025

batrick mentioned this pull request Mar 3, 2025

squid: client,mds: case-insensitive directory trees #62095

Merged

anoopcs9 mentioned this pull request Mar 4, 2025

client: handle snapped inode reply in add_updated_inode() #61838

Closed

14 tasks

cbodley reviewed Mar 4, 2025

View reviewed changes

rzarzynski mentioned this pull request Mar 17, 2025

dencoding: check struct_v against DECODE_START(v, ...) at compile-time #60159

Merged

14 tasks

anoopcs9 mentioned this pull request Mar 20, 2025

CephFS: smb2.create fails for shares configured with vfs_ceph_new samba-in-kubernetes/sit-test-cases#96

Closed

anoopcs9 mentioned this pull request Mar 20, 2025

squid: client: Handle empty pathnames for ceph_chownat() and ceph_statxat() #61166

Merged

travisn mentioned this pull request Mar 21, 2025

Failing to start the mgr pod due to failing startup and liveness probe in Ceph main and squid-devel rook/rook#15509

Closed

spuiuk removed the wip-spuiuk-tracking Sachin Prabhu - tracking label Aug 21, 2025

mchangir reviewed Sep 9, 2025

View reviewed changes

Comments

Conversation

batrick commented Nov 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

batrick commented Nov 15, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

batrick commented Jan 7, 2025

Uh oh!

batrick commented Feb 28, 2025

Uh oh!

batrick commented Feb 28, 2025

Uh oh!

vshankar commented Mar 3, 2025

Uh oh!

batrick commented Mar 3, 2025

Uh oh!

Uh oh!

cbodley Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

cbodley Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vshankar Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

batrick Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

mchangir Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

batrick Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

batrick commented Nov 15, 2024 •

edited

Loading

cbodley Mar 4, 2025 •

edited

Loading

mchangir Sep 9, 2025 •

edited

Loading