mgr/vol: handle case where clone index entry goes missing by rishabh-d-dave · Pull Request #58101 · ceph/ceph

rishabh-d-dave · 2024-06-17T19:13:11Z

In async_cloner.py, clone index entry is fetched to get next clone job
that needs to be executed. It might happen that the clone job was
cancelled just when it was going to be picked for execution (IOW, when
it was about to move from pending state to in-progress state).

Currently, in such a case MGR hangs because the exception ObjectNotFound
from CephFS Python bindings is raised and is left uncaught. To prevent
this issue catch an exception, log it and return None to tell get_job()
of async_job.py to look for next job in the queue.

Fixes: https://tracker.ceph.com/issues/66560

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

rishabh-d-dave · 2024-06-18T06:48:57Z

jenkins test make check

rishabh-d-dave · 2024-06-18T06:49:01Z

jenkins test api

rishabh-d-dave · 2024-06-18T06:49:10Z

jenkins test make check arm64

rishabh-d-dave · 2024-06-19T04:38:19Z

jenkins test make check

rishabh-d-dave · 2024-06-19T04:38:39Z

jenkins test make check arm64

src/pybind/mgr/volumes/fs/async_cloner.py

rishabh-d-dave · 2024-06-20T11:33:14Z

jenkins test make check

rishabh-d-dave · 2024-06-20T11:33:37Z

jenkins test make check arm64

rishabh-d-dave · 2024-06-20T11:33:47Z

jenkins test api

src/pybind/mgr/volumes/fs/operations/clone_index.py

vshankar

otherwise lgtm.

src/pybind/mgr/volumes/fs/async_cloner.py

rishabh-d-dave · 2024-06-25T06:42:02Z

@vshankar I've squashed the commits and made the change you had suggested. It's ready for review. PTAL.

src/pybind/mgr/volumes/fs/operations/clone_index.py

rishabh-d-dave · 2024-06-25T15:19:52Z

@vshankar Just a reminder, please re-review.

rishabh-d-dave · 2024-06-26T14:25:15Z

jenkins test make check

src/pybind/mgr/volumes/fs/operations/clone_index.py

rishabh-d-dave

https://tracker.ceph.com/projects/cephfs/wiki/Main#11-JUL-2024

Testing took some time because first there were issues with building testing branch, then there were too many infra failures, there were failures due to one of the PRs in the testing branch and finally last run's rerun had new failures that need to be checked.

There were no failures related to this PR, QA was successful.

rishabh-d-dave · 2024-07-11T11:08:11Z

jenkins test make check

rishabh-d-dave · 2024-07-11T11:09:04Z

jenkins test api

* refs/pull/58101/head: mgr/vol: handle case where clone index entry goes missing Reviewed-by: Venky Shankar <vshankar@redhat.com> Reviewed-by: Kotresh Hiremath Ravishankar <khiremat@redhat.com>

rishabh-d-dave · 2024-07-11T12:33:30Z

jenkins test api

rishabh-d-dave · 2024-07-11T13:50:47Z

I am seeing make check failures that seem unrelated - https://jenkins.ceph.com/job/ceph-pull-requests/138712/consoleFull#-1640523647e840cee4-f4a4-4183-81dd-42855615f2c1. Copying them below -

Identified `/home/jenkins-build/build/workspace/ceph-pull-requests` as project root containing a .git directory.
Found input source directory: "/home/jenkins-build/build/workspace/ceph-pull-requests/src/cephadm/cephadmlib"

Aborted!
check-black: exit 1 (0.45 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/src/cephadm> black --check -l78 -t py36 --skip-string-normalization -v cephadmlib/ pid=3832823
check-black: FAIL ✖ in 11.96 seconds

check-black: commands[0] /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr> black --check -q -l78 -t py36 --skip-string-normalization smb

Aborted!
check-black: exit 1 (0.27 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr> black --check -q -l78 -t py36 --skip-string-normalization smb pid=3951087
check-black: FAIL ✖ in 4.17 seconds

Rebasing the PR...

In `async_cloner.py`, clone index entry is fetched to get next clone job that needs to be executed. It might happen that the clone job was cancelled just when it was going to be picked for execution (IOW, when it was about to move from pending state to in-progress state). Currently, MGR hangs in such a case because exception `ObjectNotFound` from CephFS Python bindings is raised and is left uncaught. To prevent this issue catch the exception, log it and return None to tell `get_job()` of `async_job.py` to look for next job in the queue. Increase the scope of try-except in method `get_oldest_clone_entry()` of `async_cloner.py` so that when exception `cephfs.Error` or any exception under it is thrown by `self.fs.lstat()` is not left uncaught. FS object is also passed to the method `list_one_entry_at_a_time()`, so increasing scope of try-except is useful as it will not allow exceptions raised in other calls to CephFS Python binding methods to be left uncaught. Fixes: https://tracker.ceph.com/issues/66560 Signed-off-by: Rishabh Dave <ridave@redhat.com>

rishabh-d-dave · 2024-07-11T18:56:05Z

make check passed by rebasing PR branch. But API test keep different error each time it is run.

rishabh-d-dave · 2024-07-11T18:56:34Z

jenkins test api

rishabh-d-dave · 2024-07-12T03:54:56Z

CI passed, proceeding to merge. Leaving link to the comment where QA run reported - #58101 (review)

mgr/vol: handle case where clone index entry goes missing Reviewed-by: Venky Shankar <vshankar@redhat.com> Reviewed-by: Kotresh HR <khiremat@redhat.com>

rishabh-d-dave added cephfs Ceph File System needs-review labels Jun 17, 2024

github-actions bot added the pybind label Jun 17, 2024

rishabh-d-dave requested review from a team and vshankar June 17, 2024 19:13

rishabh-d-dave force-pushed the async-clone-race branch from 774ecc4 to 4ff6272 Compare June 17, 2024 19:20

vshankar reviewed Jun 19, 2024

View reviewed changes

src/pybind/mgr/volumes/fs/async_cloner.py Outdated Show resolved Hide resolved

rishabh-d-dave force-pushed the async-clone-race branch from 4ff6272 to 9323413 Compare June 19, 2024 07:06

vshankar requested changes Jun 20, 2024

View reviewed changes

src/pybind/mgr/volumes/fs/async_cloner.py Outdated Show resolved Hide resolved

rishabh-d-dave requested a review from vshankar June 20, 2024 10:28

rishabh-d-dave commented Jun 20, 2024

View reviewed changes

src/pybind/mgr/volumes/fs/operations/clone_index.py Outdated Show resolved Hide resolved

vshankar requested changes Jun 24, 2024

View reviewed changes

src/pybind/mgr/volumes/fs/async_cloner.py Outdated Show resolved Hide resolved

rishabh-d-dave force-pushed the async-clone-race branch from 3f45b6b to 9f5e79f Compare June 25, 2024 06:28

rishabh-d-dave requested a review from vshankar June 25, 2024 06:29

vshankar reviewed Jun 25, 2024

View reviewed changes

src/pybind/mgr/volumes/fs/operations/clone_index.py Outdated Show resolved Hide resolved

rishabh-d-dave force-pushed the async-clone-race branch from 9f5e79f to 9ef9e17 Compare June 25, 2024 06:58

vshankar approved these changes Jun 25, 2024

View reviewed changes

kotreshhr reviewed Jun 27, 2024

View reviewed changes

src/pybind/mgr/volumes/fs/operations/clone_index.py Outdated Show resolved Hide resolved

kotreshhr requested changes Jun 27, 2024

View reviewed changes

rishabh-d-dave force-pushed the async-clone-race branch 2 times, most recently from a67d75c to aa0f6ed Compare June 27, 2024 20:29

kotreshhr approved these changes Jun 28, 2024

View reviewed changes

rishabh-d-dave added the wip-rishabh-testing Rishabh's testing label label Jun 28, 2024

rishabh-d-dave commented Jul 11, 2024

View reviewed changes

rishabh-d-dave force-pushed the async-clone-race branch from aa0f6ed to 3cff725 Compare July 11, 2024 16:33

rishabh-d-dave merged commit e392142 into ceph:main Jul 12, 2024

rishabh-d-dave mentioned this pull request Jul 12, 2024

qa/cephfs: fix test_single_path_authorize_on_nonalphanumeric_fsname #58311

Merged

14 tasks

rishabh-d-dave deleted the async-clone-race branch July 12, 2024 06:01

This was referenced Jul 12, 2024

reef: mgr/vol: handle case where clone index entry goes missing #58556

Merged

squid: mgr/vol: handle case where clone index entry goes missing #58557

Merged

quincy: mgr/vol: handle case where clone index entry goes missing #58558

Merged

Conversation

rishabh-d-dave commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

rishabh-d-dave commented Jun 18, 2024

Uh oh!

rishabh-d-dave commented Jun 18, 2024

Uh oh!

rishabh-d-dave commented Jun 18, 2024

Uh oh!

rishabh-d-dave commented Jun 19, 2024

Uh oh!

rishabh-d-dave commented Jun 19, 2024

Uh oh!

Uh oh!

Uh oh!

rishabh-d-dave commented Jun 20, 2024

Uh oh!

rishabh-d-dave commented Jun 20, 2024

Uh oh!

rishabh-d-dave commented Jun 20, 2024

Uh oh!

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rishabh-d-dave commented Jun 25, 2024

Uh oh!

Uh oh!

rishabh-d-dave commented Jun 25, 2024

Uh oh!

rishabh-d-dave commented Jun 26, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rishabh-d-dave left a comment • 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!

rishabh-d-dave commented Jul 11, 2024

Uh oh!

rishabh-d-dave commented Jul 11, 2024

Uh oh!

rishabh-d-dave commented Jul 11, 2024

Uh oh!

rishabh-d-dave commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rishabh-d-dave commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rishabh-d-dave commented Jul 11, 2024

Uh oh!

rishabh-d-dave commented Jul 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rishabh-d-dave commented Jun 17, 2024 •

edited

Loading

rishabh-d-dave left a comment •

edited

Loading

rishabh-d-dave commented Jul 11, 2024 •

edited

Loading

rishabh-d-dave commented Jul 11, 2024 •

edited

Loading