rgw/admin: Fix assert on datalog list of invalid shard by adamemerson · Pull Request #62770 · ceph/ceph

adamemerson · 2025-04-10T21:07:24Z

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

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 test classic perf Jenkins Job | Jenkins Job Definition
jenkins test crimson perf Jenkins Job | Jenkins Job Definition
jenkins test signed Jenkins Job | Jenkins Job Definition
jenkins test make check Jenkins Job | Jenkins Job Definition
jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
jenkins test submodules Jenkins Job | Jenkins Job Definition
jenkins test dashboard Jenkins Job | Jenkins Job Definition
jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
jenkins test api Jenkins Job | Jenkins Job Definition
jenkins test docs ReadTheDocs | Github Workflow Definition
jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
jenkins test windows Jenkins Job | Jenkins Job Definition
jenkins test rook e2e Jenkins Job | Jenkins Job Definition

src/rgw/driver/rados/rgw_datalog.h

cbodley

i like the run_coro() stuff 👍

src/rgw/driver/rados/rgw_datalog.cc

src/common/async/blocked_completion.h

src/common/async/bind_handler.h

src/librados/librados_util..h

src/rgw/async_utils.h

cbodley · 2025-04-25T14:34:26Z

https://jenkins.ceph.com/job/ceph-pull-requests-arm64/73027/

run-cli-tests .............................***Failed

https://tracker.ceph.com/issues/71027

cbodley · 2025-04-25T14:34:30Z

jenkins test make check arm64

github-actions · 2025-05-26T14:32:20Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

NitzanMordhai · 2025-07-01T08:44:43Z

Rados approved: tracker.ceph.com/projects/rados/wiki/MAIN#httpstrackercephcomissues71678

NitzanMordhai · 2025-07-01T08:45:51Z

jenkins test make check arm64

SrinivasaBharath · 2025-07-02T05:32:26Z

jenkins test make check arm64

cbodley · 2025-07-02T17:02:19Z

thanks @NitzanMordhai @SrinivasaBharath for testing, but please don't merge this until it's been through the rgw suite

adamemerson · 2025-07-11T12:57:35Z

jenkins test make check arm64

ivancich · 2025-07-15T15:18:49Z

This seems to have failed a QA run involving DataLogBulky.TestSemBulky.

2025-07-15T06:55:03.021 INFO:tasks.workunit.client.0.smithi081.stdout:[ RUN ] DataLogBulky.TestSemBulky
2025-07-15T06:55:06.034 INFO:tasks.workunit.client.0.smithi081.stdout:unknown file: Failure
2025-07-15T06:55:06.035 INFO:tasks.workunit.client.0.smithi081.stdout:C++ exception with description "1 is not a valid shard. Valid shards are integers in [0, 1): Invalid argument [generic:22]" thrown in the test body.
2025-07-15T06:55:06.035 INFO:tasks.workunit.client.0.smithi081.stdout:
2025-07-15T06:55:06.035 INFO:tasks.workunit.client.0.smithi081.stdout:[ FAILED ] DataLogBulky.TestSemBulky (3014 ms)

see: https://qa-proxy.ceph.com/teuthology/anuchaithra-2025-07-15_05:47:12-rgw-wip-anrao1-testing-2025-07-14-1609-distro-default-smithi/8388018/teuthology.log

adamemerson · 2025-07-16T19:52:41Z

@anrao19 Fixed! Could you give this another go, please?

anrao19 · 2025-07-17T06:10:38Z

@anrao19 Fixed! Could you give this another go, please?

sure @adamemerson

anrao19 · 2025-07-17T06:11:21Z

jenkins test make check arm64

anrao19 · 2025-07-22T05:29:48Z

@adamemerson , please find the result : https://pulpito.ceph.com/?branch=wip-anrao3-testing-2025-07-17-1157 there are some failures

Fixes: https://tracker.ceph.com/issues/70882 Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

github-actions · 2025-08-06T18:17:03Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

I had been thinking of list and trim as purely internal interfaces, but they are called through HTTP and thus need to be prepared for bad input. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Didn't include `associated_cancellation_slot.hpp`. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Asio does not have nearly as many actual explicit concepts one can use as one might like. And there's no reason we might not want our own asynchrony-related concepts. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Reimplement with `initiate` rather than the old style. This necessitates getting rid of the old `async::Completion` in anything that was calling it, and other changes. Also, use disposition for error handling. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

A convenience function for turning coroutines that return values and use exceptions, `error_code`, or similar into `int`-returning functions that take references to out parameters. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

This avoids having two entry points with different error checking preparation, etc. to get out of sync or have a fix get forgotten. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

Easier to debug. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

adamemerson · 2025-08-07T14:40:28Z

jenkins test api

adamemerson · 2025-08-07T14:40:33Z

jenkins test make check arm64

idryomov · 2025-08-21T19:45:15Z

src/mon/MonClient.cc

  while (!version_requests.empty()) {
-    ceph::async::post(std::move(version_requests.begin()->second),
-		      monc_errc::session_reset, 0, 0);
+    asio::dispatch(asio::append(std::move(version_requests.begin()->second),


I think the switch from post() to dispatch() here is a bug -- it can cause the MonClient to lock up. I have proposed a fix in #65177.

adamemerson requested a review from a team as a code owner April 10, 2025 21:07

github-actions bot added the rgw label Apr 10, 2025

adamemerson requested a review from cbodley April 11, 2025 16:12

cbodley reviewed Apr 11, 2025

View reviewed changes

src/rgw/driver/rados/rgw_datalog.h Outdated Show resolved Hide resolved

adamemerson force-pushed the wip-70882 branch from a5747f0 to a284699 Compare April 18, 2025 16:48

adamemerson requested a review from a team as a code owner April 18, 2025 16:48

github-actions bot added common core mon tests labels Apr 18, 2025

cbodley reviewed Apr 18, 2025

View reviewed changes

adamemerson force-pushed the wip-70882 branch from a284699 to a7d7c3c Compare April 24, 2025 22:07

cbodley approved these changes Apr 25, 2025

View reviewed changes

cbodley added the needs-qa label May 1, 2025

github-actions bot added the needs-rebase label May 26, 2025

adamemerson force-pushed the wip-70882 branch from a7d7c3c to 0e74bb3 Compare June 12, 2025 21:58

github-actions bot removed the needs-rebase label Jun 12, 2025

SrinivasaBharath added the wip-bharath11-testing label Jun 15, 2025

NitzanMordhai added the ready-to-merge label Jul 1, 2025

cbodley removed the ready-to-merge label Jul 2, 2025

adamemerson force-pushed the wip-70882 branch from 0e74bb3 to 321edc5 Compare July 3, 2025 19:20

SrinivasaBharath added TESTED and removed wip-bharath11-testing labels Jul 5, 2025

anrao19 added the wip-anrao1-testing label Jul 14, 2025

anrao19 removed the wip-anrao1-testing label Jul 16, 2025

adamemerson force-pushed the wip-70882 branch from 321edc5 to 32015cd Compare July 16, 2025 19:44

anrao19 added the wip-anrao3-testing label Jul 17, 2025

anrao19 removed the wip-anrao3-testing label Jul 22, 2025

anrao19 removed the needs-qa label Aug 5, 2025

rgw/admin: Fix assert on datalog list of invalid shard

b84d7e3

Fixes: https://tracker.ceph.com/issues/70882 Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

github-actions bot added the needs-rebase label Aug 6, 2025

adamemerson added 7 commits August 6, 2025 14:29

rgw/multi: Fix error handling in public Datalog APIs

9d4968b

I had been thinking of list and trim as purely internal interfaces, but they are called through HTTP and thus need to be prepared for bad input. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

common/async/co_waiter: Add missing header

26ef486

Didn't include `associated_cancellation_slot.hpp`. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

common/async: Add concepts.h

07c77b0

Asio does not have nearly as many actual explicit concepts one can use as one might like. And there's no reason we might not want our own asynchrony-related concepts. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Add run_coro utility

9f17b63

A convenience function for turning coroutines that return values and use exceptions, `error_code`, or similar into `int`-returning functions that take references to out parameters. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw: Use run_coro to call coroutines at use

4732944

This avoids having two entry points with different error checking preparation, etc. to get out of sync or have a fix get forgotten. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

rgw/datalog: Stick noexcept on wrappers returning int

bb93aa3

Easier to debug. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

adamemerson force-pushed the wip-70882 branch from 32015cd to bb93aa3 Compare August 6, 2025 20:57

github-actions bot removed the needs-rebase label Aug 6, 2025

adamemerson removed the mon label Aug 7, 2025

adamemerson merged commit 3f96a23 into ceph:main Aug 7, 2025
13 checks passed

idryomov mentioned this pull request Aug 21, 2025

mon/MonClient: post version request completions outside of monc_lock #65177

Merged

14 tasks

idryomov reviewed Aug 21, 2025

View reviewed changes

Conversation

adamemerson commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

Uh oh!

cbodley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cbodley commented Apr 25, 2025

Uh oh!

cbodley commented Apr 25, 2025

Uh oh!

github-actions bot commented May 26, 2025

Uh oh!

NitzanMordhai commented Jul 1, 2025

Uh oh!

NitzanMordhai commented Jul 1, 2025

Uh oh!

SrinivasaBharath commented Jul 2, 2025

Uh oh!

cbodley commented Jul 2, 2025

Uh oh!

adamemerson commented Jul 11, 2025

Uh oh!

ivancich commented Jul 15, 2025

Uh oh!

adamemerson commented Jul 16, 2025

Uh oh!

anrao19 commented Jul 17, 2025

Uh oh!

anrao19 commented Jul 17, 2025

Uh oh!

anrao19 commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 6, 2025

Uh oh!

adamemerson commented Aug 7, 2025

Uh oh!

adamemerson commented Aug 7, 2025

Uh oh!

Uh oh!

idryomov Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

adamemerson commented Apr 10, 2025 •

edited

Loading

anrao19 commented Jul 22, 2025 •

edited

Loading