osdc/Objecter: reimplement list_objects

Return to caller at the end of each PG.  This allows the caller to look at
the [pg_]hash_position and get something meaningful.

If there are no objects in the PG, we skip it so that every callback has
*some* data (unless the pool is totally empty!).  So the real difference
here is that we don't move on to the next PG just to reach max_entries.

This gives the client some data sooner, but may mean more callbacks into
client code.

Signed-off-by: Sage Weil <sage@inktank.com>

3 files changed, 54 insertions, 46 deletions

diff --git a/src/librados/IoCtxImpl.cc b/src/librados/IoCtxImpl.cc
index aaa987316a3..6d35f6ba6f4 100644
--- a/src/librados/IoCtxImpl.cc
+++ b/src/librados/IoCtxImpl.cc
@@ -370,7 +370,7 @@ int librados::IoCtxImpl::list(Objecter::ListContext *context, int max_entries)
   object_t oid;
   Mutex mylock("IoCtxImpl::list::mylock");
 
-  if (context->at_end)
+  if (context->at_end())
     return 0;
 
   context->max_entries = max_entries;
diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 3f057b662ce..f767641a0f7 100644
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -1717,15 +1717,27 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m)
 void Objecter::list_objects(ListContext *list_context, Context *onfinish) {
 
   ldout(cct, 10) << "list_objects" << dendl;
-  ldout(cct, 20) << "pool_id " << list_context->pool_id
-	   << "\npool_snap_seq " << list_context->pool_snap_seq
-	   << "\nmax_entries " << list_context->max_entries
-	   << "\nlist_context " << list_context
-	   << "\nonfinish " << onfinish
-	   << "\nlist_context->current_pg" << list_context->current_pg
-	   << "\nlist_context->cookie" << list_context->cookie << dendl;
-
-  if (list_context->at_end) {
+  ldout(cct, 20) << " pool_id " << list_context->pool_id
+	   << " pool_snap_seq " << list_context->pool_snap_seq
+	   << " max_entries " << list_context->max_entries
+	   << " list_context " << list_context
+	   << " onfinish " << onfinish
+	   << " list_context->current_pg " << list_context->current_pg
+	   << " list_context->cookie " << list_context->cookie << dendl;
+
+  if (list_context->at_end_of_pg) {
+    list_context->at_end_of_pg = false;
+    ++list_context->current_pg;
+    list_context->current_pg_epoch = 0;
+    list_context->cookie = collection_list_handle_t();
+    if (list_context->current_pg >= list_context->starting_pg_num) {
+      list_context->at_end_of_pool = true;
+      ldout(cct, 20) << " no more pgs; reached end of pool" << dendl;
+    } else {
+      ldout(cct, 20) << " move to next pg " << list_context->current_pg << dendl;
+    }
+  }
+  if (list_context->at_end_of_pool) {
     onfinish->complete(0);
     return;
   }
@@ -1739,16 +1751,13 @@ void Objecter::list_objects(ListContext *list_context, Context *onfinish) {
   }
   if (list_context->starting_pg_num != pg_num) {
     // start reading from the beginning; the pgs have changed
-    ldout(cct, 10) << "The placement groups have changed, restarting with " << pg_num << dendl;
+    ldout(cct, 10) << " pg_num changed; restarting with " << pg_num << dendl;
     list_context->current_pg = 0;
     list_context->cookie = collection_list_handle_t();
     list_context->current_pg_epoch = 0;
     list_context->starting_pg_num = pg_num;
   }
-  if (list_context->current_pg == pg_num){ //this context got all the way through
-    onfinish->complete(0);
-    return;
-  }
+  assert(list_context->current_pg <= pg_num);
 
   ObjectOperation op;
   op.pg_ls(list_context->max_entries, list_context->filter, list_context->cookie,
@@ -1775,52 +1784,44 @@ void Objecter::_list_reply(ListContext *list_context, int r,
   list_context->cookie = response.handle;
   if (!list_context->current_pg_epoch) {
     // first pgls result, set epoch marker
-    ldout(cct, 20) << "first pgls piece, reply_epoch is " << reply_epoch << dendl;
+    ldout(cct, 20) << " first pgls piece, reply_epoch is "
+		   << reply_epoch << dendl;
     list_context->current_pg_epoch = reply_epoch;
   }
 
   int response_size = response.entries.size();
-  ldout(cct, 20) << "response.entries.size " << response_size
-	   << ", response.entries " << response.entries << dendl;
+  ldout(cct, 20) << " response.entries.size " << response_size
+		 << ", response.entries " << response.entries << dendl;
   list_context->extra_info.append(extra_info);
   if (response_size) {
-    ldout(cct, 20) << "got a response with objects, proceeding" << dendl;
     list_context->list.merge(response.entries);
-    if (response_size >= list_context->max_entries) {
-      final_finish->complete(0);
-      return;
-    }
-
-    // ask for fewer objects next time around
-    list_context->max_entries -= response_size;
+  }
 
-    // if the osd returns 1 (newer code), or no entries, it means we
-    // hit the end of the pg.
-    if (r == 0) {
-      // not yet done with this pg
+  // if the osd returns 1 (newer code), or no entries, it means we
+  // hit the end of the pg.
+  if (response_size == 0 || r == 1) {
+    ldout(cct, 20) << " at end of pg" << dendl;
+    list_context->at_end_of_pg = true;
+  } else {
+    // there is more for this pg; get it?
+    if (response_size < list_context->max_entries) {
+      list_context->max_entries -= response_size;
       list_objects(list_context, final_finish);
       return;
     }
   }
-
-  // if we make this this far, there are no objects left in the current pg, but we want more!
-  ++list_context->current_pg;
-  list_context->current_pg_epoch = 0;
-  ldout(cct, 20) << "emptied current pg, moving on to next one:" << list_context->current_pg << dendl;
-  if (list_context->current_pg < list_context->starting_pg_num){ // we have more pgs to go through
-    list_context->cookie = collection_list_handle_t();
-    list_objects(list_context, final_finish);
+  if (!list_context->list.empty()) {
+    ldout(cct, 20) << " returning results so far" << dendl;
+    final_finish->complete(0);
     return;
   }
-  
-  // if we make it this far, there are no more pgs
-  ldout(cct, 20) << "out of pgs, returning to" << final_finish << dendl;
-  list_context->at_end = true;
-  final_finish->complete(0);
-  return;
+
+  // continue!
+  list_objects(list_context, final_finish);
 }
 
 
+
 //snapshots
 
 int Objecter::create_pool_snap(int64_t pool, string& snap_name, Context *onfinish)
diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index ca67aff3660..4e6fa77e69b 100644
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -1014,7 +1014,8 @@ public:
     collection_list_handle_t cookie;
     epoch_t current_pg_epoch;
     int starting_pg_num;
-    bool at_end;
+    bool at_end_of_pool;
+    bool at_end_of_pg;
 
     int64_t pool_id;
     int pool_snap_seq;
@@ -1029,9 +1030,15 @@ public:
     bufferlist extra_info;
 
     ListContext() : current_pg(0), current_pg_epoch(0), starting_pg_num(0),
-		    at_end(false), pool_id(0),
+		    at_end_of_pool(false),
+		    at_end_of_pg(false),
+		    pool_id(0),
 		    pool_snap_seq(0), max_entries(0) {}
 
+    bool at_end() const {
+      return at_end_of_pool;
+    }
+
     uint32_t get_pg_hash_position() const {
       return current_pg;
     }