cls_log: adjust listing api

Listing api now also gets end time and a marker.

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

5 files changed, 109 insertions, 25 deletions

diff --git a/src/cls/log/cls_log.cc b/src/cls/log/cls_log.cc
index 6864e82633c..013c27aa5aa 100644
--- a/src/cls/log/cls_log.cc
+++ b/src/cls/log/cls_log.cc
@@ -99,14 +99,25 @@ static int cls_log_list(cls_method_context_t hctx, bufferlist *in, bufferlist *o
 
   map<string, bufferlist> keys;
 
-  string index;
+  string from_index;
+  string to_index;
 
-  get_index_time_prefix(op.from_time, index);
+  if (op.marker.empty()) {
+    get_index_time_prefix(op.from_time, from_index);
+  } else {
+    from_index = op.marker;
+  }
+  bool use_time_boundary = (op.to_time > op.from_time);
+
+  if (use_time_boundary)
+    get_index_time_prefix(op.to_time, to_index);
 
 #define MAX_ENTRIES 1000
-  size_t max_entries = min(MAX_ENTRIES, op.num_entries);
+  size_t max_entries = op.max_entries;
+  if (!max_entries || max_entries > MAX_ENTRIES)
+    max_entries = MAX_ENTRIES;
 
-  int rc = cls_cxx_map_get_vals(hctx, index, log_index_prefix, max_entries + 1, &keys);
+  int rc = cls_cxx_map_get_vals(hctx, from_index, log_index_prefix, max_entries + 1, &keys);
   if (rc < 0)
     return rc;
 
@@ -115,8 +126,18 @@ static int cls_log_list(cls_method_context_t hctx, bufferlist *in, bufferlist *o
   list<cls_log_entry>& entries = ret.entries;
   map<string, bufferlist>::iterator iter = keys.begin();
 
+  bool done = false;
+  string marker;
+
   size_t i;
   for (i = 0; i < max_entries && iter != keys.end(); ++i, ++iter) {
+    const string& index = iter->first;
+    marker = index;
+    if (use_time_boundary && index.compare(0, to_index.size(), to_index) >= 0) {
+      done = true;
+      break;
+    }
+
     bufferlist& bl = iter->second;
     bufferlist::iterator biter = bl.begin();
     try {
@@ -128,7 +149,12 @@ static int cls_log_list(cls_method_context_t hctx, bufferlist *in, bufferlist *o
     }
   }
 
-  ret.truncated = (i < keys.size());
+  if (iter == keys.end())
+    done = true;
+  else
+    ret.marker = marker;
+
+  ret.truncated = !done;
 
   ::encode(ret, *out);
 
diff --git a/src/cls/log/cls_log_client.cc b/src/cls/log/cls_log_client.cc
index a5f44bfbca3..d1c199ba263 100644
--- a/src/cls/log/cls_log_client.cc
+++ b/src/cls/log/cls_log_client.cc
@@ -64,18 +64,23 @@ int cls_log_trim(librados::IoCtx& io_ctx, string& oid, utime_t& from, utime_t& t
 
 class LogListCtx : public ObjectOperationCompletion {
   list<cls_log_entry> *entries;
+  string *marker;
   bool *truncated;
 public:
-  LogListCtx(list<cls_log_entry> *_entries, bool *_truncated) :
-                                      entries(_entries), truncated(_truncated) {}
+  LogListCtx(list<cls_log_entry> *_entries, string *_marker, bool *_truncated) :
+                                      entries(_entries), marker(_marker), truncated(_truncated) {}
   void handle_completion(int r, bufferlist& outbl) {
     if (r >= 0) {
       cls_log_list_ret ret;
       try {
         bufferlist::iterator iter = outbl.begin();
         ::decode(ret, iter);
-	*entries = ret.entries;
-        *truncated = ret.truncated;
+        if (entries)
+	  *entries = ret.entries;
+        if (truncated)
+          *truncated = ret.truncated;
+        if (marker)
+          *marker = ret.marker;
       } catch (buffer::error& err) {
         // nothing we can do about it atm
       }
@@ -83,16 +88,19 @@ public:
   }
 };
 
-void cls_log_list(librados::ObjectReadOperation& op, utime_t& from, int max,
-                  list<cls_log_entry>& entries, bool *truncated)
+void cls_log_list(librados::ObjectReadOperation& op, utime_t& from, utime_t& to,
+                  string& in_marker, int max_entries, list<cls_log_entry>& entries,
+                  string *out_marker, bool *truncated)
 {
   bufferlist inbl;
   cls_log_list_op call;
   call.from_time = from;
-  call.num_entries = max;
+  call.to_time = to;
+  call.marker = in_marker;
+  call.max_entries = max_entries;
 
   ::encode(call, inbl);
 
-  op.exec("log", "list", inbl, new LogListCtx(&entries, truncated));
+  op.exec("log", "list", inbl, new LogListCtx(&entries, out_marker, truncated));
 }
 
diff --git a/src/cls/log/cls_log_client.h b/src/cls/log/cls_log_client.h
index 79cfc459c51..6c0046b26f2 100644
--- a/src/cls/log/cls_log_client.h
+++ b/src/cls/log/cls_log_client.h
@@ -13,8 +13,9 @@ void cls_log_add(librados::ObjectWriteOperation& op, cls_log_entry& entry);
 void cls_log_add(librados::ObjectWriteOperation& op, const utime_t& timestamp,
                  const string& section, const string& name, bufferlist& bl);
 
-void cls_log_list(librados::ObjectReadOperation& op, utime_t& from, int max,
-                  list<cls_log_entry>& entries, bool *truncated);
+void cls_log_list(librados::ObjectReadOperation& op, utime_t& from, utime_t& to,
+                  string& in_marker, int max_entries, list<cls_log_entry>& entries,
+                  string *out_marker, bool *truncated);
 
 void cls_log_trim(librados::ObjectWriteOperation& op, utime_t& from, utime_t& to);
 int cls_log_trim(librados::IoCtx& io_ctx, string& oid, utime_t& from, utime_t& to);
diff --git a/src/cls/log/cls_log_ops.h b/src/cls/log/cls_log_ops.h
index 28a7dc8184e..6dc457ed5fd 100644
--- a/src/cls/log/cls_log_ops.h
+++ b/src/cls/log/cls_log_ops.h
@@ -28,21 +28,28 @@ WRITE_CLASS_ENCODER(cls_log_add_op)
 
 struct cls_log_list_op {
   utime_t from_time;
-  int num_entries;
+  string marker; /* if not empty, overrides from_time */
+  utime_t to_time; /* not inclusive */
+  int max_entries; /* upperbound to returned num of entries
+                      might return less than that and still be truncated */
 
   cls_log_list_op() {}
 
   void encode(bufferlist& bl) const {
     ENCODE_START(1, 1, bl);
     ::encode(from_time, bl);
-    ::encode(num_entries, bl);
+    ::encode(marker, bl);
+    ::encode(to_time, bl);
+    ::encode(max_entries, bl);
     ENCODE_FINISH(bl);
   }
 
   void decode(bufferlist::iterator& bl) {
     DECODE_START(1, bl);
     ::decode(from_time, bl);
-    ::decode(num_entries, bl);
+    ::decode(marker, bl);
+    ::decode(to_time, bl);
+    ::decode(max_entries, bl);
     DECODE_FINISH(bl);
   }
 };
@@ -50,6 +57,7 @@ WRITE_CLASS_ENCODER(cls_log_list_op)
 
 struct cls_log_list_ret {
   list<cls_log_entry> entries;
+  string marker;
   bool truncated;
 
   cls_log_list_ret() : truncated(false) {}
@@ -57,6 +65,7 @@ struct cls_log_list_ret {
   void encode(bufferlist& bl) const {
     ENCODE_START(1, 1, bl);
     ::encode(entries, bl);
+    ::encode(marker, bl);
     ::encode(truncated, bl);
     ENCODE_FINISH(bl);
   }
@@ -64,15 +73,21 @@ struct cls_log_list_ret {
   void decode(bufferlist::iterator& bl) {
     DECODE_START(1, bl);
     ::decode(entries, bl);
+    ::decode(marker, bl);
     ::decode(truncated, bl);
     DECODE_FINISH(bl);
   }
 };
 WRITE_CLASS_ENCODER(cls_log_list_ret)
 
+
+/*
+ * operation will return 0 when successfully removed but not done. Will return
+ * -ENODATA when done, so caller needs to repeat sending request until that.
+ */
 struct cls_log_trim_op {
   utime_t from_time;
-  utime_t to_time;
+  utime_t to_time; /* inclusive */
 
   cls_log_trim_op() {}
 
diff --git a/src/test/cls_log/test_cls_log.cc b/src/test/cls_log/test_cls_log.cc
index a45349b75ea..d79fa01064c 100644
--- a/src/test/cls_log/test_cls_log.cc
+++ b/src/test/cls_log/test_cls_log.cc
@@ -136,7 +136,11 @@ TEST(cls_rgw, test_log_add_same_time)
 
   /* check list */
 
-  cls_log_list(*rop, start_time, 1000, entries, &truncated);
+  utime_t to_time = get_time(start_time, 1, true);
+
+  string marker;
+
+  cls_log_list(*rop, start_time, to_time, marker, 0, entries, &marker, &truncated);
 
   bufferlist obl;
   ASSERT_EQ(0, ioctx.operate(oid, rop, &obl));
@@ -177,7 +181,9 @@ TEST(cls_rgw, test_log_add_same_time)
 
   /* check list again, now want to be truncated*/
 
-  cls_log_list(*rop, start_time, 1, entries, &truncated);
+  marker.clear();
+
+  cls_log_list(*rop, start_time, to_time, marker, 1, entries, &marker, &truncated);
 
   ASSERT_EQ(0, ioctx.operate(oid, rop, &obl));
 
@@ -216,9 +222,13 @@ TEST(cls_rgw, test_log_add_different_time)
   list<cls_log_entry> entries;
   bool truncated;
 
+  utime_t to_time = utime_t(start_time.sec() + 10, start_time.nsec());
+
+  string marker;
+
   /* check list */
 
-  cls_log_list(*rop, start_time, 1000, entries, &truncated);
+  cls_log_list(*rop, start_time, to_time, marker, 0, entries, &marker, &truncated);
 
   bufferlist obl;
   ASSERT_EQ(0, ioctx.operate(oid, rop, &obl));
@@ -251,14 +261,35 @@ TEST(cls_rgw, test_log_add_different_time)
 
   utime_t next_time = get_time(start_time, 1, true);
 
-  cls_log_list(*rop, next_time, 10, entries, &truncated);
+  marker.clear();
+
+  cls_log_list(*rop, next_time, to_time, marker, 0, entries, &marker, &truncated);
 
   ASSERT_EQ(0, ioctx.operate(oid, rop, &obl));
 
   ASSERT_EQ(9, (int)entries.size());
   ASSERT_EQ(0, (int)truncated);
 
-  delete rop;
+  reset_rop(&rop);
+
+  marker.clear();
+
+  i = 0;
+  do {
+    bufferlist obl;
+    string old_marker = marker;
+    cls_log_list(*rop, start_time, to_time, old_marker, 1, entries, &marker, &truncated);
+
+    ASSERT_EQ(0, ioctx.operate(oid, rop, &obl));
+    ASSERT_NE(old_marker, marker);
+    ASSERT_EQ(1, (int)entries.size());
+
+    ++i;
+    ASSERT_GE(10, i);
+  } while (truncated);
+
+  ASSERT_EQ(10, i);
+
 }
 
 TEST(cls_rgw, test_log_trim)
@@ -293,6 +324,7 @@ TEST(cls_rgw, test_log_trim)
   /* check list */
 
   /* trim */
+  utime_t to_time = get_time(start_time, 10, true);
 
   for (int i = 0; i < 10; i++) {
     utime_t trim_time = get_time(start_time, i, true);
@@ -301,7 +333,9 @@ TEST(cls_rgw, test_log_trim)
 
     ASSERT_EQ(0, cls_log_trim(ioctx, oid, zero_time, trim_time));
 
-    cls_log_list(*rop, start_time, 1000, entries, &truncated);
+    string marker;
+
+    cls_log_list(*rop, start_time, to_time, marker, 0, entries, &marker, &truncated);
 
     bufferlist obl;
     ASSERT_EQ(0, ioctx.operate(oid, rop, &obl));