From fbebd83537b66b19d51c68e2fd51359305e84a3b Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 16 Jan 2017 15:14:00 -0500 Subject: [PATCH] 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 --- PendingReleaseNotes | 11 +- src/include/rados/librados.h | 47 ++++++++- src/include/rados/librados.hpp | 72 ++++++++++++- src/librados/librados.cc | 185 +++++++++++++++++++++++++++++++-- 4 files changed, 303 insertions(+), 12 deletions(-) diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 54befe629c918..6514b9a7c6323 100644 --- 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 1b60e8e1c63e6..32cea4f1868b4 100644 --- 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 674b1bafbaade..17930e2c421b6 100644 --- 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 *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 *out_vals, + bool *pmore, int *prval); /** @@ -540,6 +557,26 @@ namespace librados const std::string &filter_prefix, uint64_t max_return, std::map *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 *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 *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 *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 *out_vals); + int omap_get_vals2(const std::string& oid, + const std::string& start_after, + uint64_t max_return, + std::map *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 *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 *out_vals, + bool *pmore); int omap_get_keys(const std::string& oid, const std::string& start_after, uint64_t max_return, std::set *out_keys); + int omap_get_keys2(const std::string& oid, + const std::string& start_after, + uint64_t max_return, + std::set *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 130b7316d9808..b14637c510426 100644 --- 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 *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 *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 *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 *out_vals) +{ + bool first = true; + string start_after = orig_start_after; + bool more = true; + while (max_return > 0 && more) { + std::map 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 *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 *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 *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 *out_keys) +{ + bool first = true; + string start_after = orig_start_after; + bool more = true; + while (max_return > 0 && more) { + std::set 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 *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 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, -- 2.39.5