diff --git a/src/db.c b/src/db.c index 2fb4ced70..49c374fb1 100644 --- a/src/db.c +++ b/src/db.c @@ -375,23 +375,22 @@ robj *dbRandomKey(redisDb *db) { key = dictGetKey(de); keyobj = createStringObject(key,sdslen(key)); - if (dbFindExpires(db, key)) { - if (allvolatile && server.masterhost && --maxtries == 0) { - /* If the DB is composed only of keys with an expire set, - * it could happen that all the keys are already logically - * expired in the slave, so the function cannot stop because - * expireIfNeeded() is false, nor it can stop because - * dictGetFairRandomKey() returns NULL (there are keys to return). - * To prevent the infinite loop we do some tries, but if there - * are the conditions for an infinite loop, eventually we - * return a key name that may be already expired. */ - return keyobj; - } - if (expireIfNeeded(db,keyobj,0) != KEY_VALID) { - decrRefCount(keyobj); - continue; /* search for another key. This expired. */ - } + if (allvolatile && server.masterhost && --maxtries == 0) { + /* If the DB is composed only of keys with an expire set, + * it could happen that all the keys are already logically + * expired in the slave, so the function cannot stop because + * expireIfNeeded() is false, nor it can stop because + * dictGetFairRandomKey() returns NULL (there are keys to return). + * To prevent the infinite loop we do some tries, but if there + * are the conditions for an infinite loop, eventually we + * return a key name that may be already expired. */ + return keyobj; } + if (expireIfNeeded(db,keyobj,0) != KEY_VALID) { + decrRefCount(keyobj); + continue; /* search for another key. This expired. */ + } + return keyobj; } } @@ -1985,17 +1984,72 @@ long long getExpire(redisDb *db, robj *key) { return dictGetSignedIntegerVal(de); } -/* Delete the specified expired key and propagate expire. */ -void deleteExpiredKeyAndPropagate(redisDb *db, robj *keyobj) { - mstime_t expire_latency; - latencyStartMonitor(expire_latency); - dbGenericDelete(db,keyobj,server.lazyfree_lazy_expire,DB_FLAG_KEY_EXPIRED); - latencyEndMonitor(expire_latency); - latencyAddSampleIfNeeded("expire-del",expire_latency); - notifyKeyspaceEvent(NOTIFY_EXPIRED,"expired",keyobj,db->id); + +/* Delete the specified expired or evicted key and propagate to replicas. + * Currently notify_type can only be NOTIFY_EXPIRED or NOTIFY_EVICTED, + * and it affects other aspects like the latency monitor event name and, + * which config to look for lazy free, stats var to increment, and so on. + * + * key_mem_freed is an out parameter which contains the estimated + * amount of memory freed due to the trimming (may be NULL) */ +static void deleteKeyAndPropagate(redisDb *db, robj *keyobj, int notify_type, long long *key_mem_freed) { + mstime_t latency; + int del_flag = notify_type == NOTIFY_EXPIRED ? DB_FLAG_KEY_EXPIRED : DB_FLAG_KEY_EVICTED; + int lazy_flag = notify_type == NOTIFY_EXPIRED ? server.lazyfree_lazy_expire : server.lazyfree_lazy_eviction; + char *latency_name = notify_type == NOTIFY_EXPIRED ? "expire-del" : "evict-del"; + char *notify_name = notify_type == NOTIFY_EXPIRED ? "expired" : "evicted"; + + /* The key needs to be converted from static to heap before deleted */ + int static_key = keyobj->refcount == OBJ_STATIC_REFCOUNT; + if (static_key) { + keyobj = createStringObject(keyobj->ptr, sdslen(keyobj->ptr)); + } + + serverLog(LL_DEBUG,"key %s %s: deleting it", (char*)keyobj->ptr, notify_type == NOTIFY_EXPIRED ? "expired" : "evicted"); + + /* We compute the amount of memory freed by db*Delete() alone. + * It is possible that actually the memory needed to propagate + * the DEL in AOF and replication link is greater than the one + * we are freeing removing the key, but we can't account for + * that otherwise we would never exit the loop. + * + * Same for CSC invalidation messages generated by signalModifiedKey. + * + * AOF and Output buffer memory will be freed eventually so + * we only care about memory used by the key space. + * + * The code here used to first propagate and then record delta + * using only zmalloc_used_memory but in CRDT we can't do that + * so we use freeMemoryGetNotCountedMemory to avoid counting + * AOF and slave buffers */ + if (key_mem_freed) *key_mem_freed = (long long) zmalloc_used_memory() - freeMemoryGetNotCountedMemory(); + latencyStartMonitor(latency); + dbGenericDelete(db, keyobj, lazy_flag, del_flag); + latencyEndMonitor(latency); + latencyAddSampleIfNeeded(latency_name, latency); + if (key_mem_freed) *key_mem_freed -= (long long) zmalloc_used_memory() - freeMemoryGetNotCountedMemory(); + + notifyKeyspaceEvent(notify_type, notify_name,keyobj, db->id); signalModifiedKey(NULL, db, keyobj); - propagateDeletion(db,keyobj,server.lazyfree_lazy_expire); - server.stat_expiredkeys++; + propagateDeletion(db, keyobj, lazy_flag); + + if (notify_type == NOTIFY_EXPIRED) + server.stat_expiredkeys++; + else + server.stat_evictedkeys++; + + if (static_key) + decrRefCount(keyobj); +} + +/* Delete the specified expired key and propagate. */ +void deleteExpiredKeyAndPropagate(redisDb *db, robj *keyobj) { + deleteKeyAndPropagate(db, keyobj, NOTIFY_EXPIRED, NULL); +} + +/* Delete the specified evicted key and propagate. */ +void deleteEvictedKeyAndPropagate(redisDb *db, robj *keyobj, long long *key_mem_freed) { + deleteKeyAndPropagate(db, keyobj, NOTIFY_EVICTED, key_mem_freed); } /* Propagate an implicit key deletion into replicas and the AOF file. @@ -2085,9 +2139,9 @@ int keyIsExpired(redisDb *db, robj *key) { * The function returns KEY_EXPIRED if the key is expired BUT not deleted, * or returns KEY_DELETED if the key is expired and deleted. */ keyStatus expireIfNeeded(redisDb *db, robj *key, int flags) { - if ((!keyIsExpired(db,key)) || - (server.lazy_expire_disabled) || - (flags & EXPIRE_ALLOW_ACCESS_EXPIRED)) + if ((server.allow_access_expired) || + (flags & EXPIRE_ALLOW_ACCESS_EXPIRED) || + (!keyIsExpired(db,key))) return KEY_VALID; /* If we are running in the context of a replica, instead of @@ -2118,16 +2172,9 @@ keyStatus expireIfNeeded(redisDb *db, robj *key, int flags) { * will have failed over and the new primary will send us the expire. */ if (isPausedActionsWithUpdate(PAUSE_ACTION_EXPIRE)) return KEY_EXPIRED; - /* The key needs to be converted from static to heap before deleted */ - int static_key = key->refcount == OBJ_STATIC_REFCOUNT; - if (static_key) { - key = createStringObject(key->ptr, sdslen(key->ptr)); - } /* Delete the key */ deleteExpiredKeyAndPropagate(db,key); - if (static_key) { - decrRefCount(key); - } + return KEY_DELETED; } diff --git a/src/evict.c b/src/evict.c index 890a845d5..059e82fe3 100644 --- a/src/evict.c +++ b/src/evict.c @@ -525,8 +525,7 @@ int performEvictions(void) { int keys_freed = 0; size_t mem_reported, mem_tofree; long long mem_freed; /* May be negative */ - mstime_t latency, eviction_latency; - long long delta; + mstime_t latency; int slaves = listLength(server.slaves); int result = EVICT_FAIL; @@ -659,34 +658,18 @@ int performEvictions(void) { /* Finally remove the selected key. */ if (bestkey) { + long long key_mem_freed; db = server.db+bestdbid; - robj *keyobj = createStringObject(bestkey,sdslen(bestkey)); - /* We compute the amount of memory freed by db*Delete() alone. - * It is possible that actually the memory needed to propagate - * the DEL in AOF and replication link is greater than the one - * we are freeing removing the key, but we can't account for - * that otherwise we would never exit the loop. - * - * Same for CSC invalidation messages generated by signalModifiedKey. - * - * AOF and Output buffer memory will be freed eventually so - * we only care about memory used by the key space. */ + enterExecutionUnit(1, 0); - delta = (long long) zmalloc_used_memory(); - latencyStartMonitor(eviction_latency); - dbGenericDelete(db,keyobj,server.lazyfree_lazy_eviction,DB_FLAG_KEY_EVICTED); - latencyEndMonitor(eviction_latency); - latencyAddSampleIfNeeded("eviction-del",eviction_latency); - delta -= (long long) zmalloc_used_memory(); - mem_freed += delta; - server.stat_evictedkeys++; - signalModifiedKey(NULL,db,keyobj); - notifyKeyspaceEvent(NOTIFY_EVICTED, "evicted", - keyobj, db->id); - propagateDeletion(db,keyobj,server.lazyfree_lazy_eviction); - exitExecutionUnit(); - postExecutionUnitOperations(); + robj *keyobj = createStringObject(bestkey,sdslen(bestkey)); + deleteEvictedKeyAndPropagate(db, keyobj, &key_mem_freed); decrRefCount(keyobj); + exitExecutionUnit(); + /* Propagate the DEL command */ + postExecutionUnitOperations(); + + mem_freed += key_mem_freed; keys_freed++; if (keys_freed % 16 == 0) { diff --git a/src/expire.c b/src/expire.c index 1a49f5384..dee55f178 100644 --- a/src/expire.c +++ b/src/expire.c @@ -36,17 +36,18 @@ static double avg_ttl_factor[16] = {0.98, 0.9604, 0.941192, 0.922368, 0.903921, * to the function to avoid too many gettimeofday() syscalls. */ int activeExpireCycleTryExpire(redisDb *db, dictEntry *de, long long now) { long long t = dictGetSignedIntegerVal(de); - if (now > t) { - enterExecutionUnit(1, 0); - sds key = dictGetKey(de); - robj *keyobj = createStringObject(key,sdslen(key)); - deleteExpiredKeyAndPropagate(db,keyobj); - decrRefCount(keyobj); - exitExecutionUnit(); - return 1; - } else { + if (now < t) return 0; - } + + enterExecutionUnit(1, 0); + sds key = dictGetKey(de); + robj *keyobj = createStringObject(key,sdslen(key)); + deleteExpiredKeyAndPropagate(db,keyobj); + decrRefCount(keyobj); + exitExecutionUnit(); + /* Propagate the DEL command */ + postExecutionUnitOperations(); + return 1; } /* Try to expire a few timed out keys. The algorithm used is adaptive and @@ -113,8 +114,6 @@ void expireScanCallback(void *privdata, const dictEntry *const_de) { long long ttl = dictGetSignedIntegerVal(de) - data->now; if (activeExpireCycleTryExpire(data->db, de, data->now)) { data->expired++; - /* Propagate the DEL command */ - postExecutionUnitOperations(); } if (ttl > 0) { /* We want the average TTL of keys yet not expired. */ @@ -465,20 +464,13 @@ void expireSlaveKeys(void) { if ((dbids & 1) != 0) { redisDb *db = server.db+dbid; dictEntry *expire = dbFindExpires(db, keyname); - - if (expire && - activeExpireCycleTryExpire(server.db+dbid,expire,start)) - { - /* Propagate the DEL (writable replicas do not propagate anything to other replicas, - * but they might propagate to AOF) and trigger module hooks. */ - postExecutionUnitOperations(); - } + int expired = expire && activeExpireCycleTryExpire(server.db+dbid,expire,start); /* If the key was not expired in this DB, we need to set the * corresponding bit in the new bitmap we set as value. * At the end of the loop if the bitmap is zero, it means we * no longer need to keep track of this key. */ - else if (expire) { + if (expire && !expired) { noexpire++; new_dbids |= (uint64_t)1 << dbid; } diff --git a/src/module.c b/src/module.c index b686df13c..5238408c7 100644 --- a/src/module.c +++ b/src/module.c @@ -8849,9 +8849,9 @@ void moduleNotifyKeyspaceEvent(int type, const char *event, robj *key, int dbid) * it will not be notified about it. */ int prev_active = sub->active; sub->active = 1; - server.lazy_expire_disabled++; + server.allow_access_expired++; sub->notify_callback(&ctx, type, event, key); - server.lazy_expire_disabled--; + server.allow_access_expired--; sub->active = prev_active; moduleFreeContext(&ctx); } @@ -11890,7 +11890,7 @@ void processModuleLoadingProgressEvent(int is_aof) { /* When a key is deleted (in dbAsyncDelete/dbSyncDelete/setKey), it * will be called to tell the module which key is about to be released. */ void moduleNotifyKeyUnlink(robj *key, robj *val, int dbid, int flags) { - server.lazy_expire_disabled++; + server.allow_access_expired++; int subevent = REDISMODULE_SUBEVENT_KEY_DELETED; if (flags & DB_FLAG_KEY_EXPIRED) { subevent = REDISMODULE_SUBEVENT_KEY_EXPIRED; @@ -11913,7 +11913,7 @@ void moduleNotifyKeyUnlink(robj *key, robj *val, int dbid, int flags) { mt->unlink(key,mv->value); } } - server.lazy_expire_disabled--; + server.allow_access_expired--; } /* Return the free_effort of the module, it will automatically choose to call diff --git a/src/server.c b/src/server.c index f4248740f..72208c7e2 100644 --- a/src/server.c +++ b/src/server.c @@ -2074,7 +2074,7 @@ void initServerConfig(void) { server.bindaddr[j] = zstrdup(default_bindaddr[j]); memset(server.listeners, 0x00, sizeof(server.listeners)); server.active_expire_enabled = 1; - server.lazy_expire_disabled = 0; + server.allow_access_expired = 0; server.skip_checksum_validation = 0; server.loading = 0; server.async_loading = 0; diff --git a/src/server.h b/src/server.h index 33ce40e74..b650f2699 100644 --- a/src/server.h +++ b/src/server.h @@ -1744,7 +1744,7 @@ struct redisServer { int tcpkeepalive; /* Set SO_KEEPALIVE if non-zero. */ int active_expire_enabled; /* Can be disabled for testing purposes. */ int active_expire_effort; /* From 1 (default) to 10, active effort. */ - int lazy_expire_disabled; /* If > 0, don't trigger lazy expire */ + int allow_access_expired; /* If > 0, allow access to logically expired keys */ int active_defrag_enabled; int sanitize_dump_payload; /* Enables deep sanitization for ziplist and listpack in RDB and RESTORE. */ int skip_checksum_validation; /* Disable checksum validation for RDB and RESTORE payload. */ @@ -3365,6 +3365,7 @@ int setModuleNumericConfig(ModuleConfig *config, long long val, const char **err /* db.c -- Keyspace access API */ int removeExpire(redisDb *db, robj *key); void deleteExpiredKeyAndPropagate(redisDb *db, robj *keyobj); +void deleteEvictedKeyAndPropagate(redisDb *db, robj *keyobj, long long *key_mem_freed); void propagateDeletion(redisDb *db, robj *key, int lazy); int keyIsExpired(redisDb *db, robj *key); long long getExpire(redisDb *db, robj *key); diff --git a/src/t_hash.c b/src/t_hash.c index dc4ddf53f..1625513eb 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -747,7 +747,7 @@ GetFieldRes hashTypeGetValue(redisDb *db, robj *o, sds field, unsigned char **vs } if ((server.loading) || - (server.lazy_expire_disabled) || + (server.allow_access_expired) || (hfeFlags & HFE_LAZY_AVOID_FIELD_DEL) || (isPausedActionsWithUpdate(PAUSE_ACTION_EXPIRE))) return GETF_EXPIRED; @@ -1929,7 +1929,7 @@ static int hashTypeExpireIfNeeded(redisDb *db, robj *o) { /* Follow expireIfNeeded() conditions of when not lazy-expire */ if ( (server.loading) || - (server.lazy_expire_disabled) || + (server.allow_access_expired) || (server.masterhost) || /* master-client or user-client, don't delete */ (isPausedActionsWithUpdate(PAUSE_ACTION_EXPIRE))) return 0;