Once https://github.com/redis/redis/pull/9656 is merged we need to come up with a module API to allow module to provide extra information about their commands
Ideally we would like it to be as declarative as possible, rather than calling a dedicated API for every tiny bit of information
Comment From: guybe7
i will soon post the two options we are considering: procedural vs. declarative
Comment From: guybe7
OPTION 1: procedural
New APIs:
RM_SetCommandArity - set the command arity. this one also allows the server to filter commands before they reach the module code
RM_SetCommandSummary - set the command summary
RM_SetCommandDebutVersion - set the debut version
RM_SetCommandComplexity - set the complexity string
RM_SetCommandHints - set the hints
RM_AppendCommandHistoryEntry - add a history entry
RM_CreateCommandArg - create an opaque structure, representing an argument
RM_AppendSubarg - add a sub-argument to an existing argument
RM_AppendArgToCommand - add an existing argument to a command
New definitions in redismodule.h:
typedef enum {
REDISMODULE_ARG_TYPE_STRING,
REDISMODULE_ARG_TYPE_INTEGER,
REDISMODULE_ARG_TYPE_DOUBLE,
REDISMODULE_ARG_TYPE_KEY, /* A string, but represents a keyname */
REDISMODULE_ARG_TYPE_PATTERN,
REDISMODULE_ARG_TYPE_UNIX_TIME,
REDISMODULE_ARG_TYPE_PURE_TOKEN,
REDISMODULE_ARG_TYPE_ONEOF, /* Must have sub-arguments */
REDISMODULE_ARG_TYPE_BLOCK /* Must have sub-arguments */
} RedisModuleCommandArgType;
#define REDISMODULE_CMD_ARG_NONE (0)
#define REDISMODULE_CMD_ARG_OPTIONAL (1<<0) /* The argument is optional (like GET in SET command) */
#define REDISMODULE_CMD_ARG_MULTIPLE (1<<1) /* The argument may repeat itself (like key in DEL) */
#define REDISMODULE_CMD_ARG_MULTIPLE_TOKEN (1<<2) /* The argument may repeat itself, and so does its token (like `GET pattern` in SORT) */
Example code (for XADD):
if (RedisModule_CreateCommand(ctx,"cmdintrospection.xadd",cmd_xadd,"write deny-oom random fast",0,0,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
RedisModuleCommand *xadd = RedisModule_GetCommand(ctx,"cmdintrospection.xadd");
if (RedisModule_AddCommandKeySpec(xadd,"write",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(xadd,spec_id,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecFindKeysRange(xadd,spec_id,0,1,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
/* NOTE: All versions specified should be the module's versions, not Redis`!
* We use Redis versions in this example for the purpose of testing (comparing
* the output with the output of the vanilla XADD */
RedisModule_SetCommandArity(xadd, -5);
RedisModule_SetCommandSummary(xadd, "Appends a new entry to a stream");
RedisModule_SetCommandDebutVersion(xadd, "5.0.0");
RedisModule_SetCommandComplexity(xadd, "O(1) when adding a new entry, O(N) when trimming where N being the number of entries evicted.");
RedisModule_AppendCommandHistoryEntry(xadd, "6.2", "Added the `NOMKSTREAM` option, `MINID` trimming strategy and the `LIMIT` option.");
RedisModule_AppendCommandHistoryEntry(xadd, "7.0", "Added support for the `<ms>-*` explicit ID form.");
RedisModule_SetCommandHints(xadd, "hint1 hint2 hint3");
// Trimming args
RedisModuleCommandArg *trim_maxlen = RedisModule_CreateCommandArg("maxlen", REDISMODULE_ARG_TYPE_PURE_TOKEN, -1, "MAXLEN", NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *trim_minid = RedisModule_CreateCommandArg("minid", REDISMODULE_ARG_TYPE_PURE_TOKEN, -1, "MINID", NULL, "6.2", REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *trim_startegy = RedisModule_CreateCommandArg("strategy", REDISMODULE_ARG_TYPE_ONEOF, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModule_AppendSubarg(trim_startegy, trim_maxlen);
RedisModule_AppendSubarg(trim_startegy, trim_minid);
RedisModuleCommandArg *trim_exact = RedisModule_CreateCommandArg("equal", REDISMODULE_ARG_TYPE_PURE_TOKEN, -1, "=", NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *trim_approx = RedisModule_CreateCommandArg("approximately", REDISMODULE_ARG_TYPE_PURE_TOKEN, -1, "~", NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *trim_op = RedisModule_CreateCommandArg("operator", REDISMODULE_ARG_TYPE_ONEOF, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_OPTIONAL);
RedisModule_AppendSubarg(trim_op, trim_exact);
RedisModule_AppendSubarg(trim_op, trim_approx);
RedisModuleCommandArg *trim_threshold = RedisModule_CreateCommandArg("threshold", REDISMODULE_ARG_TYPE_STRING, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *trim_count = RedisModule_CreateCommandArg("count", REDISMODULE_ARG_TYPE_INTEGER, -1, "LIMIT", NULL, "6.2", REDISMODULE_CMD_ARG_OPTIONAL);
RedisModuleCommandArg *trimming = RedisModule_CreateCommandArg("trim", REDISMODULE_ARG_TYPE_BLOCK, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_OPTIONAL);
RedisModule_AppendSubarg(trimming, trim_startegy);
RedisModule_AppendSubarg(trimming, trim_op);
RedisModule_AppendSubarg(trimming, trim_threshold);
RedisModule_AppendSubarg(trimming, trim_count);
// ID arg
RedisModuleCommandArg *id_auto = RedisModule_CreateCommandArg("auto_id", REDISMODULE_ARG_TYPE_PURE_TOKEN, -1, "*", NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *id_given = RedisModule_CreateCommandArg("id", REDISMODULE_ARG_TYPE_STRING, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *id = RedisModule_CreateCommandArg("id_or_auto", REDISMODULE_ARG_TYPE_ONEOF, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModule_AppendSubarg(id, id_auto);
RedisModule_AppendSubarg(id, id_given);
// Fields and values
RedisModuleCommandArg *field = RedisModule_CreateCommandArg("field", REDISMODULE_ARG_TYPE_STRING, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *value = RedisModule_CreateCommandArg("value", REDISMODULE_ARG_TYPE_STRING, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_NONE);
RedisModuleCommandArg *fieldsvalues = RedisModule_CreateCommandArg("field_value", REDISMODULE_ARG_TYPE_BLOCK, -1, NULL, NULL, NULL, REDISMODULE_CMD_ARG_MULTIPLE);
RedisModule_AppendSubarg(fieldsvalues, field);
RedisModule_AppendSubarg(fieldsvalues, value);
// Key
RedisModuleCommandArg *key = RedisModule_CreateCommandArg("key", REDISMODULE_ARG_TYPE_KEY, 0, NULL, NULL, NULL, REDISMODULE_CMD_ARG_NONE);
// NOMKSTREAM
RedisModuleCommandArg *nomkstream = RedisModule_CreateCommandArg("nomkstream", REDISMODULE_ARG_TYPE_PURE_TOKEN, -1, "NOMKSTREAM", NULL, "6.2", REDISMODULE_CMD_ARG_OPTIONAL);
// Append all args
RedisModule_AppendArgToCommand(xadd, key);
RedisModule_AppendArgToCommand(xadd, nomkstream);
RedisModule_AppendArgToCommand(xadd, trimming);
RedisModule_AppendArgToCommand(xadd, id);
RedisModule_AppendArgToCommand(xadd, fieldsvalues);
Comment From: guybe7
OPTION 2: declerative
New APIs:
RM_SetCommandInfo
New definitions in redismodule.h:
typedef enum {
REDISMODULE_ARG_TYPE_STRING,
REDISMODULE_ARG_TYPE_INTEGER,
REDISMODULE_ARG_TYPE_DOUBLE,
REDISMODULE_ARG_TYPE_KEY, /* A string, but represents a keyname */
REDISMODULE_ARG_TYPE_PATTERN,
REDISMODULE_ARG_TYPE_UNIX_TIME,
REDISMODULE_ARG_TYPE_PURE_TOKEN,
REDISMODULE_ARG_TYPE_ONEOF, /* Must have sub-arguments */
REDISMODULE_ARG_TYPE_BLOCK /* Must have sub-arguments */
} RedisModuleCommandArgType;
#define REDISMODULE_CMD_ARG_NONE (0)
#define REDISMODULE_CMD_ARG_OPTIONAL (1<<0) /* The argument is optional (like GET in SET command) */
#define REDISMODULE_CMD_ARG_MULTIPLE (1<<1) /* The argument may repeat itself (like key in DEL) */
#define REDISMODULE_CMD_ARG_MULTIPLE_TOKEN (1<<2) /* The argument may repeat itself, and so does its token (like `GET pattern` in SORT) */
typedef enum {
REDISMODULE_KSPEC_BS_UNKNOWN,
REDISMODULE_KSPEC_BS_INDEX,
REDISMODULE_KSPEC_BS_KEYWORD
} RedisModuleKeySpecBeginSearchType;
typedef enum {
REDISMODULE_KSPEC_FK_UNKNOWN,
REDISMODULE_KSPEC_FK_RANGE,
REDISMODULE_KSPEC_FK_KEYNUM
} RedisModuleKeySpecFindKeysType;
#define REDISMODULE_CMD_KEY_WRITE (1ULL<<0)
#define REDISMODULE_CMD_KEY_READ (1ULL<<1)
#define REDISMODULE_CMD_KEY_INCOMPLETE (1ULL<<2) /* meaning that the keyspec might not point out to all keys it should cover */
typedef struct RedisModuleCommandArg {
const char *name;
RedisModuleCommandArgType type;
int key_spec_index;
const char *token;
const char *summary;
const char *since;
int flags;
struct RedisModuleCommandArg *subargs;
} RedisModuleCommandArgV1;
#define RedisModuleCommandArg RedisModuleCommandArgV1
typedef struct RedisModuleCommandHistoryEntry {
const char *since;
const char *changes;
} RedisModuleCommandHistoryEntryV1;
#define RedisModuleCommandHistoryEntry RedisModuleCommandHistoryEntryV1
typedef struct RedisModuleCommandKeySpec {
uint64_t flags;
RedisModuleKeySpecBeginSearchType begin_search_type;
union {
struct {
/* The index from which we start the search for keys */
int pos;
} index;
struct {
/* The keyword that indicates the beginning of key args */
const char *keyword;
/* An index in argv from which to start searching.
* Can be negative, which means start search from the end, in reverse
* (Example: -2 means to start in reverse from the panultimate arg) */
int startfrom;
} keyword;
} bs;
RedisModuleKeySpecFindKeysType find_keys_type;
union {
/* NOTE: Indices in this struct are relative to the result of the begin_search step!
* These are: range.lastkey, keynum.keynumidx, keynum.firstkey */
struct {
/* Index of the last key.
* Can be negative, in which case it's not relative. -1 indicating till the last argument,
* -2 one before the last and so on. */
int lastkey;
/* How many args should we skip after finding a key, in order to find the next one. */
int keystep;
/* If lastkey is -1, we use limit to stop the search by a factor. 0 and 1 mean no limit.
* 2 means 1/2 of the remaining args, 3 means 1/3, and so on. */
int limit;
} range;
struct {
/* Index of the argument containing the number of keys to come */
int keynumidx;
/* Index of the fist key (Usually it's just after keynumidx, in
* which case it should be set to keynumidx+1). */
int firstkey;
/* How many args should we skip after finding a key, in order to find the next one. */
int keystep;
} keynum;
} fk;
} RedisModuleCommandKeySpecV1;
#define RedisModuleCommandKeySpec RedisModuleCommandKeySpecV1
typedef struct RedisModuleCommandInfo {
const char *summary; /* Summary of the command (optional). */
const char *complexity; /* Complexity description (optional). */
const char *since; /* Debut version of the command (optional). */
RedisModuleCommandHistoryEntry *history; /* History of the command */
const char **hints; /* An array of strings that are meant o be hints for clients/proxies regarding this command */
int arity; /* Number of arguments, it is possible to use -N to say >= N */
RedisModuleCommandKeySpec *key_specs;
RedisModuleCommandArg *args;
} RedisModuleCommandInfoV1;
#define RedisModuleCommandInfo RedisModuleCommandInfoV1
Example code (for XADD):
/* XADD history */
RedisModuleCommandHistory XADD_History[] = {
{"6.2","Added the `NOMKSTREAM` option, `MINID` trimming strategy and the `LIMIT` option."},
{"7.0","Added support for the `<ms>-*` explicit ID form."},
{0}
};
/* XADD hints */
const char *XADD_Hints[] = {
"hint1",
"hint2",
"hint3",
{0}
}
/* XADD trim strategy argument table */
RedisModuleCommandArg XADD_trim_strategy_Subargs[] = {
{"maxlen",REDISMODULE_ARG_TYPE_PURE_TOKEN,-1,"MAXLEN",NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{"minid",REDISMODULE_ARG_TYPE_PURE_TOKEN,-1,"MINID",NULL,"6.2",REDISMODULE_CMD_ARG_NONE},
{0}
};
/* XADD trim operator argument table */
RedisModuleCommandArg XADD_trim_operator_Subargs[] = {
{"equal",REDISMODULE_ARG_TYPE_PURE_TOKEN,-1,"=",NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{"approximately",REDISMODULE_ARG_TYPE_PURE_TOKEN,-1,"~",NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{0}
};
/* XADD trim argument table */
RedisModuleCommandArg XADD_trim_Subargs[] = {
{"strategy",REDISMODULE_ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_NONE,.subargs=XADD_trim_strategy_Subargs},
{"operator",REDISMODULE_ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_OPTIONAL,.subargs=XADD_trim_operator_Subargs},
{"threshold",REDISMODULE_ARG_TYPE_STRING,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{"count",REDISMODULE_ARG_TYPE_INTEGER,-1,"LIMIT",NULL,"6.2",REDISMODULE_CMD_ARG_OPTIONAL},
{0}
};
/* XADD id_or_auto argument table */
RedisModuleCommandArg XADD_id_or_auto_Subargs[] = {
{"auto_id",REDISMODULE_ARG_TYPE_PURE_TOKEN,-1,"*",NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{"id",REDISMODULE_ARG_TYPE_STRING,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{0}
};
/* XADD field_value argument table */
RedisModuleCommandArg XADD_field_value_Subargs[] = {
{"field",REDISMODULE_ARG_TYPE_STRING,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{"value",REDISMODULE_ARG_TYPE_STRING,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{0}
};
/* XADD argument table */
RedisModuleCommandArg XADD_Args[] = {
{"key",REDISMODULE_ARG_TYPE_KEY,0,NULL,NULL,NULL,REDISMODULE_CMD_ARG_NONE},
{"nomkstream",REDISMODULE_ARG_TYPE_PURE_TOKEN,-1,"NOMKSTREAM",NULL,"6.2",REDISMODULE_CMD_ARG_OPTIONAL},
{"trim",REDISMODULE_ARG_TYPE_BLOCK,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_OPTIONAL,.subargs=XADD_trim_Subargs},
{"id_or_auto",REDISMODULE_ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_NONE,.subargs=XADD_id_or_auto_Subargs},
{"field_value",REDISMODULE_ARG_TYPE_BLOCK,-1,NULL,NULL,NULL,REDISMODULE_CMD_ARG_MULTIPLE,.subargs=XADD_field_value_Subargs},
{0}
};
RedisModuleKeySpecs XADD_KeySpecs[] = {
{REDISMODULE_CMD_KEY_WRITE,REDISMODULE_KSPEC_BS_INDEX,.bs.index={1},REDISMODULE_KSPEC_FK_RANGE,.fk.range={0,1,0}},
{0}
}
RedisModuleCommandInfo XADD_Info = {
"Appends a new entry to a stream",
"O(1) when adding a new entry, O(N) when trimming where N being the number of entries evicted.",
"5.0.0",
XADD_History,
XADD_Hints,
-5,
XADD_KeySpecs,
XADD_Args
};
if (RedisModule_CreateCommand(ctx,"cmdintrospection.xadd",cmd_xadd,"write deny-oom random fast",0,0,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
RedisModuleCommand *xadd = RedisModule_GetCommand(ctx,"cmdintrospection.xadd");
RedisModule_SetCommandInfo(xadd, &XADD_Info);
Comment From: guybe7
@oranagra @yossigo @itamarhaber thoughts?
Comment From: oranagra
i'm curious to know how this declarative mode works with modules in other languages (Rust, Java?). @itamarhaber can you grab the attention of people who wrote such modules
Comment From: MeirShpilraien
@oranagra on Rust you can create structs which are compatible with C so it should be fine. Java? is there a java wrapper to RedisModuleApi to write modules in java?
Comment From: itamarhaber
I haven't heard about a Java wrapper, but have heard of Rust and CPP (Nimlang & Zig too, but probably too niche). I would like to think that CPP is compatible with some of C, so both options would be OK but maybe @sewenew can pipe in. Also, perhaps @madolson can channel her sources. Also pinged the usual suspects for review.
In terms of personal preference, I prefer the procedural option.
Comment From: madolson
Personal preference is for the declarative proposal.
We build some internal Rust modules for some features at AWS, and I don't see any compatibility issue for Rust. The folks I pinged about A/B were mostly split 50/50, mostly along the lines of either preferring it to look more like other module APIs (procedural) or the declarative being easier to read.
Comment From: sewenew
@itamarhaber The code should work with C++.
I don't think the procedural proposal is a good idea. Because it will add multiple APIs, and there're already too many module APIs...
I prefer a simplified version of declerative proposal:
int RedisModule_SetCommandInfo(RedisModuleCommand *cmd, const char *info, size_t info_len);
info and info_len specifies a JSON string describing the command information. The specification should contains some required fields, e.g. name, summary, complexity, while module developer could also set some module specific information in the JSON string.
Regards
Comment From: oranagra
so far we have: * procedural: Itamar * declarative: Oran, Madelyn, sewenew
@yossigo @guybe7 @MeirShpilraien please mention your preference.
Comment From: MeirShpilraien
@oranagra I prefer the decalarative API.
Comment From: oranagra
for the record, wc on the examples:
73 347 5158 xadd.procedural.txt
83 190 3633 xadd.declerative.txt
Comment From: yossigo
I also prefer the declarative API. I guess it's a matter of personal style mostly, but if I have to defend it with more objective arguments:
- Less code (as in
.textbytes, not LoC) - Less rope to hang yourself with (assuming
RM_SetCommandInfo()is fool proof) - Error handling and cleanup is a one liner.
- Versioning is not only better, but also more consistent with other parts of the API (or at least, the better ones).
Comment From: zuiderkwast
I prefer declarative over procedural, but I think we need to dive into the details and consider the pros and cons. Perhaps there are other options too. Here are some thoughts:
-
Structs containing tagged unions (as in RedisModuleCommandKeySpec) is a bit complex and can be messed up by users. A procedural API can protect users against that.
-
The helper variables in the declarative example makes it less clean. A single nested JSON-like structure would be nicer. I think it's possible to write everything in one C99 compound literal like this:
C RedisModuleCommandInfo XADD_Info = { .summary = "...", .history = (RedisModuleCommandHistoryEntry[]) { (RedisModuleCommandHistoryEntry){"6.2", "Added the `NOMKSTREAM` option, `MINID` trimming strategy and the `LIMIT` option."}, (RedisModuleCommandHistoryEntry){"7.0","Added support for the `<ms>-*` explicit ID form."}, {0} }, ... }@sewenew's idea of using JSON in strings is nice, but it's not syntax checked at compile time and we don't have a JSON parser in Redis so we would have to add that as a dependency, which is unfortunate. A nested compound literal is close to the JSON structure.
What if we can provide a way for module authors to write the command specs in separate JSON files and provide a tool (like our python script) to convert it to a C file which can be
#included in the module? -
A struct is extensible only if the users use the syntax C99 style
.field = value, i.e. I think we should use this style in the docs and examples and document it as the forward compatible way to write. I'm not sure the structs with "V1" in the names really provide any benefit in this sense, other than that the module will simply break if we ever change it to "V2". -
We don't have a way to include
structand other type declarations in the docs, as we do for API functions. For now, the best we can do is to write out the struct definitions manually in the docs. -
In the call
RedisModule_SetCommandInfo(xadd, &XADD_Info), the structure is copied by recursively byRM_SetCommandInfo, I suppose. In a procedural API, we wouldn't need to do that.
Comment From: oranagra
we discussed this in the core-team meeting, and concluded we wanna proceed with the declarative approach. @zuiderkwast i know @guybe7 is busy, maybe you can take this forward?