From dbaa1420c40092e26b26da6b3175d026983e32a3 Mon Sep 17 00:00:00 2001 From: Yehuda Sadeh Date: Thu, 15 Jan 2015 17:30:24 -0800 Subject: [PATCH] rgw: bilog marker related fixes Fix the way we parse the marker. Instead of specifying whether it's a sharded or not sharded bucket, we pass a shard_id. If string itself points to a singe shard, we'll use the passed shard_id, otherwise we'll parse the string and determine the shard id by that. In this way when referencing a single shard we can get the marker with either shard id specified or not. This works with the non-shard case too. Adjust the bilog listing function, set it to work with the new interface. It was broken before, and there are multiple fixes to it. Signed-off-by: Yehuda Sadeh --- src/cls/rgw/cls_rgw_client.h | 81 +++++++++++++++++++++--------------- src/rgw/rgw_rados.cc | 73 ++++++++++++++++++-------------- src/rgw/rgw_replica_log.cc | 2 +- src/rgw/rgw_rest_swift.cc | 9 +--- 4 files changed, 92 insertions(+), 73 deletions(-) diff --git a/src/cls/rgw/cls_rgw_client.h b/src/cls/rgw/cls_rgw_client.h index e0c527094b7..79de35825ef 100644 --- a/src/cls/rgw/cls_rgw_client.h +++ b/src/cls/rgw/cls_rgw_client.h @@ -150,24 +150,21 @@ public: } void to_string(string *out) const { - if (out) { - map::const_iterator iter = value_by_shards.begin(); - // No shards - if (value_by_shards.size() == 1) { - *out = iter->second; - } else { - for (; iter != value_by_shards.end(); ++iter) { - if (out->length()) { - // Not the first item, append a separator first - out->append(SHARDS_SEPARATOR); - } - char buf[16]; - snprintf(buf, sizeof(buf), "%d", iter->first); - out->append(buf); - out->append(KEY_VALUE_SEPARATOR); - out->append(iter->second); - } + if (!out) { + return; + } + out->clear(); + map::const_iterator iter = value_by_shards.begin(); + for (; iter != value_by_shards.end(); ++iter) { + if (out->length()) { + // Not the first item, append a separator first + out->append(SHARDS_SEPARATOR); } + char buf[16]; + snprintf(buf, sizeof(buf), "%d", iter->first); + out->append(buf); + out->append(KEY_VALUE_SEPARATOR); + out->append(iter->second); } } @@ -175,27 +172,45 @@ public: return marker.find(KEY_VALUE_SEPARATOR) != string::npos; } - int from_string(const string& composed_marker, bool has_shards) { + /* + * convert from string. There are two options of how the string looks like: + * + * 1. Single shard, no shard id specified, e.g. 000001.23.1 + * + * for this case, if passed shard_id >= 0, use this shard id, otherwise assume that it's a + * bucket with no shards. + * + * 2. One or more shards, shard id specified for each shard, e.g., 0#00002.12,1#00003.23.2 + * + */ + int from_string(const string& composed_marker, int shard_id) { value_by_shards.clear(); - if (!has_shards) { - add(0, composed_marker); - } else { - list shards; - get_str_list(composed_marker, SHARDS_SEPARATOR.c_str(), shards); - list::const_iterator iter = shards.begin(); - for (; iter != shards.end(); ++iter) { - size_t pos = iter->find(KEY_VALUE_SEPARATOR); - if (pos == string::npos) { + vector shards; + get_str_vec(composed_marker, SHARDS_SEPARATOR.c_str(), shards); + if (shards.size() > 1 && shard_id >= 0) { + return -EINVAL; + } + vector::const_iterator iter = shards.begin(); + for (; iter != shards.end(); ++iter) { + size_t pos = iter->find(KEY_VALUE_SEPARATOR); + if (pos == string::npos) { + if (!value_by_shards.empty()) { return -EINVAL; } - string shard_str = iter->substr(0, pos); - string err; - int shard = (int)strict_strtol(shard_str.c_str(), 10, &err); - if (!err.empty()) { - return -EINVAL; + if (shard_id < 0) { + add(0, *iter); + } else { + add(shard_id, *iter); } - value_by_shards[shard] = iter->substr(pos + 1); + return 0; } + string shard_str = iter->substr(0, pos); + string err; + int shard = (int)strict_strtol(shard_str.c_str(), 10, &err); + if (!err.empty()) { + return -EINVAL; + } + add(shard, iter->substr(pos + 1)); } return 0; } diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index 5f91704db02..164363f8303 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -6174,7 +6174,7 @@ int RGWRados::list_raw_objects(rgw_bucket& pool, const string& prefix_filter, int RGWRados::list_bi_log_entries(rgw_bucket& bucket, int shard_id, string& marker, uint32_t max, std::list& result, bool *truncated) { - ldout(cct, 20) << __func__ << bucket << " marker " << marker << " max " << max << dendl; + ldout(cct, 20) << __func__ << ": " << bucket << " marker " << marker << " shard_id=" << shard_id << " max " << max << dendl; result.clear(); librados::IoCtx index_ctx; @@ -6191,7 +6191,7 @@ int RGWRados::list_bi_log_entries(rgw_bucket& bucket, int shard_id, string& mark // should have the pattern '{shard_id_1}#{shard_marker_1},{shard_id_2}# // {shard_marker_2}...', if there is no sharding, the bi_log_list should // only contain one record, and the key is the bucket instance id. - r = marker_mgr.from_string(marker, has_shards); + r = marker_mgr.from_string(marker, shard_id); if (r < 0) return r; @@ -6200,50 +6200,60 @@ int RGWRados::list_bi_log_entries(rgw_bucket& bucket, int shard_id, string& mark return r; vector shard_ids_str; - vector::iterator> vcurrents; - vector::iterator> vends; + map::iterator> vcurrents; + map::iterator> vends; if (truncated) { *truncated = false; } map::iterator miter = bi_log_lists.begin(); for (; miter != bi_log_lists.end(); ++miter) { int shard_id = miter->first; - char buf[16]; - snprintf(buf, sizeof(buf), "%d", shard_id); - shard_ids_str.push_back(buf); - vcurrents.push_back(miter->second.entries.begin()); - vends.push_back(miter->second.entries.end()); + vcurrents[shard_id] = miter->second.entries.begin(); + vends[shard_id] = miter->second.entries.end(); if (truncated) { *truncated = (*truncated || miter->second.truncated); } } + size_t total = 0; bool has_more = true; - while (result.size() < max && has_more) { + map::iterator>::iterator viter; + map::iterator>::iterator eiter; + while (total < max && has_more) { has_more = false; - for (size_t i = 0; - result.size() < max && i < vcurrents.size() && vcurrents[i] != vends[i]; - ++vcurrents[i], ++i) { - string& shard_str = shard_ids_str[i]; - if (vcurrents[i] != vends[i]) { - rgw_bi_log_entry& entry = *(vcurrents[i]); - if (has_shards) { - // Put the shard name as part of the ID, so that caller can easy find out - // the next marker - string tmp_id; - build_bucket_index_marker(shard_str, entry.id, &tmp_id); - entry.id.swap(tmp_id); - } - marker_mgr.add(i, entry.id); - result.push_back(entry); - has_more = true; + + viter = vcurrents.begin(); + eiter = vends.begin(); + + for (; total < max && viter != vcurrents.end(); ++viter, ++eiter) { + assert (eiter != vends.end()); + + int shard_id = viter->first; + list::iterator& liter = viter->second; + + if (liter == eiter->second){ + continue; } + rgw_bi_log_entry& entry = *(liter); + if (has_shards) { + char buf[16]; + snprintf(buf, sizeof(buf), "%d", shard_id); + string tmp_id; + build_bucket_index_marker(buf, entry.id, &tmp_id); + entry.id.swap(tmp_id); + } + marker_mgr.add(shard_id, entry.id); + result.push_back(entry); + total++; + has_more = true; + ++liter; } } - for (size_t i = 0; i < vcurrents.size(); ++i) { - if (truncated) { - *truncated = (*truncated || (vcurrents[i] != vends[i])); + if (truncated) { + for (viter = vcurrents.begin(), eiter = vends.begin(); viter != vcurrents.end(); ++viter, ++eiter) { + assert (eiter != vends.end()); + *truncated = (*truncated || (viter->second != eiter->second)); } } @@ -6269,13 +6279,12 @@ int RGWRados::trim_bi_log_entries(rgw_bucket& bucket, int shard_id, string& star if (r < 0) return r; - bool has_shards = bucket_objs.size() > 1 || shard_id >= 0; BucketIndexShardsManager start_marker_mgr; - r = start_marker_mgr.from_string(start_marker, has_shards); + r = start_marker_mgr.from_string(start_marker, shard_id); if (r < 0) return r; BucketIndexShardsManager end_marker_mgr; - r = end_marker_mgr.from_string(end_marker, has_shards); + r = end_marker_mgr.from_string(end_marker, shard_id); if (r < 0) return r; diff --git a/src/rgw/rgw_replica_log.cc b/src/rgw/rgw_replica_log.cc index 02c32d3554f..d72b6af23c3 100644 --- a/src/rgw/rgw_replica_log.cc +++ b/src/rgw/rgw_replica_log.cc @@ -158,7 +158,7 @@ int RGWReplicaBucketLogger::update_bound(const rgw_bucket& bucket, int shard_id, } BucketIndexShardsManager sm; - int ret = sm.from_string(marker, true); + int ret = sm.from_string(marker, shard_id); if (ret < 0) { ldout(cct, 0) << "ERROR: could not parse shards marker: " << marker << dendl; return ret; diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index 960cf9c3d02..20ac37e9990 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -559,7 +559,6 @@ void RGWCopyObj_ObjStore_SWIFT::send_response() int RGWGetObj_ObjStore_SWIFT::send_response_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) { const char *content_type = NULL; - int orig_ret = ret; map response_attrs; map::iterator riter; @@ -601,11 +600,7 @@ int RGWGetObj_ObjStore_SWIFT::send_response_data(bufferlist& bl, off_t bl_ofs, o } } - if (partial_content && !ret) - ret = -STATUS_PARTIAL_CONTENT; - - if (ret) - set_req_state_err(s, ret); + set_req_state_err(s, (partial_content && !ret) ? STATUS_PARTIAL_CONTENT : ret); dump_errno(s); for (riter = response_attrs.begin(); riter != response_attrs.end(); ++riter) { @@ -619,7 +614,7 @@ int RGWGetObj_ObjStore_SWIFT::send_response_data(bufferlist& bl, off_t bl_ofs, o sent_header = true; send_data: - if (get_data && !orig_ret) { + if (get_data && !ret) { int r = s->cio->write(bl.c_str() + bl_ofs, bl_len); if (r < 0) return r;