From a7398e3c95607caee007e5fedff2763e36de5add Mon Sep 17 00:00:00 2001 From: "J. Eric Ivancich" Date: Thu, 20 Oct 2022 15:08:53 -0400 Subject: [PATCH] rgw: document convention used in ReadOp::read and ReadOp::iterate Both of these functions defined at the SAL layer take an offset and an end offset. It's important to note that the end offset is inclusive since a common, modern convention is for the end offset to be exclusive. Signed-off-by: J. Eric Ivancich --- src/rgw/rgw_rados.cc | 8 +++++++- src/rgw/rgw_sal.h | 17 ++++++++++++----- src/rgw/rgw_sal_daos.h | 7 +++++++ src/rgw/rgw_sal_dbstore.h | 17 ++++++++++++++--- src/rgw/rgw_sal_motr.h | 15 +++++++++++++-- src/rgw/rgw_sal_rados.h | 19 +++++++++++++++---- 6 files changed, 68 insertions(+), 15 deletions(-) diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index 47162e7dec1..63992e9bc1e 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -6382,7 +6382,13 @@ int RGWRados::Bucket::UpdateIndex::cancel(const DoutPrefixProvider *dpp, return ret; } -int RGWRados::Object::Read::read(int64_t ofs, int64_t end, bufferlist& bl, optional_yield y, const DoutPrefixProvider *dpp) +/* + * Read up through index `end` inclusive. Number of bytes read is up + * to `end - ofs + 1`. + */ +int RGWRados::Object::Read::read(int64_t ofs, int64_t end, + bufferlist& bl, optional_yield y, + const DoutPrefixProvider *dpp) { RGWRados *store = source->get_store(); diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 8bb2f476ddc..c0ea2e63196 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -883,11 +883,18 @@ class Object { /** Prepare the Read op. Must be called first */ virtual int prepare(optional_yield y, const DoutPrefixProvider* dpp) = 0; - /** Synchronous read. Read from @a ofs to @a end into @a bl */ - virtual int read(int64_t ofs, int64_t end, bufferlist& bl, optional_yield y, const DoutPrefixProvider* dpp) = 0; - /** Asynchronous read. Read from @a ofs to @a end calling @a cb on each read - * chunk. */ - virtual int iterate(const DoutPrefixProvider* dpp, int64_t ofs, int64_t end, RGWGetDataCB* cb, optional_yield y) = 0; + + /** Synchronous read. Read from @a ofs to @a end (inclusive) + * into @a bl. Length is `end - ofs + 1`. */ + virtual int read(int64_t ofs, int64_t end, bufferlist& bl, + optional_yield y, const DoutPrefixProvider* dpp) = 0; + + /** Asynchronous read. Read from @a ofs to @a end (inclusive) + * calling @a cb on each read chunk. Length is `end - ofs + + * 1`. */ + virtual int iterate(const DoutPrefixProvider* dpp, int64_t ofs, + int64_t end, RGWGetDataCB* cb, optional_yield y) = 0; + /** Get an attribute by name */ virtual int get_attr(const DoutPrefixProvider* dpp, const char* name, bufferlist& dest, optional_yield y) = 0; }; diff --git a/src/rgw/rgw_sal_daos.h b/src/rgw/rgw_sal_daos.h index 910ddea83b7..8dab0cad059 100644 --- a/src/rgw/rgw_sal_daos.h +++ b/src/rgw/rgw_sal_daos.h @@ -564,10 +564,17 @@ class DaosObject : public StoreObject { virtual int prepare(optional_yield y, const DoutPrefixProvider* dpp) override; + + /* + * Both `read` and `iterate` read up through index `end` + * *inclusive*. The number of bytes that could be returned is + * `end - ofs + 1`. + */ virtual int read(int64_t off, int64_t end, bufferlist& bl, optional_yield y, const DoutPrefixProvider* dpp) override; virtual int iterate(const DoutPrefixProvider* dpp, int64_t off, int64_t end, RGWGetDataCB* cb, optional_yield y) override; + virtual int get_attr(const DoutPrefixProvider* dpp, const char* name, bufferlist& dest, optional_yield y) override; }; diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index 94a4926c1bf..33ad7c48c62 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -539,9 +539,20 @@ protected: DBReadOp(DBObject *_source, RGWObjectCtx *_rctx); virtual int prepare(optional_yield y, const DoutPrefixProvider* dpp) override; - virtual int read(int64_t ofs, int64_t end, bufferlist& bl, optional_yield y, const DoutPrefixProvider* dpp) override; - virtual int iterate(const DoutPrefixProvider* dpp, int64_t ofs, int64_t end, RGWGetDataCB* cb, optional_yield y) override; - virtual int get_attr(const DoutPrefixProvider* dpp, const char* name, bufferlist& dest, optional_yield y) override; + + /* + * Both `read` and `iterate` read up through index `end` + * *inclusive*. The number of bytes that could be returned is + * `end - ofs + 1`. + */ + virtual int read(int64_t ofs, int64_t end, bufferlist& bl, + optional_yield y, + const DoutPrefixProvider* dpp) override; + virtual int iterate(const DoutPrefixProvider* dpp, int64_t ofs, + int64_t end, RGWGetDataCB* cb, + optional_yield y) override; + + virtual int get_attr(const DoutPrefixProvider* dpp, const char* name, bufferlist& dest, optional_yield y) override; }; struct DBDeleteOp : public DeleteOp { diff --git a/src/rgw/rgw_sal_motr.h b/src/rgw/rgw_sal_motr.h index 00e260d293d..cb190e71578 100644 --- a/src/rgw/rgw_sal_motr.h +++ b/src/rgw/rgw_sal_motr.h @@ -574,8 +574,19 @@ class MotrObject : public StoreObject { MotrReadOp(MotrObject *_source); virtual int prepare(optional_yield y, const DoutPrefixProvider* dpp) override; - virtual int read(int64_t off, int64_t end, bufferlist& bl, optional_yield y, const DoutPrefixProvider* dpp) override; - virtual int iterate(const DoutPrefixProvider* dpp, int64_t off, int64_t end, RGWGetDataCB* cb, optional_yield y) override; + + /* + * Both `read` and `iterate` read up through index `end` + * *inclusive*. The number of bytes that could be returned is + * `end - ofs + 1`. + */ + virtual int read(int64_t off, int64_t end, bufferlist& bl, + optional_yield y, + const DoutPrefixProvider* dpp) override; + virtual int iterate(const DoutPrefixProvider* dpp, int64_t off, + int64_t end, RGWGetDataCB* cb, + optional_yield y) override; + virtual int get_attr(const DoutPrefixProvider* dpp, const char* name, bufferlist& dest, optional_yield y) override; }; diff --git a/src/rgw/rgw_sal_rados.h b/src/rgw/rgw_sal_rados.h index 0771b9ad974..c54f3344d13 100644 --- a/src/rgw/rgw_sal_rados.h +++ b/src/rgw/rgw_sal_rados.h @@ -1,4 +1,4 @@ - +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab ft=cpp /* @@ -343,9 +343,20 @@ class RadosObject : public StoreObject { RadosReadOp(RadosObject *_source, RGWObjectCtx *_rctx); virtual int prepare(optional_yield y, const DoutPrefixProvider* dpp) override; - virtual int read(int64_t ofs, int64_t end, bufferlist& bl, optional_yield y, const DoutPrefixProvider* dpp) override; - virtual int iterate(const DoutPrefixProvider* dpp, int64_t ofs, int64_t end, RGWGetDataCB* cb, optional_yield y) override; - virtual int get_attr(const DoutPrefixProvider* dpp, const char* name, bufferlist& dest, optional_yield y) override; + + /* + * Both `read` and `iterate` read up through index `end` + * *inclusive*. The number of bytes that could be returned is + * `end - ofs + 1`. + */ + virtual int read(int64_t ofs, int64_t end, + bufferlist& bl, optional_yield y, + const DoutPrefixProvider* dpp) override; + virtual int iterate(const DoutPrefixProvider* dpp, + int64_t ofs, int64_t end, + RGWGetDataCB* cb, optional_yield y) override; + + virtual int get_attr(const DoutPrefixProvider* dpp, const char* name, bufferlist& dest, optional_yield y) override; }; struct RadosDeleteOp : public DeleteOp { -- 2.39.5