From e274e1097013b5a1f3da5ed04ecf6f98d9b3aed7 Mon Sep 17 00:00:00 2001 From: Yehuda Sadeh Date: Wed, 7 May 2014 16:19:56 -0700 Subject: [PATCH] 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 --- src/rgw/rgw_admin.cc | 39 +++++++++++------ src/rgw/rgw_common.cc | 9 ++++ src/rgw/rgw_common.h | 1 + src/rgw/rgw_replica_log.cc | 14 ++++-- src/rgw/rgw_replica_log.h | 11 ++--- src/rgw/rgw_rest_replica_log.cc | 77 ++++++++++++++++++++++++++------- 6 files changed, 112 insertions(+), 39 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 03b51a59685ee..cc893fd9b509f 100644 --- 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 a36d89de2a068..266150b869cdf 100644 --- 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 9de90dcae2781..306f65e06f2a2 100644 --- 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 d72b6af23c305..a9a3b711ee72b 100644 --- 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::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 a9adc9eedbe6f..544030a2ae0d3 100644 --- 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 *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 0309a2cabd25b..f842d90bff8c3 100644 --- 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 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() { -- 2.39.5