Redis [NEW] Add RedisModule_RegisterCommandPostProcessor feature

The problem/use-case that the feature addresses I want to develop a module to add ttl to the key which after write command is executed. But, RedisModule_RegisterCommandFilter seems is called before the command is executed, which cannot meet my needs

Description of the feature Add RedisModule_RegisterCommandPostProcessor to execute user-defined logic after the redis command is executed

Comment From: yossigo

@jjz921024 One way to do this is create your own command, which performs the sequence of operations as required, and use the command filter to intercept the original command and redirect it to yours.

Comment From: oranagra

@yossigo this approach can cause some side effects / issues (due to the extra nesting level). and also maybe issues with odd commands (e.g. BLPOP, WATCH)

Comment From: guybe7

we implemented something similar in our fork:

typedef void *(*RedisModulePreCommandFunc)(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
typedef void (*RedisModulePostCommandFunc)(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, void *privdata);

int RM_RegisterPreCommandFunc(RedisModuleCtx *ctx, RedisModulePreCommandFunc cb) {
    ctx->module->pre_command_cb = cb;
    return REDISMODULE_OK;
}

int RM_RegisterPostCommandFunc(RedisModuleCtx *ctx, RedisModulePostCommandFunc cb) {
    ctx->module->post_command_cb = cb;
    return REDISMODULE_OK;
}

// from RedisModuleCommandDispatcher
/* Trigger pre-command hook */
void *privdata = NULL;
if (ctx.module->pre_command_cb)
    privdata = ctx.module->pre_command_cb(&ctx,c->argv,c->argc);

/* Execute command */
cp->func(&ctx,(void**)c->argv,c->argc);

/* Trigger post-command hook */
if (ctx.module->post_command_cb)
    ctx.module->post_command_cb(&ctx,c->argv,c->argc,privdata);

@jjz921024 will that fit your case?

Comment From: jjz921024

@jjz921024 One way to do this is create your own command, which performs the sequence of operations as required, and use the command filter to intercept the original command and redirect it to yours.

@yossigo This approach seems to create a new command for each command？there are a more general approach?

Comment From: jjz921024

@guybe7 yeah，it fit my case

But, I think the result of comand execution to be passed to the post_command_cb. The post_command_cb needs to decide whether to execute based on the result

for example, exec hset command to a string key，it will result WRONGTYPE err while an error occurs, the post_command_cb is no need to execute

Comment From: yossigo

@oranagra You are correct, I should have mentioned it's not applicable in all cases.

Comment From: oranagra

Passing the result to the post callback is problematic but maybe we can rely on the error stats (also part of command stats these days) to provide an indication of success / failure.

Comment From: oranagra

@jjz921024 would an error indication (whether or not the command replied with -<errcode>) be enough? note that it might not always be sufficient, e.g. SET, SETNX, MSETNX returning 0 or Nil, won't count as a failure. @guybe7 do you wanna make a PR?

Comment From: jjz921024

@oranagra hi for my needs, it may be not enough. actually, I want to know the command is have an effect (if SETNX returning 0, that means nothing happend)

Comment From: oranagra

alright, if what you care for is if the data set changed, then maybe you can sample server.dirty before and after?

Comment From: jjz921024

@oranagra @yossigo Hello, I've submitted a PR #12680 for this issue. Could you please review it when you have some time? If there are any adjustments needed in this PR, please let me know, and I will be happy to make them. thank