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 <[email protected]>

Author: cbodley

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;
 }
 

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;

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;
 }
 

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;
   }