Describe the bug

Manipulating keys with Redis Module functions like RedisModule_StringSet don't increment redisServer.dirty, and therefore will never initiate an RDB snapshot like a normal Redis command would.

To reproduce

For context, when you set a key normally in Redis:

127.0.0.1:6379> set key value
OK

And have RDB snapshotting configured, moments later you'll see a message like this one to indicate that a snapshot has been saved to disk.

48665:M 21 Feb 2021 12:15:26.450 * 1 changes in 10 seconds. Saving...
48665:M 21 Feb 2021 12:15:26.450 * Background saving started by pid 48667
48667:C 21 Feb 2021 12:15:26.452 * DB saved on disk

If you use a command installed via Redis Module, say redis-cell (this is one I maintain):

127.0.0.1:6379> CL.THROTTLE user123 15 1 600 1

You'll notice that no RDB snapshot is taken. This appears to be because redisServer.dirty, which is how Redis tracks changes that need to be saved, is not incremented by the Redis Modules API.

You can reproduce with any Redis Module, but if you need one, you can try installing redis-cell, which is quite straightforward. Instructions for that here:

https://github.com/brandur/redis-cell#install

I posted this to the Redis mailing list first where it seems to have been confirmed to be a bug rather than some other API misuse or something along those lines.

Expected behavior

When a key is modified from a Redis Module, dirty should be incremented somewhere along the way, and from there an RDB snapshot can potentially be initiated.

Additional information

Original bug reported on redis-cell here:

https://github.com/brandur/redis-cell/issues/52

Comment From: oranagra

This is by design. By default, module commands are also not being propagated to replicas and AOF. Not even if the module uses RedisModule_Calll() (unless using the ! modifier). Instead the module needs to explicitly call RedisModule_Replicate() or RedisModule_ReplicateVerbatim(). When doing so, it does both insert a command into the replication stream / AOF, and also increments the dirty counter.

Comment From: brandur

@oranagra Thanks for the quick response! I've patched my module to use RedisModule_ReplicateVerbatim, and confirmed it fixes the problem.

But hmm, are you sure that this shouldn't be considered somewhat of a bug as well? I understand that calling one of the replication functions is necessary to trigger replication to replicas/AOF, but it's not obvious (from the standpoint of an external developer like myself) that that should also be required to cause RDB snapshots to work properly. These concepts are all related, but still orthogonal.

Comment From: oranagra

I'm quite sure this is by design. Modules are getting full control on how they use redis, and that come with a price (which is that many things don't happen automatically). I suppose we may need to improve the documentation though. In invite you to review the current docs and propose changes. This can either be the API reference generated from module.c, or maybe the intro in https://redis.io/topics/modules-intro

Comment From: brandur

Alright, will take a look at those docs to see if there might be improvements to be made. Thanks!