[NEW] Add an API for RedisModule of DiscardableResponse

[ashtul]

The problem/use-case that the feature addresses

Currently, if a module reaches an issue after a reply was sent to the server, there is no way to reverse that.

Description of the feature

Add 3 commands to the modules API:

1. RM_ReplyWithDiscardableResponse which will flag replies as discardable.
2. RM_ReplyDiscardDiscardableResponse which will discard discardable replies.
3. RM_ReplyKeepDiscardableResponse which will accept discardable replies.

Alternatives you've considered

Any alternative solutions or features you've considered, including references to existing open and closed feature requests in this repository.

Additional information

Requested for RediSearch.
@oranagra @MeirShpilraien @K-Jo

[MeirShpilraien]

I think that another option we should consider is a reply builder, I imagine something like:

rep = RM_CreateReply(ctx);
RM_ReplyBuilderAddCString(rep, msg);
RM_ReplyWithReplyObject(ctx, rep);

This will allow a module to maintain multiple replies objects and only at the end decide which one to send as reply and which one to discard (unlike the current suggestion where its only possible to maintain and discard the current reply).

[yossigo]

@MeirShpilraien I like the reply builder suggestion, we could probably leverage RedisModuleCallReply for that.

[oranagra]

considering Meir's suggestion leverages the existing RedisModuleCallReply, i also support that.

just some possibly missing context about the suggestion at the top:
the idea was to leverage deferred replies.
i.e. same as REDISMODULE_POSTPONED_LEN creates a new node in the reply list, and keeps a pointer to it and later updates it,
in that suggestion we could later trim anything after that node, or just trim that one empty node.

[madolson]

We have something similar at AWS. We have an API which allows you to get "blocks" to directly serialize data into, which avoids having to serialize data into a different buffer before calling addReply(). These blocks are composed together and can be submitted as a single reply. The intention isn't quite the same, but since this API requires setting a deferred reply, it might be useful to consider how we could support both of these two use cases.

[oranagra]

@madolson if you can, please suggest an API that can avoid the extra copying.

[madolson]

Hi, here is the API we basically have internally. While reviewing it I noticed a couple of todo's pending before open sourcing that, so if we like this API proposal I can look into it.

/*
 * Chunked String Reply Facility
 *
 * Generally, Redis string replies are copies of in-memory strings. The existing addReplyXxxxx
 * functions are properly efficient for that case. However, some modules generate string replies
 * that are constructed from other internal structures. In these cases, the
 * standard module interface is quite expensive, requiring two copies of the data.
 * (One copy to extract the data from the internal data structure and put it into a
 * RedisModuleString and then a second copy to put that string into the reply buffer chain.)
 *
 * These APIs provide an efficient mechanism to construct a string reply as a series of
 * arbitrarily sized chunks of data. The protocol for usage is as follows.
 *
 * The module creates a RedisModuleChunkedStringReply object, fills it in with one or more chunks of data and
 * submits it to the output buffer. A module can have any number of these objects in existence at any time and
 * can add chunks of data to them in any order, etc. Once the object is submitted then ownership transfers to redis
 * and the module is no longer permitted to use it. It's permissible to abandon the construction of a chunked reply at
 * any time, however, the module is responsible for explicitly destructing it (see AMZ_ChunkedStringReply_destroy).
 *
 * The normal sequence for an individual chunk is:
 *
 *RedisModule_AMZ_ChunkedStringReply_create(ctx, &chunkedReply);
 *      while ((*More data left in string*)) {
 *          void *data = RedisModule_ChunkedStringReply_beginChunk(chunkedReply, chunkSize);
 *          (* Fill in up to chunkSize bytes in "data" *);
 *          RedisModule_AMZ_ChunkedStringReply_endChunk(chunkedReply, usedSize);
 *      }
 * RedisModule_AMZ_ChunkedStringReply_submit(ctx, &chunkedReply, prefix, prefixLength);
 *
 * Note that there is no requirement that each chunk of data completely fill up the requested chunkSize
 * This can simplify module logic as it won't have to split internal objects if it's inconvenient.
 *
 * Modules don't have to worry about exceeding client output buffer limits. This is automatically
 * handled by this logic. If the client output buffer limit is exceeded, then the completed chunks
 * are simply freed rather than being appended to the reply. This doesn't save the time for
 * generating the excess data, but reduces complexity by eliminating a difficult to test and rare case.
 *
 * On the module side, there is an expected pattern as follows.
 *
 * It's assumed that the client will have a small preallocated buffer (typically stack allocated). Data is
 * streamed into the preallocated buffer. If the whole response fits into the buffer, the no ChunkedStringReply
 * needs to be created, the preallocated buffer can simply be handed to RM_ReplyWithStringBuffer. However, if
 * the preallocated buffer is not sufficient, then a ChunkedStringReply object is generated to contain the overflow.
 * When the whole response is complete, then the ChunkedStringReply object is submitted with the preallocated buffer
 * as the prefix. The resulting response is the concatenation of the pair.
 *
 * A further optimization for the client is to size the preallocated to match the available space in the client
 * output buffer (leaving room for protocol overhead).
 *
 */

struct RedisModuleChunkedStringReply {
    list *chunks;
    size_t replyBytes;      // total bytes (allocated, not used)
    size_t usedBytes;       // total bytes of valid data
    bool chunkOpen;         // Seen a begin, but no end yet. Only used for debugging.
};

/*
 * Create a chunked Reply object
 */
void RedisModuleChunkedStringReply *RM_AMZ_ChunkedStringReply_create(RedisModuleCtx *ctx);

/*
 * Destroy a constructed, but not submitted Reply Object
 */
void RM_AMZ_ChunkedStringReply_destroy(RedisModuleChunkedStringReply *reply);

/*
 * Called to add a new chunk of the reply. The returned chunk will have at least chunkSize available bytes.
 */
void *RM_ChunkedStringReply_beginChunk(RedisModuleChunkedStringReply *reply, size_t chunkSize);

/*
 * Called after data has been filled into the chunk returned from ..._beginChunk.
 * usedSize indicates the number of valid bytes. It must be <= the chunksize from ...beginChunk
 */
void RM_ChunkedStringReply_endChunk(RedisModuleChunkedStringReply *reply, size_t usedSize);

/*
 * Submit a completed ChunkedStringReply Object and a prefix buffer.
 */
void RM_ChunkedStringReply_submit(RedisModuleCtx *ctx, RedisModuleChunkedStringReply **r, const char *prefix, size_t prefixLength);
/*
 * Called to determine how much unused space is available in the current block of the output buffer.
 * This allows optimal conversion to use of the chunked string reply logic.
 */
size_t RM_ClientOutputBuffer_unusedSpaceInCurrentBlock(RedisModuleCtx *ctx);

[MeirShpilraien]

@madolson Correct me if i'm wrong, doesn't the above API means that module needs to push RESP protocol bytes directly?

After discussion with @oranagra we thought we can leverage the RedisModuleCallReply object for this purpose (as suggested by @yossigo). The idea is to allow creating an empty RedisModuleCallReply object and then populate it with data using functions like: RM_CallReplyAddCString(r, str). The population functions of the CallReply should match the RM_ReplyWith* functions. After creating the call reply it will be possible to send it using RM_ReplyWithCallReply or discard it using RM_FreeCallReply.

The advantage of this approach is that the module does not need to be aware of the protocol used by the current client (resp2 or resp3). We can also have RM_CallReplyAddRaw but I believe it will only be used for very advance use-cases.

To avoid buffer copies, we can match the underline data structure of the client reply buffers and the CallReply underline structure (client reply list can either contains sds instead of clientReplyBlock or CallReply can contains clientReplyBlock instead of sds). This way the client can simply take ownership of the CallReply underline structure instead of copy it. In addition, RM_Call can take ownership of the client reply buffers and use it to construct to CallReply object (again instead of copy).

@madolson @oranagra @yossigo if you agree I can try make a PR and continue the discussion there, let me know what you think.