rgw: fix replica log indexing

Fixes: #8251
Previously we were indexing the replica log only by bucket name, even
though we were provided with the bucket instance id. Fix that, and also
add the option to be able to revert to the old behavior. For
radosgw-admin we can do it if --replicalog-index-by-instance=false is
provided. For the replica log REST api we now have the index-by-instance
http param (defaults to true).

Signed-off-by: Yehuda Sadeh <yehuda@inktank.com>

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index 03b51a59685ee018101229254aef00611d3da37c..cc893fd9b509fd0b0d6409800e8369ebc5b2089d 100644 (file)
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -553,10 +553,17 @@ public:
   }
 };
 
-static int init_bucket(string& bucket_name, RGWBucketInfo& bucket_info, rgw_bucket& bucket)
+static int init_bucket(const string& bucket_name, const string& bucket_id,
+                       RGWBucketInfo& bucket_info, rgw_bucket& bucket)
 {
   if (!bucket_name.empty()) {
-    int r = store->get_bucket_info(NULL, bucket_name, bucket_info, NULL);
+    int r;
+    if (bucket_id.empty()) {
+      r = store->get_bucket_info(NULL, bucket_name, bucket_info, NULL);
+    } else {
+      string bucket_instance_id = bucket_name + ":" + bucket_id;
+      r = store->get_bucket_instance_info(NULL, bucket_instance_id, bucket_info, NULL, NULL);
+    }
     if (r < 0) {
       cerr << "could not get bucket info for bucket=" << bucket_name << std::endl;
       return r;
@@ -886,6 +893,8 @@ int main(int argc, char **argv)
 
   int sync_stats = false;
 
+  int replicalog_index_by_instance = true;
+
   uint64_t min_rewrite_size = 4 * 1024 * 1024;
   uint64_t max_rewrite_size = ULLONG_MAX;
   uint64_t min_rewrite_stripe_size = 0;
@@ -1035,6 +1044,8 @@ int main(int argc, char **argv)
      // do nothing
     } else if (ceph_argparse_binary_flag(args, i, &include_all, NULL, "--include-all", (char*)NULL)) {
      // do nothing
+    } else if (ceph_argparse_binary_flag(args, i, &replicalog_index_by_instance, NULL, "--replicalog-index-by-instance", (char*)NULL)) {
+     // do nothing
     } else if (ceph_argparse_witharg(args, i, &val, "--caps", (char*)NULL)) {
       caps = val;
     } else if (ceph_argparse_witharg(args, i, &val, "-i", "--infile", (char*)NULL)) {
@@ -1560,7 +1571,7 @@ int main(int argc, char **argv)
       RGWBucketAdminOp::info(store, bucket_op, f);
     } else {
       RGWBucketInfo bucket_info;
-      int ret = init_bucket(bucket_name, bucket_info, bucket);
+      int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
       if (ret < 0) {
         cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
         return -ret;
@@ -1887,7 +1898,7 @@ next:
 
   if (opt_cmd == OPT_OBJECT_RM) {
     RGWBucketInfo bucket_info;
-    int ret = init_bucket(bucket_name, bucket_info, bucket);
+    int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
     if (ret < 0) {
       cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
       return -ret;
@@ -1911,7 +1922,7 @@ next:
     }
 
     RGWBucketInfo bucket_info;
-    int ret = init_bucket(bucket_name, bucket_info, bucket);
+    int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
     if (ret < 0) {
       cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
       return -ret;
@@ -1943,7 +1954,7 @@ next:
     }
 
     RGWBucketInfo bucket_info;
-    int ret = init_bucket(bucket_name, bucket_info, bucket);
+    int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
     if (ret < 0) {
       cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
       return -ret;
@@ -2037,7 +2048,7 @@ next:
 
   if (opt_cmd == OPT_OBJECT_UNLINK) {
     RGWBucketInfo bucket_info;
-    int ret = init_bucket(bucket_name, bucket_info, bucket);
+    int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
     if (ret < 0) {
       cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
       return -ret;
@@ -2053,7 +2064,7 @@ next:
 
   if (opt_cmd == OPT_OBJECT_STAT) {
     RGWBucketInfo bucket_info;
-    int ret = init_bucket(bucket_name, bucket_info, bucket);
+    int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
     if (ret < 0) {
       cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
       return -ret;
@@ -2337,7 +2348,7 @@ next:
       return -EINVAL;
     }
     RGWBucketInfo bucket_info;
-    int ret = init_bucket(bucket_name, bucket_info, bucket);
+    int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
     if (ret < 0) {
       cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
       return -ret;
@@ -2377,7 +2388,7 @@ next:
       return -EINVAL;
     }
     RGWBucketInfo bucket_info;
-    int ret = init_bucket(bucket_name, bucket_info, bucket);
+    int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
     if (ret < 0) {
       cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
       return -ret;
@@ -2558,14 +2569,14 @@ next:
         return -EINVAL;
       }
       RGWBucketInfo bucket_info;
-      int ret = init_bucket(bucket_name, bucket_info, bucket);
+      int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
       if (ret < 0) {
         cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
         return -ret;
       }
 
       RGWReplicaBucketLogger logger(store);
-      ret = logger.get_bounds(bucket, shard_id, bounds);
+      ret = logger.get_bounds(bucket, shard_id, bounds, replicalog_index_by_instance);
       if (ret < 0)
         return -ret;
     } else { // shouldn't get here
@@ -2609,14 +2620,14 @@ next:
         return -EINVAL;
       }
       RGWBucketInfo bucket_info;
-      int ret = init_bucket(bucket_name, bucket_info, bucket);
+      int ret = init_bucket(bucket_name, bucket_id, bucket_info, bucket);
       if (ret < 0) {
         cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
         return -ret;
       }
 
       RGWReplicaBucketLogger logger(store);
-      ret = logger.delete_bound(bucket, shard_id, daemon_id);
+      ret = logger.delete_bound(bucket, shard_id, daemon_id, replicalog_index_by_instance);
       if (ret < 0)
         return -ret;
     }

diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc
index a36d89de2a06861aa1a6d6f20a5ad624538023fc..266150b869cdf3fccac47149c23c379ccd5ffca2 100644 (file)
--- a/src/rgw/rgw_common.cc
+++ b/src/rgw/rgw_common.cc
@@ -614,6 +614,15 @@ int XMLArgs::get_bool(const char *name, bool *val, bool *exists)
   return get_bool(s, val, exists);
 }
 
+void XMLArgs::get_bool(const char *name, bool *val, bool def_val)
+{
+  bool exists = false;
+  if ((get_bool(name, val, &exists) < 0) ||
+      !exists) {
+    *val = def_val;
+  }
+}
+
 bool verify_bucket_permission(struct req_state *s, int perm)
 {
   if (!s->bucket_acl)

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 9de90dcae278109b93dad54807ea678395766ec2..306f65e06f2a2a28bdc92dcda1b8a770dd042de5 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -252,6 +252,7 @@ class XMLArgs
   string& get(const char *name, bool *exists = NULL);
   int get_bool(const string& name, bool *val, bool *exists);
   int get_bool(const char *name, bool *val, bool *exists);
+  void get_bool(const char *name, bool *val, bool def_val);
 
   /** see if a parameter is contained in this XMLArgs */
   bool exists(const char *name) {

diff --git a/src/rgw/rgw_replica_log.cc b/src/rgw/rgw_replica_log.cc
index d72b6af23c305bf7fdf0b9d56b4ba69939e5481d..a9a3b711ee72beded035ca9fbae0ea91265d9a96 100644 (file)
--- a/src/rgw/rgw_replica_log.cc
+++ b/src/rgw/rgw_replica_log.cc
@@ -135,9 +135,15 @@ RGWReplicaBucketLogger::RGWReplicaBucketLogger(RGWRados *_store) :
   prefix.append(".");
 }
 
-string RGWReplicaBucketLogger::obj_name(const rgw_bucket& bucket, int shard_id)
+string RGWReplicaBucketLogger::obj_name(const rgw_bucket& bucket, int shard_id, bool index_by_instance)
 {
-  string s = prefix + bucket.name;
+  string s;
+
+  if (index_by_instance) {
+    s = prefix + bucket.name + ":" + bucket.bucket_id;
+  } else {
+    s = prefix + bucket.name;
+  }
 
   if (shard_id >= 0) {
     char buf[16];
@@ -153,7 +159,7 @@ int RGWReplicaBucketLogger::update_bound(const rgw_bucket& bucket, int shard_id,
 {
   if (shard_id >= 0 ||
       !BucketIndexShardsManager::is_shards_marker(marker)) {
-    return RGWReplicaLogger::update_bound(obj_name(bucket, shard_id), pool,
+    return RGWReplicaLogger::update_bound(obj_name(bucket, shard_id, true), pool,
                                           daemon_id, marker, time, entries);
   }
 
@@ -171,7 +177,7 @@ int RGWReplicaBucketLogger::update_bound(const rgw_bucket& bucket, int shard_id,
   map<int, string>::iterator iter;
   for (iter = vals.begin(); iter != vals.end(); ++iter) {
     ldout(cct, 20) << "updating bound: bucket=" << bucket << " shard=" << iter->first << " marker=" << marker << dendl;
-    int r = RGWReplicaLogger::update_bound(obj_name(bucket, iter->first), pool,
+    int r = RGWReplicaLogger::update_bound(obj_name(bucket, iter->first, true), pool,
                                           daemon_id, iter->second, time, entries);
     if (r < 0) {
       ldout(cct, 0) << "failed to update bound: bucket=" << bucket << " shard=" << iter->first << " marker=" << marker << dendl;

diff --git a/src/rgw/rgw_replica_log.h b/src/rgw/rgw_replica_log.h
index a9adc9eedbe6ff1ee89ccba9c27df522ed47cdd7..544030a2ae0d363341ae1174c2d3eed4d8855958 100644 (file)
--- a/src/rgw/rgw_replica_log.h
+++ b/src/rgw/rgw_replica_log.h
@@ -98,18 +98,19 @@ class RGWReplicaBucketLogger : private RGWReplicaLogger {
   string pool;
   string prefix;
 
-  string obj_name(const rgw_bucket& bucket, int shard_id);
+  string obj_name(const rgw_bucket& bucket, int shard_id, bool index_by_instance);
+
 public:
   RGWReplicaBucketLogger(RGWRados *_store);
   int update_bound(const rgw_bucket& bucket, int shard_id, const string& daemon_id,
                    const string& marker, const utime_t& time,
                    const list<RGWReplicaItemMarker> *entries);
-  int delete_bound(const rgw_bucket& bucket, int shard_id, const string& daemon_id) {
-    return RGWReplicaLogger::delete_bound(obj_name(bucket, shard_id), pool,
+  int delete_bound(const rgw_bucket& bucket, int shard_id, const string& daemon_id, bool index_by_instance) {
+    return RGWReplicaLogger::delete_bound(obj_name(bucket, shard_id, index_by_instance), pool,
                                           daemon_id);
   }
-  int get_bounds(const rgw_bucket& bucket, int shard_id, RGWReplicaBounds& bounds) {
-    return RGWReplicaLogger::get_bounds(obj_name(bucket, shard_id), pool,
+  int get_bounds(const rgw_bucket& bucket, int shard_id, RGWReplicaBounds& bounds, bool index_by_instance) {
+    return RGWReplicaLogger::get_bounds(obj_name(bucket, shard_id, index_by_instance), pool,
                                         bounds);
   }
 };

diff --git a/src/rgw/rgw_rest_replica_log.cc b/src/rgw/rgw_rest_replica_log.cc
index 0309a2cabd25b56bed87cb46fa109978acec7195..f842d90bff8c3606d0c5143bfebfb79aedd5cece 100644 (file)
--- a/src/rgw/rgw_rest_replica_log.cc
+++ b/src/rgw/rgw_rest_replica_log.cc
@@ -159,21 +159,62 @@ static int bucket_instance_to_bucket(RGWRados *store, string& bucket_instance, r
   return 0;
 }
 
+static int get_bucket_for_bounds(RGWRados *store, XMLArgs& args, rgw_bucket& bucket, bool *index_by_instance)
+{
+  string bucket_instance = args.get("bucket-instance");
+
+  args.get_bool("index-by-instance", index_by_instance, true);
+
+  if (*index_by_instance) {
+    int r;
+    if ((r = bucket_instance_to_bucket(store, bucket_instance, bucket)) < 0)  {
+      return r;
+    }
+  } else {
+    string bucket_name = args.get("bucket");
+
+    if (bucket_name.empty()) {
+      bucket_name = bucket_instance;
+
+      ssize_t pos = bucket_name.find(':');
+      if (pos >= 0) {
+        bucket_name = bucket_name.substr(0, pos);
+      }
+    }
+
+    if (bucket_name.empty()) {
+      dout(5) << " Error - invalid parameter list" << dendl;
+      return -EINVAL;
+    }
+    RGWBucketInfo bucket_info;
+  
+    int r = store->get_bucket_info(NULL, bucket_name, bucket_info, NULL, NULL);
+    if (r < 0) {
+      dout(5) << "could not get bucket info for bucket=" << bucket_name << ": " << cpp_strerror(r) << dendl;
+      return r;
+    }
+
+    bucket = bucket_info.bucket;
+  }
+
+  return 0;
+
+}
+
 void RGWOp_BILog_SetBounds::execute() {
   string bucket_instance = s->info.args.get("bucket-instance"),
          marker = s->info.args.get("marker"),
          time = s->info.args.get("time"),
          daemon_id = s->info.args.get("daemon_id");
 
-  if (bucket_instance.empty() ||
-      marker.empty() ||
+  if (marker.empty() ||
       time.empty() ||
       daemon_id.empty()) {
     dout(5) << "Error - invalid parameter list" << dendl;
     http_ret = -EINVAL;
     return;
   }
-  
+
   utime_t ut;
   
   if (parse_to_utime(time, ut) < 0) {
@@ -189,9 +230,12 @@ void RGWOp_BILog_SetBounds::execute() {
   }
 
   rgw_bucket bucket;
-  if ((http_ret = bucket_instance_to_bucket(store, bucket_instance, bucket)) < 0) 
-    return;
+  bool index_by_instance;
 
+  if ((http_ret = get_bucket_for_bounds(store, s->info.args, bucket, &index_by_instance)) < 0) {
+    return;
+  }
+  
   RGWReplicaBucketLogger rl(store);
   bufferlist bl;
   list<RGWReplicaItemMarker> markers;
@@ -206,10 +250,10 @@ void RGWOp_BILog_SetBounds::execute() {
 
 void RGWOp_BILog_GetBounds::execute() {
   string bucket_instance = s->info.args.get("bucket-instance");
+  rgw_bucket bucket;
+  bool index_by_instance;
 
-  if (bucket_instance.empty()) {
-    dout(5) << " Error - invalid parameter list" << dendl;
-    http_ret = -EINVAL;
+  if ((http_ret = get_bucket_for_bounds(store, s->info.args, bucket, &index_by_instance)) < 0) {
     return;
   }
 
@@ -220,12 +264,11 @@ void RGWOp_BILog_GetBounds::execute() {
     return;
   }
 
-  rgw_bucket bucket;
   if ((http_ret = bucket_instance_to_bucket(store, bucket_instance, bucket)) < 0) 
     return;
 
   RGWReplicaBucketLogger rl(store);
-  http_ret = rl.get_bounds(bucket, shard_id, bounds);
+  http_ret = rl.get_bounds(bucket, shard_id, bounds, index_by_instance);
 }
 
 void RGWOp_BILog_GetBounds::send_response() {
@@ -241,11 +284,10 @@ void RGWOp_BILog_GetBounds::send_response() {
 }
 
 void RGWOp_BILog_DeleteBounds::execute() {
-  string bucket_instance = s->info.args.get("bucket-instance"),
-         daemon_id = s->info.args.get("daemon_id");
+  string bucket_instance = s->info.args.get("bucket-instance");
+  string daemon_id = s->info.args.get("daemon_id");
 
-  if (bucket_instance.empty() ||
-      daemon_id.empty()) {
+  if (daemon_id.empty()) {
     dout(5) << "Error - invalid parameter list" << dendl;
     http_ret = -EINVAL;
     return;
@@ -259,11 +301,14 @@ void RGWOp_BILog_DeleteBounds::execute() {
   }
 
   rgw_bucket bucket;
-  if ((http_ret = bucket_instance_to_bucket(store, bucket_instance, bucket)) < 0) 
+  bool index_by_instance;
+
+  if ((http_ret = get_bucket_for_bounds(store, s->info.args, bucket, &index_by_instance)) < 0) {
     return;
+  }
   
   RGWReplicaBucketLogger rl(store);
-  http_ret = rl.delete_bound(bucket, shard_id, daemon_id);
+  http_ret = rl.delete_bound(bucket, shard_id, daemon_id, index_by_instance);
 }
 
 RGWOp *RGWHandler_ReplicaLog::op_get() {