MB-54267: Add extras to RangeScan continue responses

As the value uses a different encoding depending on the scan type
it is useful to tag the responses so the value can be decoded
without knowing the exact configuration of the scan.

Change-Id: Ifc1f2782ba2b6d964fab3431e935d88829ee0a14
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/182205
Tested-by: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>

Author: jimwwalker

daemon/cookie.cc

@@ -1220,10 +1220,12 @@ bool Cookie::checkThrottle(size_t pendingRBytes, size_t pendingWBytes) {
             *this, true, pendingRBytes + pendingWBytes);
 }
 
-bool Cookie::sendResponse(cb::engine_errc status, std::string_view value) {
+bool Cookie::sendResponse(cb::engine_errc status,
+                          std::string_view extras,
+                          std::string_view value) {
     return connection.sendResponse(*this,
                                    cb::mcbp::to_status(status),
-                                   {},
+                                   extras,
                                    {},
                                    value,
                                    uint8_t(cb::mcbp::Datatype::Raw),

daemon/cookie.h

@@ -648,7 +648,10 @@ class Cookie : public CookieIface {
 
     /// Cookie implementation checks the bucket throttle
     bool checkThrottle(size_t pendingRBytes, size_t pendingWBytes) override;
-    bool sendResponse(cb::engine_errc status, std::string_view value) override;
+    /// Cookie implementation calls through to Connection::sendResponse
+    bool sendResponse(cb::engine_errc status,
+                      std::string_view extras,
+                      std::string_view value) override;
 
 protected:
     /**

docs/range_scans/range_scan_continue.md

@@ -66,54 +66,62 @@ The extras for a continue range scan encodes:
 ## Response format
 
 A successful continue will trigger the server to iterate forwards through the
-range of keys, return keys/documents using a bulk framing.
+range of keys and return keys/documents using a bulk framing.
 
-The many keys/documents of a single continue maybe sent (framed) in multiple
-responses, where a final response (with bespoke status) indicates the end of the
-individual continue or the end of the scan itself. Each response of the continue
-may encode multiple keys or documents, the formatting of which is described
-in the following sections.
+The many keys/documents of a single continue maybe sent using multiple responses.
+A final response uses a bespoke range-scan status code to indicate the end of the
+individual range-scan-continue or that the scan has completed.
 
+Each response generated by the range-scan-continue may contain a value which
+encodes multiple keys or documents, the formatting of which is described
+in the following sections. Note that each response will also be given an extras
+section, which is defined in a following section.
 
-A successful continue will return key/documents using the following framing. A
-series of key/documents always begins with the standard mcbp 24-byte response
-header which will include a success status and a value, the value encodes many
-keys/documents.
+A batch of keys or documents always begins with the standard mcbp 24-byte response
+header which will include a "success" status, extras and a value. It is the value
+that contains the keys or documents.
 
-The following examples highlight how the responses to range-scan-continue works.
+The following examples intend to highlight how the responses from a range-scan-continue work.
 
-For example a range may cover an unknown number of keys and the client only
+For example a range covers an unknown number of keys and the client only
 wants to handle up to 500 keys per range-scan-continue. Thus the client would
-first issue a range-scan-continue (0xDB) with the uuid from range-scan-create
-(0xDA), item limit set to 500 and the time limit set to 0.
+first issue a range-scan-continue (0xDB) with:
 
-The server will respond with a series of responses, there is no definition of
-how many keys each response may encode.
+* the uuid from range-scan-create (0xDA)
+* item limit set to 500
+* time limit set to 0
+* byte limit set to 0
 
-Example 1, range scan progresses, yet more continues will be required.
+The server will respond with a series of responses. Clients must note that there
+is no definition of how many keys or documents each response may include.
+
+Example 1: A continue hits a limit and indicates that another continue is
+required because more keys or documents maybe available.
 
 * response status=success, bodylen=261
 * response status=success, bodylen=259
 * response status=success, bodylen=260
 * response status=range-scan-more (0xA6), bodylen=241
 
-In this sequence 500 keys are returned (distributed over 4 different responses).
-The final response has the status of range-scan-more informing the client that
-the scan has more data.
+In this sequence 500 keys are distributed over 4 different responses. The final
+response has the status of range-scan-more informing the client that the scan
+could have more data.
 
-Example 2, range scan progress to the end
+Example 2: A continue now reaches the end of the requested range.
 
 * response status=success, bodylen=262
 * response status=range-scan-complete (0xA7), bodylen=120
 
-In this sequence <= 500 keys have been returned (distributed over 2 different
-responses). The final response as the status of range-scan-complete, informing
-the client that the scan has now completed (server has closed the scan).
+In this sequence <= 500 keys have been distributed over 2 different responses.
+The status of range-scan-complete informs the client that the scan has now
+completed. The server has closed the scan and any further continue will be
+rejected.
 
-Example 3, range scan progresses, but vbucket state change interrupts. This
-example demonstrates that the client must be aware that the sequence of continue
-responses could be bookmarked with a status indicating some other issue. In this
-case the range-scan has also been cancelled due to the issue.
+Example 3: range scan progresses, but is stopped due to a system state change,
+in this case the vbucket is no longer active. This example demonstrates that the
+client must be aware that the sequence of continue responses could end with a
+status indicating a terminal issue. For these situations the range-scan is
+cancelled by the server.
 
 * response status=success, bodylen=261
 * response status=success, bodylen=259
@@ -122,6 +130,15 @@ case the range-scan has also been cancelled due to the issue.
 In this example < 500 keys have been spread over three responses, the final
 response indicates that the target vbucket has changed state.
 
+### Extras Definition
+
+Each response packet includes an extras structure which contains a single 4-byte
+"flags" field. This field currently describes the content of the value (keys or
+key/meta/value). The field uses network byte order.
+
+* 0 response has keys only (i.e. a `key_only:true` scan)
+* 1 response has keys + meta + documents (i.e. a `key_only:false` scan)
+
 ### Response Value Encoding `"key_only":true`
 
 The mcbp header defines how much data follows, in this command only a value is
@@ -221,14 +238,25 @@ E.g. a document with key="key0" and value="value0"
 In the above layout, a second document would begin at offset 37, no padding or
 alignment.
 
-### Success
+### Success status codes
+
+A range-scan-continue response sequence indicates success using the following
+status codes.
+
+**Status::Success 0x00**
+
+Used for intermediate responses making up a larger response.
+
+**Status::RangeScanMore 0xA6**
+
+Scan has reached a limit and more data maybe available, the client must issue
+another continue.
+
+**Status::RangeScanComplete 0xA7**
 
-A range-scan-continue response sequence indicates success using status codes
+Scan has reached the end of the range, the scan itself will be removed from the
+server and any subsequent range-scan continue or cancel will be rejected.
 
-* Status::Success 0x00: Used for intermediate responses making up a larger response.
-* Status::RangeScanMore 0xA6: Scan has reached a limit and has not reached the end
-key, more data maybe available and the client must issue another continue.
-* Status::RangeScanComplete 0xA7: Scan has reached the end of the range.
 
 ### Errors
 
@@ -259,5 +287,4 @@ The collection was dropped.
 
 **Status::RangeScanCancelled (0xA5)**
 
-The scan was cancelled whilst returning data. This could be the only status if
-the cancel was noticed before a key/value was loaded.
+The scan was cancelled whilst returning data.

engines/ep/src/ep_vb.cc

@@ -1284,7 +1284,8 @@ std::pair<cb::engine_errc, cb::rangescan::Id> EPVBucket::createRangeScan(
 
     // If no handler has been given, create one.
     if (!handler) {
-        handler = std::make_unique<RangeScanDataHandler>(bucket->getEPEngine());
+        handler = std::make_unique<RangeScanDataHandler>(
+                bucket->getEPEngine(), keyOnly == cb::rangescan::KeyOnly::Yes);
     }
 
     // Create a task and give it the RangeScanCreateData, on failure the task

engines/ep/src/range_scans/range_scan_callbacks.cc

@@ -24,10 +24,12 @@
 #include <memcached/range_scan_status.h>
 #include <statistics/cbstat_collector.h>
 
-RangeScanDataHandler::RangeScanDataHandler(EventuallyPersistentEngine& engine)
+RangeScanDataHandler::RangeScanDataHandler(EventuallyPersistentEngine& engine,
+                                           bool keyOnly)
     : engine(engine),
       sendTriggerThreshold(
-              engine.getConfiguration().getRangeScanReadBufferSendSize()) {
+              engine.getConfiguration().getRangeScanReadBufferSendSize()),
+      keyOnly(keyOnly) {
 }
 
 RangeScanDataHandler::Status RangeScanDataHandler::checkAndSend(
@@ -55,9 +57,11 @@ RangeScanDataHandler::Status RangeScanDataHandler::send(
         CookieIface& cookie, cb::engine_errc status) {
     bool sendSuccess = false;
     {
+        cb::mcbp::response::RangeScanContinueResponseExtras extras(keyOnly);
         NonBucketAllocationGuard guard;
         sendSuccess = cookie.sendResponse(
                 status,
+                extras.getBuffer(),
                 {reinterpret_cast<const char*>(responseBuffer.data()),
                  responseBuffer.size()});
     }

engines/ep/src/range_scans/range_scan_callbacks.h

@@ -100,7 +100,7 @@ class RangeScanDataHandlerIFace {
  */
 class RangeScanDataHandler : public RangeScanDataHandlerIFace {
 public:
-    RangeScanDataHandler(EventuallyPersistentEngine& engine);
+    RangeScanDataHandler(EventuallyPersistentEngine& engine, bool keyOnly);
 
     Status handleKey(CookieIface& cookie, DocKey key) override;
 
@@ -131,14 +131,16 @@ class RangeScanDataHandler : public RangeScanDataHandlerIFace {
     std::vector<uint8_t> responseBuffer;
 
     /// the trigger for pushing data to send, set from engine configuration
-    size_t sendTriggerThreshold{0};
+    const size_t sendTriggerThreshold{0};
 
     /**
      * As the scan continues, and reads data it accumulates how many bytes
      * are read in this member for use in checking the bucket throttle and
      * updating the metering counter.
      */
     size_t pendingReadBytes{0};
+
+    const bool keyOnly{false};
 };
 
 /**

include/mcbp/codec/range_scan_continue_codec.h

@@ -81,4 +81,25 @@ class RangeScanContinueValuePayload {
     std::string_view payload;
 };
 
+// The structure which is attached to all response packets from RangeScan
+// continue
+class RangeScanContinueResponseExtras {
+public:
+    enum class Flags : uint32_t { KeyScan = 0, ValueScan = 1 };
+
+    RangeScanContinueResponseExtras(bool keyOnly);
+
+    std::string_view getBuffer() const {
+        return {reinterpret_cast<const char*>(this), sizeof(*this)};
+    }
+
+    Flags getFlags() const;
+
+protected:
+    uint32_t flags{0};
+};
+
+static_assert(sizeof(RangeScanContinueResponseExtras) == 4,
+              "Unexpected object size");
+
 } // namespace cb::mcbp::response

include/memcached/cookie_iface.h

@@ -143,10 +143,12 @@ class CookieIface : public cb::tracing::Traceable {
     /**
      * Send a status/value in a response message
      * @param status the status to use in the response message
-     * @param value view onto a value to send
+     * @param extras view onto extras to attach to the response
+     * @param value view onto a value to attach to the response
      * @return true if successful, false if not (e.g. disconnect)
      */
     virtual bool sendResponse(cb::engine_errc status,
+                              std::string_view extras,
                               std::string_view value) = 0;
 
     /**

programs/engine_testapp/mock_cookie.cc

@@ -152,7 +152,9 @@ bool MockCookie::checkThrottle(size_t, size_t) {
     return false;
 }
 
-bool MockCookie::sendResponse(cb::engine_errc, std::string_view) {
+bool MockCookie::sendResponse(cb::engine_errc,
+                              std::string_view,
+                              std::string_view) {
     // do nothing
     return true;
 }

programs/engine_testapp/mock_cookie.h

@@ -169,7 +169,9 @@ class MockCookie : public CookieIface {
                                   uint64_t manifestUid,
                                   bool metered) override;
     bool checkThrottle(size_t, size_t) override;
-    bool sendResponse(cb::engine_errc status, std::string_view value) override;
+    bool sendResponse(cb::engine_errc status,
+                      std::string_view extras,
+                      std::string_view value) override;
 
     /// An alternative function to call for notifyIoComplete
     void setUserNotifyIoComplete(std::function<void(cb::engine_errc)> func) {