Skip to content

squid: pybind/cephfs, mgr/volumes: introduce non-recursive rmtree(), refactor purge() to use it and add MDS optimizations#65817

Open
rishabh-d-dave wants to merge 10 commits intoceph:squidfrom
rishabh-d-dave:wip-73392-squid
Open

squid: pybind/cephfs, mgr/volumes: introduce non-recursive rmtree(), refactor purge() to use it and add MDS optimizations#65817
rishabh-d-dave wants to merge 10 commits intoceph:squidfrom
rishabh-d-dave:wip-73392-squid

Conversation

@rishabh-d-dave
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave commented Oct 7, 2025 •
edited
Loading

@rishabh-d-dave
pybind/cephfs, mgr/volumes: refactor purge() to be non-recursive
9fa0e8e 
Method purge() in trash.py calls rmtree() which is recursive method. To
avoid Python's recurision limit, switch to non-recursive approach.

Path to directory along directory handle are clubbed in to a tuple and
that tuple is stored on the stack. Storing directory handle reduces call
to opendir() dramatically.

Fixes: https://tracker.ceph.com/issues/71648
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit f9046ca)
@rishabh-d-dave
test/pybind: add unit tests for rmtree() in cephfs python bindings
99c7c56 
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 05082a9)
@rishabh-d-dave rishabh-d-dave added this to the squid milestone Oct 7, 2025
@github-actions github-actions bot added cephfs Ceph File System common tests labels Oct 7, 2025
@joscollin
Copy link
Member

joscollin commented Oct 8, 2025

@rishabh-d-dave
Looks like this is related:

Undefined reference in Client.cc:(.text+0x5696f) to `filepath::get_trimmed_path[abi:cxx11]() const'

@rishabh-d-dave rishabh-d-dave force-pushed the wip-73392-squid branch 2 times, most recently from d3109fd to 898cce6 Compare October 8, 2025 03:22
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Oct 8, 2025

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Oct 8, 2025

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Oct 8, 2025

jenkins test docs

@rishabh-d-dave rishabh-d-dave force-pushed the wip-73392-squid branch 2 times, most recently from 7afc5fa to 1b0103d Compare October 8, 2025 12:07
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Oct 9, 2025

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Oct 9, 2025

jenkins test windows

@joscollin
Copy link
Member

joscollin commented Oct 13, 2025

jenkins test make check

@joscollin
Copy link
Member

joscollin commented Oct 16, 2025

This PR is under test in https://tracker.ceph.com/issues/73560.

@rishabh-d-dave
qa, test: run unit tests for cephfs.pyx with non-root user
7666ced 
Run test_python.sh with non-root user. This makes it necessary to change
the owner user and group of file system root to be same as this non-root
user. This brings testing closer to the real-world scenario and also
allows exercising negative tests where an FS op would fail for a non-root
user but it would pass for root user.

There are few tests that exercise FS operations where root user is
needed. Group these tests under a separate class and add extra code for
this class that allows these tests to run with root UID and GID.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 6021dda)

Conflicts:
src/test/pybind/test_cephfs.py
- PR ceph#64999 is merged in main but not in Squid, causing this branch to
  have less tests that need to be moved to class TestWithRootUser.
@rishabh-d-dave
mds: for logging generate only 10 final components of inode path
f7bbbd3 
Generating full absolute path for inodes for printing in MDS logs slows
down the FS to a great extent especially when the path is very long
(imagine a path with 2000 components). Also printing such long paths in
MDS logs is not only pointless but also greatly reduces the readability
of the MDS logs.

Therefore, generate only 10 final components of inode paths for logging.

Fixes: https://tracker.ceph.com/issues/72779
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 1518690)

Conflicts:
src/mds/CDir.cc
- Certain code region where trimmed inode path was generated was
  modified to generated inode path but that code region is absent on
  this branch.
@rishabh-d-dave
mds: for logging generate only 10 final components of dentry path
41e9633 
Generating full absolute path for dentries for printing in MDS logs
slows the down the FS to a great extent especially when the path is very
long (imagine a path with 2000 components). Printing such long paths in
MDS logs is not only pointless but also greatly reduces the readability
of MDS logs.

Therefore, generate only 10 final components of the dentry paths for logging.

Fixes: https://tracker.ceph.com/issues/72779
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 1430cd6)

Conflicts:
src/include/filepath.cc
- Unlike main branch, this file is absent in squid
@rishabh-d-dave
mds: log trimmed path wherever generating full path is necessary
b550e43 
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 11de1e5)

Conflicts:
src/mds/MDSAuthCaps.cc
src/test/mds/TestMDSAuthCaps.cc
- is_capable takes one less argument in Squid compared to main branch
  version.

src/mds/Server.cc
src/mds/SessionMap.cc
-  There is lesser including of other header files in both these files
   leading to difficulty in patch application in this region.
@rishabh-d-dave
client: trim path before logging it
8123a93 
Path can be virtually infinitely long and logging a long long path
(imagine around 2000 path components) is un-useful as well as lowers
readability of the log. Therefore, trim before logging.

Fixes: https://tracker.ceph.com/issues/72993
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit bdc8aae)

Conflicts:
src/include/filepath.cc
src/include/filepath.h
- Unlink main branch, filepath.cc is absent in this branch. Therefore,
  the changes must be moved to filepath.h.
@rishabh-d-dave
mds: fix few log entries
604f0ff 
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e4c301b)

Conflicts:
src/mds/MDSAuthCaps.cc
- is_capable()'s log message is slightly different in Squid branch
  leading to conflict.
@rishabh-d-dave
mds/ScrubStack: avoid generating inode path since it is unused
8f542b8 
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit 10e4ccb)
@rishabh-d-dave
src/common: add helper to prepend "..." to trimmed paths
9d90fd4 
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit c38a913)
@joscollin
Copy link
Member

joscollin commented Oct 16, 2025

jenkins test api

@rishabh-d-dave rishabh-d-dave changed the title squid: pybind/cephfs, mgr/volumes: introduce non-recurisve rmtree(), refactor purge() to use it and add MDS optimizations squid: pybind/cephfs, mgr/volumes: introduce non-recursive rmtree(), refactor purge() to use it and add MDS optimizations Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cephfs Ceph File System common pybind tests wip-jcollin-testing-squid11

Projects

None yet

Milestone

squid

Development

Successfully merging this pull request may close these issues.

2 participants

@rishabh-d-dave @joscollin