From: Casey Bodley Date: Wed, 29 May 2024 16:40:10 +0000 (-0400) Subject: rgw/period: move realm notify out of period commit X-Git-Tag: v20.0.0~1509^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ab30392209740ceeae349eb1020a5f075d5d55e6;p=ceph.git rgw/period: move realm notify out of period commit require the callers to trigger the realm notify manually. RGWOp_Period_Post takes advantage of this to delay the realm notify until after it's sent its response to the client. this avoids racing with realm reload which will close the connection commit returns EEXIST errors directly so the caller can avoid the realm notify Signed-off-by: Casey Bodley --- diff --git a/src/rgw/driver/rados/rgw_period.cc b/src/rgw/driver/rados/rgw_period.cc index 4a16faccefb3..f18e8e46bc5f 100644 --- a/src/rgw/driver/rados/rgw_period.cc +++ b/src/rgw/driver/rados/rgw_period.cc @@ -271,7 +271,6 @@ int RGWPeriod::commit(const DoutPrefixProvider *dpp, } ldpp_dout(dpp, 4) << "Promoted to master zone and committed new period " << id << dendl; - realm.notify_new_period(dpp, *this, y); return 0; } // period must be based on current epoch @@ -295,10 +294,6 @@ int RGWPeriod::commit(const DoutPrefixProvider *dpp, } // set as latest epoch r = update_latest_epoch(dpp, epoch, y); - if (r == -EEXIST) { - // already have this epoch (or a more recent one) - return 0; - } if (r < 0) { ldpp_dout(dpp, 0) << "failed to set latest epoch: " << cpp_strerror(-r) << dendl; return r; @@ -310,7 +305,6 @@ int RGWPeriod::commit(const DoutPrefixProvider *dpp, } ldpp_dout(dpp, 4) << "Committed new epoch " << epoch << " for period " << id << dendl; - realm.notify_new_period(dpp, *this, y); return 0; } diff --git a/src/rgw/driver/rados/rgw_rest_realm.cc b/src/rgw/driver/rados/rgw_rest_realm.cc index 91381d772954..881819237eb1 100644 --- a/src/rgw/driver/rados/rgw_rest_realm.cc +++ b/src/rgw/driver/rados/rgw_rest_realm.cc @@ -1,6 +1,7 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab ft=cpp +#include #include "common/errno.h" #include "rgw_rest_realm.h" #include "rgw_rest_s3.h" @@ -81,14 +82,17 @@ void RGWOp_Period_Get::execute(optional_yield y) // POST /admin/realm/period class RGWOp_Period_Post : public RGWOp_Period_Base { + std::optional notify_realm; public: void execute(optional_yield y) override; + void send_response() override; int check_caps(const RGWUserCaps& caps) override { return caps.check_cap("zone", RGW_CAP_WRITE); } int verify_permission(optional_yield) override { return check_caps(s->user->get_caps()); } + const char* name() const override { return "post_period"; } RGWOpType get_type() override { return RGW_OP_PERIOD_POST; } }; @@ -139,9 +143,15 @@ void RGWOp_Period_Post::execute(optional_yield y) // if period id is empty, handle as 'period commit' if (period.get_id().empty()) { op_ret = period.commit(this, driver, realm, current_period, error_stream, y); + if (op_ret == -EEXIST) { + op_ret = 0; // succeed on retries so the op is idempotent + return; + } if (op_ret < 0) { ldpp_dout(this, -1) << "master zone failed to commit period" << dendl; + return; } + notify_realm = std::move(realm); // trigger realm reload return; } @@ -221,7 +231,7 @@ void RGWOp_Period_Post::execute(optional_yield y) ldpp_dout(this, 4) << "period " << period.get_id() << " is newer than current period " << current_period.get_id() << ", updating realm's current period and notifying zone" << dendl; - realm.notify_new_period(this, period, y); + notify_realm = std::move(realm); // trigger realm reload return; } // reflect the period into our local objects @@ -234,11 +244,22 @@ void RGWOp_Period_Post::execute(optional_yield y) ldpp_dout(this, 4) << "period epoch " << period.get_epoch() << " is newer than current epoch " << current_period.get_epoch() << ", updating period's latest epoch and notifying zone" << dendl; - realm.notify_new_period(this, period, y); + notify_realm = std::move(realm); // trigger realm reload // update the period history period_history->insert(RGWPeriod{period}); } +void RGWOp_Period_Post::send_response() +{ + RGWOp_Period_Base::send_response(); + + if (notify_realm) { + // trigger realm reload after sending the response, because reload may + // race to close this connection + notify_realm->notify_new_period(this, period, s->yield); + } +} + class RGWHandler_Period : public RGWHandler_Auth_S3 { protected: using RGWHandler_Auth_S3::RGWHandler_Auth_S3; diff --git a/src/rgw/driver/rados/rgw_zone.cc b/src/rgw/driver/rados/rgw_zone.cc index 2c504712381c..1e9361c88891 100644 --- a/src/rgw/driver/rados/rgw_zone.cc +++ b/src/rgw/driver/rados/rgw_zone.cc @@ -769,7 +769,6 @@ int commit_period(const DoutPrefixProvider* dpp, optional_yield y, } ldpp_dout(dpp, 4) << "Promoted to master zone and committed new period " << info.id << dendl; - (void) cfgstore->realm_notify_new_period(dpp, y, info); return 0; } // period must be based on current epoch @@ -788,10 +787,6 @@ int commit_period(const DoutPrefixProvider* dpp, optional_yield y, // write the period constexpr bool exclusive = true; int r = cfgstore->create_period(dpp, y, exclusive, info); - if (r == -EEXIST) { - // already have this epoch (or a more recent one) - return 0; - } if (r < 0) { ldpp_dout(dpp, 0) << "failed to store period: " << cpp_strerror(r) << dendl; return r; @@ -803,7 +798,6 @@ int commit_period(const DoutPrefixProvider* dpp, optional_yield y, } ldpp_dout(dpp, 4) << "Committed new epoch " << info.epoch << " for period " << info.id << dendl; - (void) cfgstore->realm_notify_new_period(dpp, y, info); return 0; } diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 61f5477d2d5b..89cf4f29f672 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -1942,6 +1942,7 @@ static int commit_period(rgw::sal::ConfigStore* cfgstore, if (ret < 0) { cerr << "failed to commit period: " << cpp_strerror(-ret) << std::endl; } + (void) cfgstore->realm_notify_new_period(dpp(), null_yield, period); return ret; }