rgw: switch back to boost::asio for spawn() and yield_context by cbodley · Pull Request #55592 · ceph/ceph

cbodley · 2024-02-15T04:14:56Z

a fork of boost::asio::spawn() was introduced in 2020 with spawn::spawn() from #31580. this fork enabled rgw to customize how the coroutine stacks are allocated in order to avoid stack overflows in frontend request coroutines. this customization was based on a StackAllocator concept from the boost::context library

in boost 1.80, that same StackAllocator overload was added to boost::asio::spawn(), along with other improvements like per-op cancellation. now that boost has everything we need, switch back and drop the spawn submodule

this required switching a lot of async functions from async_completion<> to async_initiate<>. similar changes were necessary to enable the c++20 coroutine token boost::asio::use_awaitable

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

adamemerson

LGTM.

github-actions · 2024-02-20T16:41:32Z

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

cbodley · 2024-02-21T23:06:39Z

i'll wait a while to merge this. it conflicts with things like #55214 and #55051 that we want on squid

cbodley · 2024-02-22T16:39:17Z

did pass qa in https://pulpito.ceph.com/cbodley-2024-02-20_00:28:35-rgw-wip-cbodley-testing-distro-default-smithi/ with rerun https://pulpito.ceph.com/cbodley-2024-02-21_19:22:22-rgw-wip-cbodley-testing-distro-default-smithi/

github-actions · 2024-03-08T18:26:07Z

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

adamemerson · 2024-03-13T19:44:39Z

src/rgw/driver/rados/rgw_pubsub_push.cc

@@ -122,7 +122,6 @@ namespace {
 class Waiter {
  using Signature = void(boost::system::error_code);


I should probably make an attempt to get rid of this class entirely, but that's not urgent.

currently, it is only used to yield a coroutine.
so, for starter, we can remove the mutex+cond var code.

cbodley · 2024-04-19T20:10:59Z

i tried rebasing but saw lots of errors building new d4n stuff. i'll have to work through that too

Signed-off-by: Casey Bodley <cbodley@redhat.com>

the qat async initiator functions were based on async_completion<> and its completion_handler member, but the updated boost::asio::yield_context doesn't provide a completion_handler. switch to the updated async_initate() method which does work with boost::asio::yield_context Signed-off-by: Casey Bodley <cbodley@redhat.com>

Signed-off-by: Casey Bodley <cbodley@redhat.com>

a fork of boost::asio::spawn() was introduced in 2020 with spawn::spawn() from ceph#31580. this fork enabled rgw to customize how the coroutine stacks are allocated in order to avoid stack overflows in frontend request coroutines. this customization was based on a StackAllocator concept from the boost::context library in boost 1.80, that same StackAllocator overload was added to boost::asio::spawn(), along with other improvements like per-op cancellation. now that boost has everything we need, switch back and drop the spawn submodule this required switching a lot of async functions from async_completion<> to async_initiate<>. similar changes were necessary to enable the c++20 coroutine token boost::asio::use_awaitable Signed-off-by: Casey Bodley <cbodley@redhat.com>

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley · 2024-05-13T16:58:09Z

rebased over #56979 and #57083

cbodley · 2024-05-21T17:33:58Z

passed qa in https://pulpito.ceph.com/cbodley-2024-05-14_15:58:02-rgw-wip-boost-asio-spawn-distro-default-smithi/ with rerun https://pulpito.ceph.com/cbodley-2024-05-15_12:34:15-rgw-wip-boost-asio-spawn-distro-default-smithi/

cbodley · 2024-05-21T17:34:01Z

jenkins test api

cbodley · 2024-05-21T17:34:08Z

jenkins test make check

yuvalif · 2024-05-22T11:01:41Z

passed qa in https://pulpito.ceph.com/cbodley-2024-05-14_15:58:02-rgw-wip-boost-asio-spawn-distro-default-smithi/ with rerun https://pulpito.ceph.com/cbodley-2024-05-15_12:34:15-rgw-wip-boost-asio-spawn-distro-default-smithi/

@cbodley the kafka test failure in the above is a regression with this PR (although it is hard to tell as a similar failure existed before).
the issue is reproduced locally consistently (you would need kafka to run onn localhost):

$ MON=1 OSD=1 MDS=0 MGR=0 RGW=1 ../src/vstart.sh -n -d
$ aws --endpoint-url http://localhost:8000 sns create-topic --name=fishtopic --attributes='{"push-endpoint":  kafka://localhost"}'
$ aws --endpoint-url http://localhost:8000 s3 mb s3://fish
$ aws --endpoint-url http://localhost:8000 s3api put-bucket-notification-configuration --bucket fish --notification-configuration='{"TopicConfigurations": [{"Id": "notif1", "TopicArn": "arn:aws:sns:default::fishtopic", "Events": []}]}'
$ head -c 512 </dev/urandom > myfile
$ aws --endpoint-url http://localhost:8000 s3 cp myfile s3://fish

last call will hang.
in case of persistent notification, the call will not hang (as it just pushed the notification to the queue). but the processing of the queue by the notification manager will hang.

cbodley · 2024-05-22T12:16:57Z

src/rgw/driver/rados/rgw_pubsub_push.cc

+          }, token, yield.get_executor());
      l.unlock();
-      init.result.get();


@yuvalif i think the regression is probably due to this change. the unlock() should happen before suspend, so probably needs to be inside the lambda

this change was done here: #54697
and was backported to reef and quincy.
so, we probably need to make this chnage there as well.

somewhat related, we may have an race condition in the http client:
https://github.com/ceph/ceph/blob/main/src/rgw/rgw_http_client.cc#L73
we don't use the lock to protect the "done" flag. maybe this is the reason for: https://tracker.ceph.com/issues/66033 ?

interesting. done is atomic, but it does look like wait() could still race with the finish() on another thread. i wonder why we haven't seen this anywhere else

i pushed a commit cbodley@4eab016 to lock before the done check but test_notification_push_http still fails the same way

it could be that there is another issue. however, isn't it better to fix that regardless?

opened #57632

github-actions bot added build/ops common core rgw tests labels Feb 15, 2024

cbodley force-pushed the wip-boost-asio-spawn branch 3 times, most recently from e1b1277 to 593002f Compare February 15, 2024 14:47

cbodley marked this pull request as ready for review February 15, 2024 15:43

cbodley requested review from a team as code owners February 15, 2024 15:43

cbodley requested a review from adamemerson February 15, 2024 15:43

cbodley force-pushed the wip-boost-asio-spawn branch from 593002f to 6ce8b0b Compare February 15, 2024 15:51

adamemerson approved these changes Feb 15, 2024

View reviewed changes

cbodley added needs-qa wip-cbodley-testing labels Feb 15, 2024

github-actions bot added the needs-rebase label Feb 20, 2024

cbodley mentioned this pull request Feb 20, 2024

tracer/osd/librados/build/rgw: rgw and osd end2end tracing using opentelemetry #52114

Merged

14 tasks

cbodley force-pushed the wip-boost-asio-spawn branch from 6ce8b0b to b3beeec Compare February 20, 2024 17:46

github-actions bot removed the needs-rebase label Feb 20, 2024

cbodley added the do-not-merge label Feb 21, 2024

cbodley removed needs-qa wip-cbodley-testing labels Feb 22, 2024

github-actions bot added the needs-rebase label Mar 8, 2024

adamemerson approved these changes Mar 13, 2024

View reviewed changes

cbodley added 11 commits May 13, 2024 12:13

rgw/d4n: SSDDriver uses async_initiate

e07dcc4

Signed-off-by: Casey Bodley <cbodley@redhat.com>

common/async: SharedMutex uses async_initiate

c2ec0c0

Signed-off-by: Casey Bodley <cbodley@redhat.com>

librados/asio: functions use async_initiate

675df44

Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/pubsub: Waiter uses async_initiate

3618271

Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/http: async_wait() uses async_initiate

c21bdb9

Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/d3n: async_read() uses async_initiate

0651e43

Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/dmclock: async_request() uses async_initiate

d478d66

Signed-off-by: Casey Bodley <cbodley@redhat.com>

rgw/aio: YieldingAioThrottle::async_wait() uses async_initiate

9117352

Signed-off-by: Casey Bodley <cbodley@redhat.com>

spawn: remove spawn submodule

79a6459

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley force-pushed the wip-boost-asio-spawn branch from d448be7 to 79a6459 Compare May 13, 2024 16:57

github-actions bot added core and removed needs-rebase labels May 13, 2024

cbodley added needs-qa and removed TESTED labels May 13, 2024

cbodley added ready-to-merge and removed needs-qa labels May 21, 2024

cbodley merged commit b7328fe into ceph:main May 21, 2024

cbodley deleted the wip-boost-asio-spawn branch May 21, 2024 20:24

cbodley commented May 22, 2024

View reviewed changes

cbodley mentioned this pull request May 22, 2024

rgw/pubsub: Waiter unlocks before suspend #57630

Merged

cbodley mentioned this pull request Sep 11, 2024

squid: rgw/beast: optimize for accept when meeting error in listenning #59708

Closed

liangmingyuanneo mentioned this pull request Oct 5, 2024

squid: rgw: switch back to boost::asio for spawn() and yield_context #60133

Merged

cbodley mentioned this pull request Dec 3, 2024

rgw: base optional_yield on std::optional<> #60911

Closed

14 tasks

		@@ -122,7 +122,6 @@ namespace {
		class Waiter {
		using Signature = void(boost::system::error_code);

Conversation

cbodley commented Feb 15, 2024

Uh oh!

adamemerson left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 20, 2024

Uh oh!

cbodley commented Feb 21, 2024

Uh oh!

cbodley commented Feb 22, 2024

Uh oh!

github-actions bot commented Mar 8, 2024

Uh oh!

adamemerson Mar 13, 2024

Choose a reason for hiding this comment

Uh oh!

yuvalif Apr 21, 2024

Choose a reason for hiding this comment

Uh oh!

cbodley commented Apr 19, 2024

Uh oh!

cbodley commented May 13, 2024

Uh oh!

cbodley commented May 21, 2024

Uh oh!

cbodley commented May 21, 2024

Uh oh!

cbodley commented May 21, 2024

Uh oh!

yuvalif commented May 22, 2024

Uh oh!

cbodley May 22, 2024

Choose a reason for hiding this comment

Uh oh!

yuvalif May 22, 2024

Choose a reason for hiding this comment

Uh oh!

cbodley May 22, 2024

Choose a reason for hiding this comment

Uh oh!

yuvalif May 22, 2024

Choose a reason for hiding this comment

Uh oh!

cbodley May 22, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants