From: Casey Bodley <cbodley@redhat.com>
Date: Tue, 2 May 2017 16:16:45 +0000 (-0400)
Subject: rgw: call update_latest_epoch() on all period updates
X-Git-Tag: v12.1.1~21^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d825b9d854864b815890cf9204d7e72e1e1a9ada;p=ceph.git

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

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index 5a65eb5a5150..e3b862d02e1e 100644
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -1848,7 +1848,8 @@ static int do_period_pull(RGWRESTConn *remote_conn, const string& url,
   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 b76f73e4a53b..2c811a358ba1 100644
--- 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 c83eb51339e2..3cbf27316a8c 100644
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -1300,6 +1300,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);
@@ -1571,7 +1572,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 0f5abcdfe053..7289d1389526 100644
--- a/src/rgw/rgw_rest_realm.cc
+++ b/src/rgw/rgw_rest_realm.cc
@@ -140,6 +140,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()) {
@@ -190,22 +203,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;