RedisModule_ReplicateVerbatim doesn't seems to work if called after UnblockClient

[Simone Mosciatti]

Hi Folks,

for my module rediSQL there are two thread.

The main one and the executor one. The main one collect commands and send them to the Executor, the executor actually "execute" the actions on the database.
After the execution is done, some data are saved in memory and control is returned to the main thread that will perform IO and send the result (saved in memory by the executor) to the client.

Now I am adding replication.

I would like to avoid to replicate commands that don't modify the databases (namely SELECTs).
What I am doing now in the Executor it is to understand if the comand will modify the database, save such information in a boolean, execute the command, unblock the client, now control pass again to the main thread.
At this point the main thread check if everything went right and if everythings looks good and the command modify the database it runs a RedisModule_ReplicateVerbatim.
However this doesn't seems to work.

I use `tail -f appendonly.aof` and I see all the standard comands but I don't see the one executed following the procedure above.

Am I doing something wrong?

Cheers,

Simone

[Dvir Volk]

IIRC There was a problem that after returning from unblocking the client, the command argument strings are no longer valid. 

I remember discussing this with Salvatore but don't recall the outcome. 

One workaround is to duplicate the command arguments into a new array of redis strings, then call RM_Replicate with the exact arguments. 

[Dvir Volk]

Ok, this is the relevant issue - that problem has been fixed and argc/argv are now valid after unblocking the client.

https://github.com/antirez/redis/issues/3844

Can you try and create a sample module that reproduces the problem?

i.e. has a command that blocks the client, unblocks it and then calls RM_ReplicateVerbatim?

[Simone Mosciatti]

Hi Dvir,

Thanks so much for your help :)

Actually I am not using automemory, so I am not quite sure that the problem is the same.

Yes I can definitely try to re-produce the problem, I am afraid it will take a little while but I will try to do it asap.

Thanks

[Simone Mosciatti]

Hi All,

I am still debugging the issues, but I can't really figure it out.

I am trying to follow what happens when we call RM_ReplicateVerbatim(ctx), however I get lost in the flow.

Now, this is the flow:

```
int RM_ReplicateVerbatim(RedisModuleCtx *ctx) {
     alsoPropagate(ctx->client->cmd,ctx->client->db->id,
         ctx->client->argv,ctx->client->argc,
         PROPAGATE_AOF|PROPAGATE_REPL);
     return REDISMODULE_OK;
}
```
ReplicateVerbatim calls alsoPropagate():

```
void alsoPropagate(struct redisCommand *cmd, int dbid, robj **argv, int argc,
int target)
{
    robj **argvcopy;
    int j;
    if (server.loading) return; /* No propagation during loading. */
  
    argvcopy = zmalloc(sizeof(robj*)*argc);
    for (j = 0; j < argc; j++) {
        argvcopy[j] = argv[j];
        incrRefCount(argv[j]);
    }
    redisOpArrayAppend(&server.also_propagate,cmd,dbid,argvcopy,argc,target);
}
```

server.loading it equal to zero, incrRefCount doesn't do anything interesting so let's explore redisOpArray


```

int redisOpArrayAppend(redisOpArray *oa, struct redisCommand *cmd, int dbid,
                       robj **argv, int argc, int target)
{
    redisOp *op;

    oa->ops = zrealloc(oa->ops,sizeof(redisOp)*(oa->numops+1));
    op = oa->ops+oa->numops;
    op->cmd = cmd;
    op->dbid = dbid;
    op->argv = argv;
    op->argc = argc;
    op->target = target;
    oa->numops++;
    return oa->numops;
}
```

This command just add the whole history of command to replicate to server.also_propagate, however, nothing happens after that.

On Monday, October 16, 2017 at 3:40:21 PM UTC+2, Dvir Volk wrote:

[Simone Mosciatti]

Hi all,

I finally spend some time investigating the issue.

Since we talked about it quite a while ago, and since my understanding of the problem changed quite a bit, let me re-word the problem.

Redis does not replicate the commands if the call to RM_Replicate(Verbatim) happens after the client got blocked.
So it is impossible to replicate a command in a different thread (eg. call ClientBlock, then Replicate(Verbatim), the ClientUnblock) or in the main redis thread after unblocking in the reply function.

I explored the codebase and it seems to me that the case of replicate after a block, simply, is not considered.

Now I am wondering if this is by-design? Or it is suppose to work?

Here the files (from helloblock.c) that show the problem:
https://github.com/siscia/redis/blob/faster_unblock/src/modules/helloblock.c

And here the (2 lines long) diff:
https://github.com/siscia/redis/commit/1d3bfbb31499794003ae3674250f01f5c7261718
 
To replicate the issue, just compile the module and load it.
Then set the appendonly to yes `config set appendonly yes`
Set a value `SET A 3`
Finally call  the module `HELLO.BLOCK 1 3000`
Confirm the log from the module (It should appear something like: "# <helloblock> It should replicate")

Exploring the AOF file you will not see the log of `HELLO.BLOCK` but you will see the one of `SET`

Happy holidays everybody :)

Cheers,

Simone