summaryrefslogtreecommitdiff
path: root/src/module.c
diff options
context:
space:
mode:
authorMeir Shpilraien (Spielrein) <meir@redis.com>2022-10-18 19:45:46 +0300
committerGitHub <noreply@github.com>2022-10-18 19:45:46 +0300
commitb43f254813025e3deea6ef65126ea2bad49af857 (patch)
tree778745fc48cf338ddbf2ea97cc837984cc1ff59b /src/module.c
parent20d286f77eabccd657be66d3193556ea7ebe4d26 (diff)
downloadredis-b43f254813025e3deea6ef65126ea2bad49af857.tar.gz
Avoid saving module aux on RDB if no aux data was saved by the module. (#11374)
### Background The issue is that when saving an RDB with module AUX data, the module AUX metadata (moduleid, when, ...) is saved to the RDB even though the module did not saved any actual data. This prevent loading the RDB in the absence of the module (although there is no actual data in the RDB that requires the module to be loaded). ### Solution The solution suggested in this PR is that module AUX will be saved on the RDB only if the module actually saved something during `aux_save` function. To support backward compatibility, we introduce `aux_save2` callback that acts the same as `aux_save` with the tiny change of avoid saving the aux field if no data was actually saved by the module. Modules can use the new API to make sure that if they have no data to save, then it will be possible to load the created RDB even without the module. ### Concerns A module may register for the aux load and save hooks just in order to be notified when saving or loading starts or completed (there are better ways to do that, but it still possible that someone used it). However, if a module didn't save a single field in the save callback, it means it's not allowed to read in the read callback, since it has no way to distinguish between empty and non-empty payloads. furthermore, it means that if the module did that, it must never change it, since it'll break compatibility with it's old RDB files, so this is really not a valid use case. Since some modules (ones who currently save one field indicating an empty payload), need to know if saving an empty payload is valid, and if Redis is gonna ignore an empty payload or store it, we opted to add a new API (rather than change behavior of an existing API and expect modules to check the redis version) ### Technical Details To avoid saving AUX data on RDB, we change the code to first save the AUX metadata (moduleid, when, ...) into a temporary buffer. The buffer is then flushed to the rio at the first time the module makes a write operation inside the `aux_save` function. If the module saves nothing (and `aux_save2` was used), the entire temporary buffer is simply dropped and no data about this AUX field is saved to the RDB. This make it possible to load the RDB even in the absence of the module. Test was added to verify the fix.
Diffstat (limited to 'src/module.c')
-rw-r--r--src/module.c29
1 files changed, 28 insertions, 1 deletions
diff --git a/src/module.c b/src/module.c
index 11b47d99f..33ac151a2 100644
--- a/src/module.c
+++ b/src/module.c
@@ -6381,6 +6381,9 @@ robj *moduleTypeDupOrReply(client *c, robj *fromkey, robj *tokey, int todb, robj
* so that meta information such as key name and db id can be obtained.
* * **copy2**: Similar to `copy`, but provides the `RedisModuleKeyOptCtx` parameter
* so that meta information such as key names and db ids can be obtained.
+ * * **aux_save2**: Similar to `aux_save`, but with small semantic change, if the module
+ * saves nothing on this callback then no data about this aux field will be written to the
+ * RDB and it will be possible to load the RDB even if the module is not loaded.
*
* Note: the module name "AAAAAAAAA" is reserved and produces an error, it
* happens to be pretty lame as well.
@@ -6433,6 +6436,9 @@ moduleType *RM_CreateDataType(RedisModuleCtx *ctx, const char *name, int encver,
moduleTypeUnlinkFunc2 unlink2;
moduleTypeCopyFunc2 copy2;
} v4;
+ struct {
+ moduleTypeAuxSaveFunc aux_save2;
+ } v5;
} *tms = (struct typemethods*) typemethods_ptr;
moduleType *mt = zcalloc(sizeof(*mt));
@@ -6461,6 +6467,9 @@ moduleType *RM_CreateDataType(RedisModuleCtx *ctx, const char *name, int encver,
mt->free_effort2 = tms->v4.free_effort2;
mt->copy2 = tms->v4.copy2;
}
+ if (tms->version >= 5) {
+ mt->aux_save2 = tms->v5.aux_save2;
+ }
memcpy(mt->name,name,sizeof(mt->name));
listAddNodeTail(ctx->module->types,mt);
return mt;
@@ -6575,11 +6584,25 @@ int RM_IsIOError(RedisModuleIO *io) {
return io->error;
}
+static int flushRedisModuleIOBuffer(RedisModuleIO *io) {
+ if (!io->pre_flush_buffer) return 0;
+
+ /* We have data that must be flushed before saving the current data.
+ * Lets flush it. */
+ sds pre_flush_buffer = io->pre_flush_buffer;
+ io->pre_flush_buffer = NULL;
+ ssize_t retval = rdbWriteRaw(io->rio, pre_flush_buffer, sdslen(pre_flush_buffer));
+ sdsfree(pre_flush_buffer);
+ if (retval >= 0) io->bytes += retval;
+ return retval;
+}
+
/* Save an unsigned 64 bit value into the RDB file. This function should only
* be called in the context of the rdb_save method of modules implementing new
* data types. */
void RM_SaveUnsigned(RedisModuleIO *io, uint64_t value) {
if (io->error) return;
+ if (flushRedisModuleIOBuffer(io) == -1) goto saveerr;
/* Save opcode. */
int retval = rdbSaveLen(io->rio, RDB_MODULE_OPCODE_UINT);
if (retval == -1) goto saveerr;
@@ -6633,6 +6656,7 @@ int64_t RM_LoadSigned(RedisModuleIO *io) {
* the RDB file. */
void RM_SaveString(RedisModuleIO *io, RedisModuleString *s) {
if (io->error) return;
+ if (flushRedisModuleIOBuffer(io) == -1) goto saveerr;
/* Save opcode. */
ssize_t retval = rdbSaveLen(io->rio, RDB_MODULE_OPCODE_STRING);
if (retval == -1) goto saveerr;
@@ -6651,6 +6675,7 @@ saveerr:
* as input. */
void RM_SaveStringBuffer(RedisModuleIO *io, const char *str, size_t len) {
if (io->error) return;
+ if (flushRedisModuleIOBuffer(io) == -1) goto saveerr;
/* Save opcode. */
ssize_t retval = rdbSaveLen(io->rio, RDB_MODULE_OPCODE_STRING);
if (retval == -1) goto saveerr;
@@ -6709,6 +6734,7 @@ char *RM_LoadStringBuffer(RedisModuleIO *io, size_t *lenptr) {
* It is possible to load back the value with RedisModule_LoadDouble(). */
void RM_SaveDouble(RedisModuleIO *io, double value) {
if (io->error) return;
+ if (flushRedisModuleIOBuffer(io) == -1) goto saveerr;
/* Save opcode. */
int retval = rdbSaveLen(io->rio, RDB_MODULE_OPCODE_DOUBLE);
if (retval == -1) goto saveerr;
@@ -6744,6 +6770,7 @@ loaderr:
* It is possible to load back the value with RedisModule_LoadFloat(). */
void RM_SaveFloat(RedisModuleIO *io, float value) {
if (io->error) return;
+ if (flushRedisModuleIOBuffer(io) == -1) goto saveerr;
/* Save opcode. */
int retval = rdbSaveLen(io->rio, RDB_MODULE_OPCODE_FLOAT);
if (retval == -1) goto saveerr;
@@ -6814,7 +6841,7 @@ ssize_t rdbSaveModulesAux(rio *rdb, int when) {
listRewind(module->types,&li);
while((ln = listNext(&li))) {
moduleType *mt = ln->value;
- if (!mt->aux_save || !(mt->aux_save_triggers & when))
+ if ((!mt->aux_save && !mt->aux_save2) || !(mt->aux_save_triggers & when))
continue;
ssize_t ret = rdbSaveSingleModuleAux(rdb, when, mt);
if (ret==-1) {