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 <yehuda@redhat.com>

src/cls/rgw/cls_rgw_client.h

@@ -150,24 +150,21 @@ public:
   }
 
   void to_string(string *out) const {
-    if (out) {
-      map<int, string>::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<int, string>::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<string> shards;
-      get_str_list(composed_marker, SHARDS_SEPARATOR.c_str(), shards);
-      list<string>::const_iterator iter = shards.begin();
-      for (; iter != shards.end(); ++iter) {
-        size_t pos = iter->find(KEY_VALUE_SEPARATOR);
-        if (pos == string::npos) {
+    vector<string> shards;
+    get_str_vec(composed_marker, SHARDS_SEPARATOR.c_str(), shards);
+    if (shards.size() > 1 && shard_id >= 0) {
+      return -EINVAL;
+    }
+    vector<string>::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;
   }

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<rgw_bi_log_entry>& 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<string> shard_ids_str;
-  vector<list<rgw_bi_log_entry>::iterator> vcurrents;
-  vector<list<rgw_bi_log_entry>::iterator> vends;
+  map<int, list<rgw_bi_log_entry>::iterator> vcurrents;
+  map<int, list<rgw_bi_log_entry>::iterator> vends;
   if (truncated) {
     *truncated = false;
   }
   map<int, cls_rgw_bi_log_list_ret>::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<int, list<rgw_bi_log_entry>::iterator>::iterator viter;
+  map<int, list<rgw_bi_log_entry>::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<rgw_bi_log_entry>::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;
 

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;

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<string, string> response_attrs;
   map<string, string>::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;