librados: add omap_get_{keys,vals}2 with pmore output arg

Expose public methods that include a new output argument to indicate
whether there are more keys to fetch or not.

Mark the old interfaces deprecated.

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

diff --git a/PendingReleaseNotes b/PendingReleaseNotes
index 54befe629c918d3835d9f8ce86bf6840b0daf0ed..6514b9a7c63236694e6f4c6c082c8d3f287779ce 100644 (file)
--- a/PendingReleaseNotes
+++ b/PendingReleaseNotes
@@ -1,3 +1,12 @@
-11.1.1
+12.0.0
 ------
 
+ * The omap_get_keys and omap_get_vals librados functions have been deprecated
+   in favor of omap_get_vals2 and omap_get_keys2.  The new methods include an
+   output argument indicating whether there are additional keys left to fetch.
+   Previously this had to be inferred from the requested key count vs the
+   number of keys returned, but this breaks with new OSD-side limits on the
+   number of keys or bytes that can be returned by a single omap request.  These
+   limits were introduced by kraken but were effectively disabled (by setting a
+   very large limit of 1 GB) because users of the newly deprecated interface
+   cannot tell whether they should fetch more keys or not.

diff --git a/src/include/rados/librados.h b/src/include/rados/librados.h
index 1b60e8e1c63e6ab82cb7076edf769f30be443b93..32cea4f1868b4c15acee83b09da6bc0eba01e8e7 100644 (file)
--- a/src/include/rados/librados.h
+++ b/src/include/rados/librados.h
@@ -3124,7 +3124,29 @@ CEPH_RADOS_API void rados_read_op_omap_get_vals(rados_read_op_t read_op,
                                                const char *filter_prefix,
                                                uint64_t max_return,
                                                rados_omap_iter_t *iter,
-                                               int *prval);
+                                               int *prval)
+  __attribute__((deprecated)); /* use v2 below */
+
+/**
+ * Start iterating over key/value pairs on an object.
+ *
+ * They will be returned sorted by key.
+ *
+ * @param read_op operation to add this action to
+ * @param start_after list keys starting after start_after
+ * @param filter_prefix list only keys beginning with filter_prefix
+ * @param max_return list no more than max_return key/value pairs
+ * @param iter where to store the iterator
+ * @param pmore flag indicating whether there are more keys to fetch
+ * @param prval where to store the return value from this action
+ */
+CEPH_RADOS_API void rados_read_op_omap_get_vals2(rados_read_op_t read_op,
+                                                const char *start_after,
+                                                const char *filter_prefix,
+                                                uint64_t max_return,
+                                                rados_omap_iter_t *iter,
+                                                unsigned char *pmore,
+                                                int *prval);
 
 /**
  * Start iterating over keys on an object.
@@ -3142,7 +3164,28 @@ CEPH_RADOS_API void rados_read_op_omap_get_keys(rados_read_op_t read_op,
                                                const char *start_after,
                                                uint64_t max_return,
                                                rados_omap_iter_t *iter,
-                                               int *prval);
+                                               int *prval)
+  __attribute__((deprecated)); /* use v2 below */
+
+/**
+ * Start iterating over keys on an object.
+ *
+ * They will be returned sorted by key, and the iterator
+ * will fill in NULL for all values if specified.
+ *
+ * @param read_op operation to add this action to
+ * @param start_after list keys starting after start_after
+ * @param max_return list no more than max_return keys
+ * @param iter where to store the iterator
+ * @param pmore flag indicating whether there are more keys to fetch
+ * @param prval where to store the return value from this action
+ */
+CEPH_RADOS_API void rados_read_op_omap_get_keys2(rados_read_op_t read_op,
+                                                const char *start_after,
+                                                uint64_t max_return,
+                                                rados_omap_iter_t *iter,
+                                                unsigned char *pmore,
+                                                int *prval);
 
 /**
  * Start iterating over specific key/value pairs

diff --git a/src/include/rados/librados.hpp b/src/include/rados/librados.hpp
index 674b1bafbaade4e500908b5c4c692f0b51c37050..17930e2c421b6b066bd16daea162d0ab93b30384 100644 (file)
--- a/src/include/rados/librados.hpp
+++ b/src/include/rados/librados.hpp
@@ -522,6 +522,23 @@ namespace librados
       const std::string &start_after,
       uint64_t max_return,
       std::map<std::string, bufferlist> *out_vals,
+      int *prval) __attribute__ ((deprecated));  // use v2
+
+    /**
+     * omap_get_vals: keys and values from the object omap
+     *
+     * Get up to max_return keys and values beginning after start_after
+     *
+     * @param start_after [in] list no keys smaller than start_after
+     * @param max_return [in] list no more than max_return key/value pairs
+     * @param out_vals [out] place returned values in out_vals on completion
+     * @param prval [out] place error code in prval upon completion
+     */
+    void omap_get_vals2(
+      const std::string &start_after,
+      uint64_t max_return,
+      std::map<std::string, bufferlist> *out_vals,
+      bool *pmore,
       int *prval);
 
     /**
@@ -540,6 +557,26 @@ namespace librados
       const std::string &filter_prefix,
       uint64_t max_return,
       std::map<std::string, bufferlist> *out_vals,
+      int *prval) __attribute__ ((deprecated));  // use v2
+
+    /**
+     * omap_get_vals2: keys and values from the object omap
+     *
+     * Get up to max_return keys and values beginning after start_after
+     *
+     * @param start_after [in] list keys starting after start_after
+     * @param filter_prefix [in] list only keys beginning with filter_prefix
+     * @param max_return [in] list no more than max_return key/value pairs
+     * @param out_vals [out] place returned values in out_vals on completion
+     * @param pmore [out] pointer to bool indicating whether there are more keys
+     * @param prval [out] place error code in prval upon completion
+     */
+    void omap_get_vals2(
+      const std::string &start_after,
+      const std::string &filter_prefix,
+      uint64_t max_return,
+      std::map<std::string, bufferlist> *out_vals,
+      bool *pmore,
       int *prval);
 
 
@@ -556,7 +593,24 @@ namespace librados
     void omap_get_keys(const std::string &start_after,
                        uint64_t max_return,
                        std::set<std::string> *out_keys,
-                       int *prval);
+                       int *prval) __attribute__ ((deprecated)); // use v2
+
+    /**
+     * omap_get_keys2: keys from the object omap
+     *
+     * Get up to max_return keys beginning after start_after
+     *
+     * @param start_after [in] list keys starting after start_after
+     * @param max_return [in] list no more than max_return keys
+     * @param out_keys [out] place returned values in out_keys on completion
+     * @param pmore [out] pointer to bool indicating whether there are more keys
+     * @param prval [out] place error code in prval upon completion
+     */
+    void omap_get_keys2(const std::string &start_after,
+                       uint64_t max_return,
+                       std::set<std::string> *out_keys,
+                       bool *pmore,
+                       int *prval);
 
     /**
      * omap_get_header: get header from object omap
@@ -745,15 +799,31 @@ namespace librados
                       const std::string& start_after,
                       uint64_t max_return,
                       std::map<std::string, bufferlist> *out_vals);
+    int omap_get_vals2(const std::string& oid,
+                      const std::string& start_after,
+                      uint64_t max_return,
+                      std::map<std::string, bufferlist> *out_vals,
+                      bool *pmore);
     int omap_get_vals(const std::string& oid,
                       const std::string& start_after,
                       const std::string& filter_prefix,
                       uint64_t max_return,
                       std::map<std::string, bufferlist> *out_vals);
+    int omap_get_vals2(const std::string& oid,
+                      const std::string& start_after,
+                      const std::string& filter_prefix,
+                      uint64_t max_return,
+                      std::map<std::string, bufferlist> *out_vals,
+                      bool *pmore);
     int omap_get_keys(const std::string& oid,
                       const std::string& start_after,
                       uint64_t max_return,
                       std::set<std::string> *out_keys);
+    int omap_get_keys2(const std::string& oid,
+                      const std::string& start_after,
+                      uint64_t max_return,
+                      std::set<std::string> *out_keys,
+                      bool *pmore);
     int omap_get_header(const std::string& oid,
                         bufferlist *bl);
     int omap_get_vals_by_keys(const std::string& oid,

diff --git a/src/librados/librados.cc b/src/librados/librados.cc
index 130b7316d980847ccf53b185fd696d601c4603d8..b14637c5104260af0fc9f60cedb80085ee7410e6 100644 (file)
--- a/src/librados/librados.cc
+++ b/src/librados/librados.cc
@@ -270,6 +270,19 @@ void librados::ObjectReadOperation::omap_get_vals(
                   prval);
 }
 
+void librados::ObjectReadOperation::omap_get_vals2(
+  const std::string &start_after,
+  const std::string &filter_prefix,
+  uint64_t max_return,
+  std::map<std::string, bufferlist> *out_vals,
+  bool *pmore,
+  int *prval)
+{
+  ::ObjectOperation *o = &impl->o;
+  o->omap_get_vals(start_after, filter_prefix, max_return, out_vals, pmore,
+                  prval);
+}
+
 void librados::ObjectReadOperation::omap_get_vals(
   const std::string &start_after,
   uint64_t max_return,
@@ -280,6 +293,17 @@ void librados::ObjectReadOperation::omap_get_vals(
   o->omap_get_vals(start_after, "", max_return, out_vals, nullptr, prval);
 }
 
+void librados::ObjectReadOperation::omap_get_vals2(
+  const std::string &start_after,
+  uint64_t max_return,
+  std::map<std::string, bufferlist> *out_vals,
+  bool *pmore,
+  int *prval)
+{
+  ::ObjectOperation *o = &impl->o;
+  o->omap_get_vals(start_after, "", max_return, out_vals, pmore, prval);
+}
+
 void librados::ObjectReadOperation::omap_get_keys(
   const std::string &start_after,
   uint64_t max_return,
@@ -290,6 +314,17 @@ void librados::ObjectReadOperation::omap_get_keys(
   o->omap_get_keys(start_after, max_return, out_keys, nullptr, prval);
 }
 
+void librados::ObjectReadOperation::omap_get_keys2(
+  const std::string &start_after,
+  uint64_t max_return,
+  std::set<std::string> *out_keys,
+  bool *pmore,
+  int *prval)
+{
+  ::ObjectOperation *o = &impl->o;
+  o->omap_get_keys(start_after, max_return, out_keys, pmore, prval);
+}
+
 void librados::ObjectReadOperation::omap_get_header(bufferlist *bl, int *prval)
 {
   ::ObjectOperation *o = &impl->o;
@@ -336,19 +371,61 @@ void librados::ObjectReadOperation::is_dirty(bool *is_dirty, int *prval)
 }
 
 int librados::IoCtx::omap_get_vals(const std::string& oid,
-                                   const std::string& start_after,
+                                   const std::string& orig_start_after,
                                    const std::string& filter_prefix,
                                    uint64_t max_return,
                                    std::map<std::string, bufferlist> *out_vals)
+{
+  bool first = true;
+  string start_after = orig_start_after;
+  bool more = true;
+  while (max_return > 0 && more) {
+    std::map<std::string,bufferlist> out;
+    ObjectReadOperation op;
+    op.omap_get_vals2(start_after, filter_prefix, max_return, &out, &more,
+                     nullptr);
+    bufferlist bl;
+    int ret = operate(oid, &op, &bl);
+    if (ret < 0) {
+      return ret;
+    }
+    if (more) {
+      if (out.empty()) {
+       return -EINVAL;  // wth
+      }
+      start_after = out.rbegin()->first;
+    }
+    if (out.size() <= max_return) {
+      max_return -= out.size();
+    } else {
+      max_return = 0;
+    }
+    if (first) {
+      out_vals->swap(out);
+      first = false;
+    } else {
+      out_vals->insert(out.begin(), out.end());
+      out.clear();
+    }
+  }
+  return 0;
+}
+
+int librados::IoCtx::omap_get_vals2(
+  const std::string& oid,
+  const std::string& start_after,
+  const std::string& filter_prefix,
+  uint64_t max_return,
+  std::map<std::string, bufferlist> *out_vals,
+  bool *pmore)
 {
   ObjectReadOperation op;
   int r;
-  op.omap_get_vals(start_after, filter_prefix, max_return, out_vals, &r);
+  op.omap_get_vals2(start_after, filter_prefix, max_return, out_vals, pmore, &r);
   bufferlist bl;
   int ret = operate(oid, &op, &bl);
   if (ret < 0)
     return ret;
-
   return r;
 }
 
@@ -1325,31 +1402,80 @@ int librados::IoCtx::omap_get_vals(const std::string& oid,
                                    const std::string& start_after,
                                    uint64_t max_return,
                                    std::map<std::string, bufferlist> *out_vals)
+{
+  return omap_get_vals(oid, start_after, string(), max_return, out_vals);
+}
+
+int librados::IoCtx::omap_get_vals2(
+  const std::string& oid,
+  const std::string& start_after,
+  uint64_t max_return,
+  std::map<std::string, bufferlist> *out_vals,
+  bool *pmore)
 {
   ObjectReadOperation op;
   int r;
-  op.omap_get_vals(start_after, max_return, out_vals, &r);
+  op.omap_get_vals2(start_after, max_return, out_vals, pmore, &r);
   bufferlist bl;
   int ret = operate(oid, &op, &bl);
   if (ret < 0)
     return ret;
-
   return r;
 }
 
 int librados::IoCtx::omap_get_keys(const std::string& oid,
-                                   const std::string& start_after,
+                                   const std::string& orig_start_after,
                                    uint64_t max_return,
                                    std::set<std::string> *out_keys)
+{
+  bool first = true;
+  string start_after = orig_start_after;
+  bool more = true;
+  while (max_return > 0 && more) {
+    std::set<std::string> out;
+    ObjectReadOperation op;
+    op.omap_get_keys2(start_after, max_return, &out, &more, nullptr);
+    bufferlist bl;
+    int ret = operate(oid, &op, &bl);
+    if (ret < 0) {
+      return ret;
+    }
+    if (more) {
+      if (out.empty()) {
+       return -EINVAL;  // wth
+      }
+      start_after = *out.rbegin();
+    }
+    if (out.size() <= max_return) {
+      max_return -= out.size();
+    } else {
+      max_return = 0;
+    }
+    if (first) {
+      out_keys->swap(out);
+      first = false;
+    } else {
+      out_keys->insert(out.begin(), out.end());
+      out.clear();
+    }
+  }
+  return 0;
+}
+
+int librados::IoCtx::omap_get_keys2(
+  const std::string& oid,
+  const std::string& start_after,
+  uint64_t max_return,
+  std::set<std::string> *out_keys,
+  bool *pmore)
 {
   ObjectReadOperation op;
   int r;
-  op.omap_get_keys(start_after, max_return, out_keys, &r);
+  op.omap_get_keys2(start_after, max_return, out_keys, pmore, &r);
   bufferlist bl;
   int ret = operate(oid, &op, &bl);
   if (ret < 0)
     return ret;
-
   return r;
 }
 
@@ -5727,6 +5853,30 @@ extern "C" void rados_read_op_omap_get_vals(rados_read_op_t read_op,
   tracepoint(librados, rados_read_op_omap_get_vals_exit, *iter);
 }
 
+extern "C" void rados_read_op_omap_get_vals2(rados_read_op_t read_op,
+                                            const char *start_after,
+                                            const char *filter_prefix,
+                                            uint64_t max_return,
+                                            rados_omap_iter_t *iter,
+                                            unsigned char *pmore,
+                                            int *prval)
+{
+  tracepoint(librados, rados_read_op_omap_get_vals_enter, read_op, start_after, filter_prefix, max_return, prval);
+  RadosOmapIter *omap_iter = new RadosOmapIter;
+  const char *start = start_after ? start_after : "";
+  const char *filter = filter_prefix ? filter_prefix : "";
+  ((::ObjectOperation *)read_op)->omap_get_vals(
+    start,
+    filter,
+    max_return,
+    &omap_iter->values,
+    (bool*)pmore,
+    prval);
+  ((::ObjectOperation *)read_op)->add_handler(new C_OmapIter(omap_iter));
+  *iter = omap_iter;
+  tracepoint(librados, rados_read_op_omap_get_vals_exit, *iter);
+}
+
 struct C_OmapKeysIter : public Context {
   RadosOmapIter *iter;
   std::set<std::string> keys;
@@ -5758,6 +5908,25 @@ extern "C" void rados_read_op_omap_get_keys(rados_read_op_t read_op,
   tracepoint(librados, rados_read_op_omap_get_keys_exit, *iter);
 }
 
+extern "C" void rados_read_op_omap_get_keys2(rados_read_op_t read_op,
+                                            const char *start_after,
+                                            uint64_t max_return,
+                                            rados_omap_iter_t *iter,
+                                            unsigned char *pmore,
+                                            int *prval)
+{
+  tracepoint(librados, rados_read_op_omap_get_keys_enter, read_op, start_after, max_return, prval);
+  RadosOmapIter *omap_iter = new RadosOmapIter;
+  C_OmapKeysIter *ctx = new C_OmapKeysIter(omap_iter);
+  ((::ObjectOperation *)read_op)->omap_get_keys(
+    start_after ? start_after : "",
+    max_return, &ctx->keys,
+    (bool*)pmore, prval);
+  ((::ObjectOperation *)read_op)->add_handler(ctx);
+  *iter = omap_iter;
+  tracepoint(librados, rados_read_op_omap_get_keys_exit, *iter);
+}
+
 extern "C" void rados_read_op_omap_get_vals_by_keys(rados_read_op_t read_op,
                                                    char const* const* keys,
                                                    size_t keys_len,