From 35b3fbd90c2ad2c503c9e3d28bfbffff13099925 Mon Sep 17 00:00:00 2001 From: Binbin Date: Sun, 9 Oct 2022 13:18:34 +0800 Subject: Freeze time sampling during command execution, and scripts (#10300) Freeze time during execution of scripts and all other commands. This means that a key is either expired or not, and doesn't change state during a script execution. resolves #10182 This PR try to add a new `commandTimeSnapshot` function. The function logic is extracted from `keyIsExpired`, but the related calls to `fixed_time_expire` and `mstime()` are removed, see below. In commands, we will avoid calling `mstime()` multiple times and just use the one that sampled in call. The background is, e.g. using `PEXPIRE 1` with valgrind sometimes result in the key being deleted rather than expired. The reason is that both `PEXPIRE` command and `checkAlreadyExpired` call `mstime()` separately. There are other more important changes in this PR: 1. Eliminate `fixed_time_expire`, it is no longer needed. When we want to sample time we should always use a time snapshot. We will use `in_nested_call` instead to update the cached time in `call`. 2. Move the call for `updateCachedTime` from `serverCron` to `afterSleep`. Now `commandTimeSnapshot` will always return the sample time, the `lookupKeyReadWithFlags` call in `getNodeByQuery` will get a outdated cached time (because `processCommand` is out of the `call` context). We put the call to `updateCachedTime` in `aftersleep`. 3. Cache the time each time the module lock Redis. Call `updateCachedTime` in `moduleGILAfterLock`, affecting `RM_ThreadSafeContextLock` and `RM_ThreadSafeContextTryLock` Currently the commandTimeSnapshot change affects the following TTL commands: - SET EX / SET PX - EXPIRE / PEXPIRE - SETEX / PSETEX - GETEX EX / GETEX PX - TTL / PTTL - EXPIRETIME / PEXPIRETIME - RESTORE key TTL And other commands just use the cached mstime (including TIME). This is considered to be a breaking change since it can break a script that uses a loop to wait for a key to expire. --- src/replication.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/replication.c') diff --git a/src/replication.c b/src/replication.c index f60fcfd00..ad05d175a 100644 --- a/src/replication.c +++ b/src/replication.c @@ -4006,7 +4006,7 @@ void failoverCommand(client *c) { serverLog(LL_NOTICE,"FAILOVER requested to any replica."); } - mstime_t now = mstime(); + mstime_t now = commandTimeSnapshot(); if (timeout_in_ms) { server.failover_end_time = now + timeout_in_ms; } -- cgit v1.2.1