In this issue I'll try to provide a design draft for two features that are very desirable in the modules systems API:

  1. An API that allows the module to know about important events that are happening in the server.
  2. An API that allows to store into an RDB file, and load back, data which is private to module and has nothing to do with the key space of Redis. Note that this is very different compared to storing module private values as defined by module-exported data types.

Naming

It is very important for features to have names, otherwise they cannot effectively be communicated among the Redis core community, nor to the users of Redis. We happen to already have APIs doing notifications, for instance:

RedisModule_SubscribeToKeyspaceEvents()

Fortunately the previous API specifies clearly that we are talking about keyspace events, this is surely an advantage in order to pick future good names. I propose the following two names for the two above features:

  1. Server events notifications API.
  2. RDB modules private data API.

Note that the second feature is not called modules aux fields because it does not use the auxiliary fields inside RDB. Instead the RDB is already designed to have module-specific data, even if this feature is not used right now, but only implemented to the degree that allows it to be future-compatible. The mechanism used is much solid than using AUX fields.

Server events notifications

Modules will end having a huge number of callbacks they can subscribe to. This is unfortunately but there is no way out: if we want the modules system to be able to do a lot, and at the same time we want Redis to remain simple, we have to provide modules some way to hook into different subsystems. So the temptation of using a single hooking mechanism implementation inside Redis is big. However different hooks and callbacks require different kinds of functions, arguments, opaque structures returned and so for.

However there are many events a module may want to know that is more or less stateless, and that there is nothing the module should be able to do in such context, if not calling the usual APIs. All this cases can be served via a single API, that is indeed, this server events notification API we are going to show here.

There are a number of examples in the following list:

  • RDB persistence started
  • RDB persistence ended
  • AOF persistence started
  • AOF persistence ended
  • The server is ready for shutdown
  • The server just flushed all its data
  • A server "tick" elapsed (serverCron() is going to be called)
  • A new module was loaded
  • A module was unloaded
  • The server changed role
  • The server got a new replica in online status
  • The instance is a replica and the connection with the master is no longer valid

And so forth. All such events should have something in common:

  1. They are rarely occurring, so even running the list of currently loaded modules to check if they want such event delivered is not going to be a problem. This is, for instance, why there is no "client disconnected" event in the list above (such feature will be added with a different API for multiple reasons).
  2. There is no state to communicate.

This means that the implementation can just be obtained by having a new field in the RedisModule structure itself, with bits set for every different condition. Then a single API inside the core will deliver the messages: such API can run all the modules loaded so far, since, to start, we usually have very few modules loaded, not even in the order of tens often (no latency issues). Secondly because such events are rarely occurring so anyway the time spent in total is small (no big total CPU usage issues).

The above API should be exported to modules with something like that:

RedisModule_SubscribeToServerEvents(ctx, REDISMODULE_SE_RDB_SAVE_START | REDIS..., myCallBack);

The callback argument may be NULL in order to unsubscribe. Note that in order to subscribe or unsubscribe to multiple events we can just use bitwise OR of the conditions, or multiple calls.

Inside the core a similar function will be called when needed:

notifyModulesOnServerEvent(REDISMODULE_SE_EMPTYDB);

This will just run the list of modules for matching ones.

Important: the module should use a single callback for all the server notifications. Setting a different callback will route all the new notifications to the new callback. This should be clearly documented.

Modules private data (RDB MPD fields)

Modules often have the need to register substantial amount of data inside the RDB file. Such data may often be about keys, but they may not necessarily be about the key value itself. For instance a module adding expires to Redis hashes may not necessarily want to replace the hash implementation, but instead may want to simply record additional information about each hash key.

Moreover certain times, when the module private data to store inside the RDB is about keys that we will load, there are cases where it is a lot more comfortable to store the data before the key itself, so that the callback of the key loaded will be invoked later, after we already loaded our private data, and certain other times we may want to do the reverse, that is, store the data at the end (for instance in the case of the hash fields expires this may make sense), so that we may load the additional module-private data only after the key space is already populated inside Redis.

This need was sensed time ago, so Redis 5 already shipped with the internal RDB modifications to allow for such feature, but without actually implementing it. See the RDB_OPCODE_MODULE_AUX opcode inside rdb.c for more information.

Now it is time to actually implement this system inside the module system.

This is the proposed API.

  1. In order to enable the module private data, the following call should be performed:

    RedisModule_EnableMPD(ctx, load_callback, save_callback);

This function will normally be called from the module initialization function, but the implementation will be better if it can be called at any time. Again by calling the function with both callbacks set to NULL, we can turn off the feature.

The callbacks take a RedisModuleIO handle as usually, however there is another integer argument that is passed to the save() callback, that is called "when", because the callback is called actually in two different contexts:

  1. Before the RDB is starting to save keys.
  2. After the RDB already saved all the keys.

So the save callback is called also two times with the when parameter set to either REDISMODULE_MPD_BEFORE_RDB, REDISMODULE_MPD_AFTER_RDB.

IMPORTANT: during a Tel Aviv meeting with my colleagues at Redis Labs we thought that after all there was no need to also call the save callback after each key was persisted, also specifying which key it was. There is to evaluate if we still think likewise. This complicates the implementation and the API but better to be sure than sorry later.

The load callback should probably not need to know if the data is stored before or after, because this should be module-specific and fixed. Moreover Redis has no simple way to know this because of how RDB is structured, and when this is really important, the save callback can save as first data an integer with value 0 or 1 (1 byte overhead).

Inside the callbacks, it will be possible to call the usual functions to store data, such as:

RedisModule_SaveUnsigned)(RedisModuleIO *io, uint64_t value);

And all the other functions of this family. Note that like the module private key values, also the MPD can be parsed with zero knowledge by the rdb-check tool, since each data part is well defined and there is a terminator.

Feedbacks

Please comment in this issue with feedbacks and use cases, and how your use cases will match or mismatch the proposed API and features.

Comment From: oranagra

good plan, but here are a few quick notes:

i know the notification list is only partial, and you gave some good examples about replica state, but I think these should be added to the list right from the start: * aof load start / end * rdb load start / end

I recently said that we may not need these if the MPD is implemented with a different api, but I think, especially about AOF load, the module might wanna know when it's done (maybe trigger some calculation or garbage collection)

I hate to ruin the nice plan about notification not needing an argument, but I think that: * flushdb may need to carry dbid and async flag with it (and my be better called before rather than after, or maybe both?) * it is useful for cron tick to carry tick count and effective hz with it so modules can implement things similar to the active expire mechanism * so together with the above, and the client disconnection notifications, maybe we do need to design a way to pass arguments to the callback.

I think we do still need the key name when loading or saving the key.. think of the hash expiry use case you mentioned. we have a simple piece of code for that which we can PR.

Comment From: yossigo

Hi @antirez, I find this a good plan indeed, especially the clear distinction between high throughput and low throughput APIs. I do think like @oranagra that it might be inevitable to pass some data to some of the event callbacks.

Comment From: MeirShpilraien

@antirez it looks very good and I already can think of many use-cases in which I can use such an api.

Some small comments:

Regarding the hook api, I agree with @oranagra I think we need to supply different callback for each hook. For example, on A new module was loaded\unloaded event, I want to know information about the module (name, version) so I will know if I need to load its shared api. In addition I did not see any mentioning on cluster hook, I think it will be useful to get notification about topology changes (slot map changed, node added/removed, node is down ... ).

Comment From: antirez

Thanks for the feedbacks folks. @oranagra maybe the callback argument could be just a void *auxdata field that the API user will have to cast to the right structure depending on the API call? So our plan for a generic API may still hold.

Comment From: oranagra

@antirez that's fine, but obviously these structs should be defined in our header (otherwise the documentation on the types and order of fields will be a nightmare). and these structs should be packed (attribute) since we need other compilers to handle them correctly.

alternatively (additionally) it can be a union of structs (so that users don't need to do the casting). And also it may be a wise idea to add a version field as first field, so that we have some way to add changes in the future. or actually the module can check the redis version or something like that (we'll need to do non-breaking changes anyway)

Comment From: yossigo

@oranagra this would not be the first time a struct makes it to the header (RedisModuleTypeMethods is already used) and with any sane compiler configuration I believe it should be fine.

If we want to be more defensive and stick to the current API theme it's also possible to provide helper functions to fetch specific info from the void* argument. This will also make it easy to handle versioning, etc.

Comment From: oranagra

nice idea, but since as you pointed we already have a struct, then I think we can continue that path (easier to use structs than get methods). regarding packing, I don't think we should assume same compiler settings, or even same compiler... I think these should be manually padded if needed and strictly packed.

Comment From: AngusP

  1. There is no state to communicate.

This sadly seems like an assumption that will end up proving untrue or needing some funky work-around!

It could be possible to give state to the callback through some other API function, rather than as an argument to the callback - This prevents the need for void pointers, unions of structs or other things that can be a little dangerous! I assume the callback will know which event triggered it if subscribed to more than one, so we can leave it to the programmer to pull the information they wanted based on this, with further RedisModule_* functions that perhaps may only be called within the context of a callback (and return nothing otherwise); for example:

/* enum mostly to get the idea across, whichever type for passing flags */
void myCallback(enum REDISMODULE_SE_FLAGS fs, RedisModuleCtx* ctx)
{
        ...
        if (fs == REDISMODULE_SE_EMPTYDB){
                was_async = RedisModule_SE_FlushType(ctx); // for example
                db = RedisModule_GetSelectedDb(ctx);
        }
}

This also means API changes won't break older modules, or will at least break them at compile time rather than the first time they misuse a void pointer conferring state... This way we can also change anything about how state is communicated on a per-callback basis if desired

edit: added ctx to callback signature

Comment From: yossigo

@AngusP in your example, it is not clear where ctx comes from unless it's a global (which would probably not be a good idea).

I think passing any callback-specific information on the stack (as an argument) is the preferred way, and requiring users to use specific API functions to access this data (like your example) should provide just enough safety and compatibility assurances. A void * or some opaque struct)would still be involved, but I believe the underlying implementation can be made robust enough.

Comment From: AngusP

@yossigo the ctx is just the RedisModuleCtx *ctx that all the existing modules api functions have

Comment From: yossigo

@AngusP RedisModuleCtx is generated on the stack and passed every time to registered module command functions. It can be used and passed as an argument to such callbacks as well, although it would probably be safer to avoid it here to prevent modules from attempting to perform operations that should not be performed in this context (e.g. open keys, etc.).

Comment From: AngusP

@yossigo good point, though the context could be some new RedisModuleEventsCtx or alternatively, as far as I'm aware, all the modules calls (such as opening a key) have a defined return type/value upon failure, which could be what you'll always get back if it's an operation that is disallowed from within a server events callback

Comment From: nkochakian

The current module API is somewhat limited in what can be changed. It would be nice if it was possible to add things such as new classes of keyspace notifications or an entirely new notification subsystem, for example. We are currently using a fork of Redis that adds notifications for PUB/SUB events, which is very important for our application. See: https://github.com/nkochakian/redis/compare/556b2d2bee22d1307e696090c9be10fc10a47cd3...bf770547fcf2c908710750ffa5e82e58c37a7648

Being able to implement something like this through a module would be a lot easier than having to maintain a fork for features that might not be incorporated into Redis itself.

Comment From: antirez

After talking privately with @oranagra, that wants to give a try to implement MPD (module private data) APIs. We reached a few conclusions:

  1. In my top comment where I outline the specification draft, it is not clear if the MPDs are bound or not to the exported module data types. Well, they are, because the only way we are able to lookup a module is by the exported (and registered) data types. In short, data about a given module in the RDB file, is always stored prefixed by the "type ID". Because there is such type ID, we then can lookup the module that registered to handle such type ID.
  2. Because of that, we don't actually need any API to enable MPDs. We just need to extent what we got:
    RedisModuleTypeMethods tm = {
        .version = REDISMODULE_TYPE_METHOD_VERSION,
        .rdb_load = HelloTypeRdbLoad,
        .rdb_save = HelloTypeRdbSave,
        .aof_rewrite = HelloTypeAofRewrite,
        .mem_usage = HelloTypeMemUsage,
        .free = HelloTypeFree,
        .digest = HelloTypeDigest
    };

We'll just add:

.rdb_aux_save
.rdb_aux_load

  1. Another thing is related to the when argument. In the original specification we said we want to call the save callback at two different times: before and after. An open question was, does it make sense to have also call the callback every time a key of the associated data is stored, and have another value for the when argument? Well, it looks completely useless: as you serialize your value, anyway you can store everything you want. And this brings us to "4".

  2. Given that each module type will be called just two times, and will serialized everything it want, we'll have, at maximum, just two blobs of data before/after, not a blob for each key or alike. What this means is that it is totally cheap (1 or 2 bytes more in the RDB file!) to do the following: we also store, transparently, the "when" value the aux_save callback was called with, as a first byte. In this way, in the aux_load callback, we can also have the "when" argument as well, that will be set to BEFORE or AFTER. This way a module may decide, for instance, to store data both before and after the RDB file, and the load callback will receive such info every time it is called, which can be useful.

Comment From: oranagra

Module AUX implemented in #6264

Comment From: antirez

Folks, I'm resurrecting this issue to have a final design for the hooks mechanism.

That's what I think we should do, trying to summing all the contributions obtained above.

  1. Add a void* pointer to the callback. This will be able to hold anything if casted.
  2. Use the versioning trick in the structure, but expose the structure directly to the module itself: to have accessors for every field is IMHO too much verbose.

However there is a problem with this approach and ABI compatibility. Take as example the event CLIENT_DISCONNECTION and CLIENT_CONNECTION: those will return a clientInfo structure (check the draft in 8eed6c28, but this commit will go away so I'm pasting it here):

#define REDISMODULE_CLIENTINFO_VERSION 1
typedef struct RedisModuleClientInfo {
    uint64_t version;       /* Version of this structure for ABI compat. */
    uint64_t flags;         /* REDISMODULE_CLIENTINFO_FLAG_* */
    char addr[46];          /* IPv4 or IPv6 address. */
    uint16_t port;          /* TCP port. */
    uint16_t db;            /* Selected DB. */
} RedisModuleClientInfo;

The 'version' thing is great when we pass it to some Redis Module API to get it populated:

      RedisModuleClientInfo ci = REDISMODULE_CLIENTINFO_INITIALIZER;
      int retval = RedisModule_GetClientInfoById(ctx,&ci);

The first field is populated with the version of the structure, so the API is able to fill the structure having the layout that the compiled module (maybe with an older version of the header has in memory.

However with the callbacks, things are different. The callback will return a void pointer to the module, but it does not know the version of the structures.

I suggest fixing this problem like that:

The call RedisModule_SubscribeToServerEvents() may actually be function that takes two arguments: the first is a RedisModuleServerEventType, which is just a structure having two fixed fields we'll never change: event-type, event-structure-version. The second is the callback (use NULL to unregister the event). So you do something like that:

RedisModule_SubscribeToServerEvents(ctx, RedisModuleServerEvent_ClientConnection, myCallback);

In redismodule.h, RedisModuleServerEvent_ClientConnection is defined like that:

struct RedisModuleServerEvent {
    uint64_t event_flag;
    uint64_t event_structure_version;
};

struct RedisModuleServerEvent RedisModuleServerEvent_ClientConnection = {.event_flag = REDISMDOULE_SERVER_EVENT_CLIENT_CONNECT, .event_structure_version = REDISMODULE_CLIENTINFO_VERSION};

This way we'll know how to populate the structure before calling the callback, in a way that the module, when casting back to its version of the structure, will receive the correct data. What do you think?

Comment From: yossigo

This discussion has already been concluded committed, closing.