summaryrefslogtreecommitdiff
path: root/src/aof.c
diff options
context:
space:
mode:
authorguybe7 <guy.benoish@redislabs.com>2021-12-22 23:03:48 +0100
committerGitHub <noreply@github.com>2021-12-23 00:03:48 +0200
commit7ac213079cd9afc49f87f08362155aa447c95c4f (patch)
treef350c11e894601d61362b459a9864fed9290a1ac /src/aof.c
parentb7567394e1802906904fc380f95c05367e1f0c3f (diff)
downloadredis-7ac213079cd9afc49f87f08362155aa447c95c4f.tar.gz
Sort out mess around propagation and MULTI/EXEC (#9890)
The mess: Some parts use alsoPropagate for late propagation, others using an immediate one (propagate()), causing edge cases, ugly/hacky code, and the tendency for bugs The basic idea is that all commands are propagated via alsoPropagate (i.e. added to a list) and the top-most call() is responsible for going over that list and actually propagating them (and wrapping them in MULTI/EXEC if there's more than one command). This is done in the new function, propagatePendingCommands. Callers to propagatePendingCommands: 1. top-most call() (we want all nested call()s to add to the also_propagate array and just the top-most one to propagate them) - via `afterCommand` 2. handleClientsBlockedOnKeys: it is out of call() context and it may propagate stuff - via `afterCommand`. 3. handleClientsBlockedOnKeys edge case: if the looked-up key is already expired, we will propagate the expire but will not unblock any client so `afterCommand` isn't called. in that case, we have to propagate the deletion explicitly. 4. cron stuff: active-expire and eviction may also propagate stuff 5. modules: the module API allows to propagate stuff from just about anywhere (timers, keyspace notifications, threads). I could have tried to catch all the out-of-call-context places but it seemed easier to handle it in one place: when we free the context. in the spirit of what was done in call(), only the top-most freeing of a module context may cause propagation. 6. modules: when using a thread-safe ctx it's not clear when/if the ctx will be freed. we do know that the module must lock the GIL before calling RM_Replicate/RM_Call so we propagate the pending commands when releasing the GIL. A "known limitation", which were actually a bug, was fixed because of this commit (see propagate.tcl): When using a mix of RM_Call with `!` and RM_Replicate, the command would propagate out-of-order: first all the commands from RM_Call, and then the ones from RM_Replicate Another thing worth mentioning is that if, in the past, a client would issue a MULTI/EXEC with just one write command the server would blindly propagate the MULTI/EXEC too, even though it's redundant. not anymore. This commit renames propagate() to propagateNow() in order to cause conflicts in pending PRs. propagatePendingCommands is the only caller of propagateNow, which is now a static, internal helper function. Optimizations: 1. alsoPropagate will not add stuff to also_propagate if there's no AOF and replicas 2. alsoPropagate reallocs also_propagagte exponentially, to save calls to memmove Bugfixes: 1. CONFIG SET can create evictions, sending notifications which can cause to dirty++ with modules. we need to prevent it from propagating to AOF/replicas 2. We need to set current_client in RM_Call. buggy scenario: - CONFIG SET maxmemory, eviction notifications, module hook calls RM_Call - assertion in lookupKey crashes, because current_client has CONFIG SET, which isn't CMD_WRITE 3. minor: in eviction, call propagateDeletion after notification, like active-expire and all commands (we always send a notification before propagating the command)
Diffstat (limited to 'src/aof.c')
-rw-r--r--src/aof.c2
1 files changed, 2 insertions, 0 deletions
diff --git a/src/aof.c b/src/aof.c
index beaecefca..1647dd009 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -628,6 +628,8 @@ sds genAofTimestampAnnotationIfNeeded(int force) {
void feedAppendOnlyFile(int dictid, robj **argv, int argc) {
sds buf = sdsempty();
+ serverAssert(dictid >= 0 && dictid < server.dbnum);
+
/* Feed timestamp if needed */
if (server.aof_timestamp_enabled) {
sds ts = genAofTimestampAnnotationIfNeeded(0);