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 <cbodley@redhat.com>

diff --git a/src/rgw/driver/rados/rgw_period.cc b/src/rgw/driver/rados/rgw_period.cc
index 4a16faccefb3a83bbee513229f857c98564a444d..f18e8e46bc5f0e3b19fb165e3e72d654ac2afda1 100644 (file)
--- 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 91381d7729541912245663407ba1a851e5c687be..881819237eb1afddc4757f0a6ca3fc4a9cc1ab6e 100644 (file)
--- 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 <optional>
 #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<RGWRealm> 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 2c504712381c52785e3018c3e78f6641d2a70e3f..1e9361c888913bb6e9ffad784e769718e1ce57bb 100644 (file)
--- 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 61f5477d2d5ba9f59a6469b37fb67738038e5189..89cf4f29f672508375274acf2262c13526c23d13 100644 (file)
--- 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;
   }