rgw: Make Neorados Available#51084
Closed
adamemerson wants to merge 46 commits intoceph:wip-coro-after-reeffrom
Closed
rgw: Make Neorados Available#51084adamemerson wants to merge 46 commits intoceph:wip-coro-after-reeffrom
adamemerson wants to merge 46 commits intoceph:wip-coro-after-reeffrom
Conversation
Since we'll be iterating quickly, let them have the current semi-stable-ish version. They can switch back to the main line version after it settles down a bit. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Previously we had versions of all the calls that took an int64_t and optional key and namespace. This didn't really offer much benefit and doubled the maintenance burden for changing anything. As such just make the IOContext constructor non-explicit, and make all its mutators return the same IOContext so people can supply them builder-style if they want to. Also don't wrap things in 'optional' when the underlying library doesn't. The callers just end up having to wrap and unwrap with value-or repeatedly. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
This form of completion handling is compatible with C++20 Completions and generally more flexible stuff. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
The lock will continue to be held over the 'dispatch' with C++20 coroutines. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Apparently in some cases we were using references to objects on the caller stack that happen to no longer exist if the caller's stack goes away. C++20 Coroutines tickled this bug. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Use `min` rather than `max` when deciding how much we need to read in `read()`. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Just as a demonstration to see how well they work and how to put things together with them. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Needed to fix coroutine detection under Clang TODO: update boost package sha1 in install-deps.sh when we have packages uploaded Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
In the `destroy_` functions of `CompletionImpl` we were getting the associated allocator after moving out of the handler into the call to `bind_and_forward`. This was triggering a crash on null-pointer access in operations made with `co_composed`. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Since reading CLS values in a friendly way requires calling out to RADOS then decoding the returned structure, make a function to help with that. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Google Test does not support C++ coroutines, so kludge together a test harness that supports coroutines reasonably well. Also add a couple utility functions. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
…utines Signed-off-by: Casey Bodley <cbodley@redhat.com>
We should not be using std::list everywhere, and this is an excellent time to switch. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
If our decoder function returns a tuple of multiple values, flatten it so our signature is `void(error_code, T, U, V)` not `void(error_code, std::tuple<T, U, V>)`. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
And return them to the client by setting the error cod and result in the vector and returning an error from the operation as a whole. Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Since they can be reported now, report them Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
b4466dd to
6eeb2f0
Compare
cbodley
reviewed
Apr 26, 2023
Contributor
cbodley
left a comment
There was a problem hiding this comment.
looks great, but i'd like to find a way to exercise the frontend shutdown path. can you try to do some smoke testing with killall radosgw while a workload like warp or s3tests is running?
cbodley
reviewed
Apr 26, 2023
src/rgw/rgw_asio_frontend.cc
Outdated
Comment on lines
+1136
to
+1137
| ceph::async::io_context_pool& context_pool) | ||
| : impl(new Impl(env, conf, sched_ctx, context_pool)) |
Contributor
There was a problem hiding this comment.
does the frontend still need the io_context_pool, now that AppMain handles the shutdown? it looks like this could just take a reference to io_context
6eeb2f0 to
534a944
Compare
Pull the `io_context` and threads out of `AsioFrontend`, pass in a reference to `io_context_pool` so it can be shut down at `AsioFrontend::join` Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
534a944 to
2f03efc
Compare
Contributor
Author
Ran a half hour warp with a restart of radosgw every second and shutdown seems to work fine. I moved the thread shutdown to just before closing the driver, and this seems to make everything happy. |
cbodley
approved these changes
May 1, 2023
Contributor
|
jenkins test make check |
Contributor
|
odd failure from 'make check':
|
Contributor
|
@adamemerson @cbodley pls push it to |
bd1de90 to
ecb46f1
Compare
|
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
ecb46f1 to
42f22d8
Compare
6790e11 to
39ded6d
Compare
Contributor
Author
|
This is included in #49976 |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plumb
io_contextintoRadosDriverso a Neorados handle can use it.Make
AsioFrontendshare the sameio_contextas the rest of RGW.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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows