rgw: call update_latest_epoch() on all period updates

when updating the period, callers use the atomic result of
update_latest_epoch() to determine whether they need to call
RGWPeriod::reflect() and RGWRealm::notify_new_period()

this adds a missing call to RGWPeriod::reflect() to RGWPeriodPuller,
which was previously not safe to do without atomic updates to
latest_epoch

Fixes: http://tracker.ceph.com/issues/19817
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit d825b9d854864b815890cf9204d7e72e1e1a9ada)

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index 60596e25507bd6a274634cb3472dc37bb99fb74a..08116d4506e24e6512eb0b849eafc154d48f64a7 100644 (file)
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -1639,7 +1639,8 @@ static int do_period_pull(const string& remote, const string& url, const string&
   if (ret < 0) {
     cerr << "Error storing period " << period->get_id() << ": " << cpp_strerror(ret) << std::endl;
   }
-
+  // store latest epoch (ignore errors)
+  period->update_latest_epoch(period->get_epoch());
   return 0;
 }
 

diff --git a/src/rgw/rgw_period_puller.cc b/src/rgw/rgw_period_puller.cc
index b76f73e4a53b3a5dc745af78609e998e622cd0af..2c811a358ba1a4f492168097218b8282d884cd24 100644 (file)
--- a/src/rgw/rgw_period_puller.cc
+++ b/src/rgw/rgw_period_puller.cc
@@ -83,17 +83,22 @@ int RGWPeriodPuller::pull(const std::string& period_id, RGWPeriod& period)
       lderr(store->ctx()) << "failed to store period " << period_id << dendl;
       return r;
     }
-    // XXX: if this is a newer epoch, we should overwrite the existing
-    // latest_epoch. but there's no way to do that atomically
-    bool exclusive = true;
-    r = period.set_latest_epoch(period.get_epoch(), exclusive);
+    // update latest epoch
+    r = period.update_latest_epoch(period.get_epoch());
     if (r == -EEXIST) {
-      r = 0;
-    } else if (r < 0) {
+      // already have this epoch (or a more recent one)
+      return 0;
+    }
+    if (r < 0) {
       lderr(store->ctx()) << "failed to update latest_epoch for period "
           << period_id << dendl;
       return r;
     }
+    // reflect period objects if this is the latest version
+    r = period.reflect();
+    if (r < 0) {
+      return r;
+    }
     ldout(store->ctx(), 14) << "period " << period_id
         << " pulled and written to local storage" << dendl;
   } else {

diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc
index 091a14c41ba98c23621f55c246fd5366d88a133e..1d0fd5ef09819b32d9e2a8cfc143bfce3d50cd03 100644 (file)
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -1131,6 +1131,7 @@ int RGWPeriod::create(bool exclusive)
   ret = store_info(exclusive);
   if (ret < 0) {
     ldout(cct, 0) << "ERROR:  storing info for " << id << ": " << cpp_strerror(-ret) << dendl;
+    return ret;
   }
 
   ret = set_latest_epoch(epoch);
@@ -1376,7 +1377,11 @@ int RGWPeriod::commit(RGWRealm& realm, const RGWPeriod& current_period,
     return r;
   }
   // set as latest epoch
-  r = set_latest_epoch(epoch);
+  r = update_latest_epoch(epoch);
+  if (r == -EEXIST) {
+    // already have this epoch (or a more recent one)
+    return 0;
+  }
   if (r < 0) {
     ldout(cct, 0) << "failed to set latest epoch: " << cpp_strerror(-r) << dendl;
     return r;

diff --git a/src/rgw/rgw_rest_realm.cc b/src/rgw/rgw_rest_realm.cc
index fb1ba2f10e73569d05a6a0b3a2ab84e5279b47a2..fd32512421a2e8500b3043ee1e783c987a231ed4 100644 (file)
--- a/src/rgw/rgw_rest_realm.cc
+++ b/src/rgw/rgw_rest_realm.cc
@@ -142,6 +142,19 @@ void RGWOp_Period_Post::execute()
     lderr(cct) << "failed to store period " << period.get_id() << dendl;
     return;
   }
+  // set as latest epoch
+  http_ret = period.update_latest_epoch(period.get_epoch());
+  if (http_ret == -EEXIST) {
+    // already have this epoch (or a more recent one)
+    ldout(cct, 4) << "already have epoch >= " << period.get_epoch()
+        << " for period " << period.get_id() << dendl;
+    http_ret = 0;
+    return;
+  }
+  if (http_ret < 0) {
+    lderr(cct) << "failed to set latest epoch" << dendl;
+    return;
+  }
 
   // decide whether we can set_current_period() or set_latest_epoch()
   if (period.get_id() != current_period.get_id()) {
@@ -192,22 +205,9 @@ void RGWOp_Period_Post::execute()
     realm.notify_new_period(period);
     return;
   }
-
-  if (period.get_epoch() <= current_period.get_epoch()) {
-    lderr(cct) << "period epoch " << period.get_epoch() << " is not newer "
-        "than current epoch " << current_period.get_epoch()
-        << ", discarding update" << dendl;
-    return;
-  }
-  // set as latest epoch
-  http_ret = period.set_latest_epoch(period.get_epoch());
-  if (http_ret < 0) {
-    lderr(cct) << "failed to set latest epoch" << dendl;
-    return;
-  }
   // reflect the period into our local objects
-  http_ret  = period.reflect();
-  if (http_ret  < 0) {
+  http_ret = period.reflect();
+  if (http_ret < 0) {
     lderr(cct) << "failed to update local objects: "
         << cpp_strerror(-http_ret) << dendl;
     return;