diff --git a/00-RELEASENOTES b/00-RELEASENOTES index 3572bcc54..9d1670536 100644 --- a/00-RELEASENOTES +++ b/00-RELEASENOTES @@ -114,7 +114,10 @@ Redis 6.0 is mostly a strict superset of 5.0, you should not have any problem upgrading your application from 5.0 to 6.0. However this is a list of small non-backward compatible changes introduced in the 6.0 release: -* Nothing found yet. +* The SPOP command no longer returns null when the set key does not + exist. Now it returns the empty set as it should and as happens when it is + called with a 0 argument. This is technically a fix, however it changes the + old behavior. -------------------------------------------------------------------------------- diff --git a/src/ae.cpp b/src/ae.cpp index d6fd1bc76..9f0285e3f 100644 --- a/src/ae.cpp +++ b/src/ae.cpp @@ -403,6 +403,7 @@ extern "C" void aeDeleteEventLoop(aeEventLoop *eventLoop) { close(eventLoop->fdCmdRead); close(eventLoop->fdCmdWrite); + /* Free the time events list. */ auto *te = eventLoop->timeEventHead; while (te) { diff --git a/src/aof.cpp b/src/aof.cpp index acec221b4..1472a832e 100644 --- a/src/aof.cpp +++ b/src/aof.cpp @@ -1848,14 +1848,15 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) { serverLog(LL_VERBOSE, "Background AOF rewrite signal handler took %lldus", ustime()-now); } else if (!bysignal && exitcode != 0) { + g_pserver->aof_lastbgrewrite_status = C_ERR; + + serverLog(LL_WARNING, + "Background AOF rewrite terminated with error"); + } else { /* SIGUSR1 is whitelisted, so we have a way to kill a child without * tirggering an error condition. */ if (bysignal != SIGUSR1) g_pserver->aof_lastbgrewrite_status = C_ERR; - serverLog(LL_WARNING, - "Background AOF rewrite terminated with error"); - } else { - g_pserver->aof_lastbgrewrite_status = C_ERR; serverLog(LL_WARNING, "Background AOF rewrite terminated by signal %d", bysignal); diff --git a/src/config.cpp b/src/config.cpp index caa4be15b..3e86c4dbd 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -110,12 +110,12 @@ clientBufferLimitsConfig clientBufferLimitsDefaults[CLIENT_TYPE_OBUF_COUNT] = { /* Generic config infrastructure function pointers * int is_valid_fn(val, err) * Return 1 when val is valid, and 0 when invalid. - * Optionslly set err to a static error string. + * Optionally set err to a static error string. * int update_fn(val, prev, err) * This function is called only for CONFIG SET command (not at config file parsing) * It is called after the actual config is applied, * Return 1 for success, and 0 for failure. - * Optionslly set err to a static error string. + * Optionally set err to a static error string. * On failure the config change will be reverted. */ @@ -567,7 +567,8 @@ void loadServerConfigFromString(char *config) { return; loaderr: - fprintf(stderr, "\n*** FATAL CONFIG FILE ERROR ***\n"); + fprintf(stderr, "\n*** FATAL CONFIG FILE ERROR (Redis %s) ***\n", + KEYDB_REAL_VERSION); fprintf(stderr, "Reading the configuration file, at line %d\n", linenum); fprintf(stderr, ">>> '%s'\n", lines[i]); fprintf(stderr, "%s\n", err); @@ -777,7 +778,7 @@ void configSetCommand(client *c) { * config_set_memory_field(name,var) */ } config_set_memory_field( "client-query-buffer-limit",cserver.client_max_querybuf_len) { - /* Everyhing else is an error... */ + /* Everything else is an error... */ } config_set_else { addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s", (char*)ptrFromObj(c->argv[2])); @@ -1745,16 +1746,15 @@ static int enumConfigSet(typeData data, sds value, int update, const char **err) sds enumerr = sdsnew("argument must be one of the following: "); configEnum *enumNode = data.enumd.enum_value; while(enumNode->name != NULL) { - enumerr = sdscatlen(enumerr, enumNode->name, strlen(enumNode->name)); + enumerr = sdscatlen(enumerr, enumNode->name, + strlen(enumNode->name)); enumerr = sdscatlen(enumerr, ", ", 2); enumNode++; } + sdsrange(enumerr,0,-3); /* Remove final ", ". */ - enumerr[sdslen(enumerr) - 2] = '\0'; - - /* Make sure we don't overrun the fixed buffer */ - enumerr[LOADBUF_SIZE - 1] = '\0'; strncpy(loadbuf, enumerr, LOADBUF_SIZE); + loadbuf[LOADBUF_SIZE - 1] = '\0'; sdsfree(enumerr); *err = loadbuf; diff --git a/src/db.cpp b/src/db.cpp index 4b48f8df3..0e945b7a7 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1559,6 +1559,8 @@ int expireIfNeeded(redisDb *db, robj *key) { /* ----------------------------------------------------------------------------- * API to get key arguments from commands * ---------------------------------------------------------------------------*/ +#define MAX_KEYS_BUFFER 256 +thread_local static int getKeysTempBuffer[MAX_KEYS_BUFFER]; /* The base case is to use the keys position as given in the command table * (firstkey, lastkey, step). */ @@ -1573,7 +1575,12 @@ int *getKeysUsingCommandTable(struct redisCommand *cmd,robj **argv, int argc, in last = cmd->lastkey; if (last < 0) last = argc+last; - keys = (int*)zmalloc(sizeof(int)*((last - cmd->firstkey)+1), MALLOC_SHARED); + + int count = ((last - cmd->firstkey)+1); + keys = getKeysTempBuffer; + if (count > MAX_KEYS_BUFFER) + keys = (int*)zmalloc(sizeof(int)*count); + for (j = cmd->firstkey; j <= last; j += cmd->keystep) { if (j >= argc) { /* Modules commands, and standard commands with a not fixed number @@ -1583,7 +1590,7 @@ int *getKeysUsingCommandTable(struct redisCommand *cmd,robj **argv, int argc, in * return no keys and expect the command implementation to report * an arity or syntax error. */ if (cmd->flags & CMD_MODULE || cmd->arity < 0) { - zfree(keys); + getKeysFreeResult(keys); *numkeys = 0; return NULL; } else { @@ -1619,7 +1626,8 @@ int *getKeysFromCommand(struct redisCommand *cmd, robj **argv, int argc, int *nu /* Free the result of getKeysFromCommand. */ void getKeysFreeResult(int *result) { - zfree(result); + if (result != getKeysTempBuffer) + zfree(result); } /* Helper function to extract keys from following commands: @@ -1640,7 +1648,9 @@ int *zunionInterGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *nu /* Keys in z{union,inter}store come from two places: * argv[1] = storage key, * argv[3...n] = keys to intersect */ - keys = (int*)zmalloc(sizeof(int)*(num+1), MALLOC_SHARED); + keys = getKeysTempBuffer; + if (num+1>MAX_KEYS_BUFFER) + keys = (int*)zmalloc(sizeof(int)*(num+1)); /* Add all key positions for argv[3...n] to keys[] */ for (i = 0; i < num; i++) keys[i] = 3+i; @@ -1666,7 +1676,10 @@ int *evalGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys) return NULL; } - keys = (int*)zmalloc(sizeof(int)*num, MALLOC_SHARED); + keys = getKeysTempBuffer; + if (num>MAX_KEYS_BUFFER) + keys = (int*)zmalloc(sizeof(int)*num); + *numkeys = num; /* Add all key positions for argv[3...n] to keys[] */ @@ -1687,7 +1700,7 @@ int *sortGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys) UNUSED(cmd); num = 0; - keys = (int*)zmalloc(sizeof(int)*2, MALLOC_SHARED); /* Alloc 2 places for the worst case. */ + keys = getKeysTempBuffer; /* Alloc 2 places for the worst case. */ keys[num++] = 1; /* is always present. */ @@ -1745,7 +1758,10 @@ int *migrateGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkey } } - keys = (int*)zmalloc(sizeof(int)*num, MALLOC_SHARED); + keys = getKeysTempBuffer; + if (num>MAX_KEYS_BUFFER) + keys = (int*)zmalloc(sizeof(int)*num); + for (i = 0; i < num; i++) keys[i] = first+i; *numkeys = num; return keys; @@ -1778,7 +1794,9 @@ int *georadiusGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numk * argv[1] = key, * argv[5...n] = stored key if present */ - keys = (int*)zmalloc(sizeof(int) * num, MALLOC_SHARED); + keys = getKeysTempBuffer; + if (num>MAX_KEYS_BUFFER) + keys = (int*)zmalloc(sizeof(int) * num); /* Add all key positions to keys[] */ keys[0] = 1; @@ -1796,7 +1814,7 @@ int *memoryGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys UNUSED(cmd); if (argc >= 3 && !strcasecmp(szFromObj(argv[1]),"usage")) { - keys = (int*)zmalloc(sizeof(int) * 1); + keys = getKeysTempBuffer; keys[0] = 2; *numkeys = 1; return keys; @@ -1843,7 +1861,10 @@ int *xreadGetKeys(struct redisCommand *cmd, robj **argv, int argc, int *numkeys) num /= 2; /* We have half the keys as there are arguments because there are also the IDs, one per key. */ - keys = (int*)zmalloc(sizeof(int) * num, MALLOC_SHARED); + keys = getKeysTempBuffer; + if (num>MAX_KEYS_BUFFER) + keys = (int*)zmalloc(sizeof(int) * num); + for (i = streams_pos+1; i < argc-num; i++) keys[i-streams_pos-1] = i; *numkeys = num; return keys; diff --git a/src/debug.cpp b/src/debug.cpp index 5ffa8df47..9ef78e387 100644 --- a/src/debug.cpp +++ b/src/debug.cpp @@ -694,9 +694,12 @@ NULL sds stats = sdsempty(); char buf[4096]; - if (getLongFromObjectOrReply(c, c->argv[2], &dbid, NULL) != C_OK) + if (getLongFromObjectOrReply(c, c->argv[2], &dbid, NULL) != C_OK) { + sdsfree(stats); return; + } if (dbid < 0 || dbid >= cserver.dbnum) { + sdsfree(stats); addReplyError(c,"Out of range database"); return; } diff --git a/src/dict.cpp b/src/dict.cpp index 2cfc25334..4e58d0064 100644 --- a/src/dict.cpp +++ b/src/dict.cpp @@ -871,6 +871,10 @@ unsigned long dictScan(dict *d, if (dictSize(d) == 0) return 0; + /* Having a safe iterator means no rehashing can happen, see _dictRehashStep. + * This is needed in case the scan callback tries to do dictFind or alike. */ + d->iterators++; + if (!dictIsRehashing(d)) { t0 = &(d->ht[0]); m0 = t0->sizemask; @@ -937,6 +941,9 @@ unsigned long dictScan(dict *d, } while (v & (m0 ^ m1)); } + /* undo the ++ at the top */ + d->iterators--; + return v; } diff --git a/src/module.cpp b/src/module.cpp index a27bf95ff..e218afbd5 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -1056,6 +1056,17 @@ RedisModuleString *RM_CreateStringFromLongLong(RedisModuleCtx *ctx, long long ll return RM_CreateString(ctx,buf,len); } +/* Like RedisModule_CreatString(), but creates a string starting from a double + * integer instead of taking a buffer and its length. + * + * The returned string must be released with RedisModule_FreeString() or by + * enabling automatic memory management. */ +RedisModuleString *RM_CreateStringFromDouble(RedisModuleCtx *ctx, double d) { + char buf[128]; + size_t len = d2string(buf,sizeof(buf),d); + return RM_CreateString(ctx,buf,len); +} + /* Like RedisModule_CreatString(), but creates a string starting from a long * double. * @@ -3605,6 +3616,8 @@ void moduleTypeNameByID(char *name, uint64_t moduleid) { * // Optional fields * .digest = myType_DigestCallBack, * .mem_usage = myType_MemUsageCallBack, + * .aux_load = myType_AuxRDBLoadCallBack, + * .aux_save = myType_AuxRDBSaveCallBack, * } * * * **rdb_load**: A callback function pointer that loads data from RDB files. @@ -3612,6 +3625,10 @@ void moduleTypeNameByID(char *name, uint64_t moduleid) { * * **aof_rewrite**: A callback function pointer that rewrites data as commands. * * **digest**: A callback function pointer that is used for `DEBUG DIGEST`. * * **free**: A callback function pointer that can free a type value. + * * **aux_save**: A callback function pointer that saves out of keyspace data to RDB files. + * 'when' argument is either REDISMODULE_AUX_BEFORE_RDB or REDISMODULE_AUX_AFTER_RDB. + * * **aux_load**: A callback function pointer that loads out of keyspace data from RDB files. + * Similar to aux_save, returns REDISMODULE_OK on success, and ERR otherwise. * * The **digest* and **mem_usage** methods should currently be omitted since * they are not yet implemented inside the Redis modules core. @@ -6641,24 +6658,32 @@ void RM_ScanCursorDestroy(RedisModuleScanCursor *cursor) { zfree(cursor); } -/* Scan api that allows a module to scan all the keys and value in the selected db. +/* Scan API that allows a module to scan all the keys and value in + * the selected db. * * Callback for scan implementation. - * void scan_callback(RedisModuleCtx *ctx, RedisModuleString *keyname, RedisModuleKey *key, void *privdata); - * - ctx - the redis module context provided to for the scan. - * - keyname - owned by the caller and need to be retained if used after this function. - * - key - holds info on the key and value, it is provided as best effort, in some cases it might - * be NULL, in which case the user should (can) use RedisModule_OpenKey (and CloseKey too). - * when it is provided, it is owned by the caller and will be free when the callback returns. - * - privdata - the user data provided to RedisModule_Scan. + * void scan_callback(RedisModuleCtx *ctx, RedisModuleString *keyname, + * RedisModuleKey *key, void *privdata); + * ctx - the redis module context provided to for the scan. + * keyname - owned by the caller and need to be retained if used after this + * function. + * + * key - holds info on the key and value, it is provided as best effort, in + * some cases it might be NULL, in which case the user should (can) use + * RedisModule_OpenKey (and CloseKey too). + * when it is provided, it is owned by the caller and will be free when the + * callback returns. + * + * privdata - the user data provided to RedisModule_Scan. * * The way it should be used: * RedisModuleCursor *c = RedisModule_ScanCursorCreate(); * while(RedisModule_Scan(ctx, c, callback, privateData)); * RedisModule_ScanCursorDestroy(c); * - * It is also possible to use this API from another thread while the lock is acquired durring - * the actuall call to RM_Scan: + * It is also possible to use this API from another thread while the lock + * is acquired durring the actuall call to RM_Scan: + * * RedisModuleCursor *c = RedisModule_ScanCursorCreate(); * RedisModule_ThreadSafeContextLock(ctx); * while(RedisModule_Scan(ctx, c, callback, privateData)){ @@ -6668,9 +6693,26 @@ void RM_ScanCursorDestroy(RedisModuleScanCursor *cursor) { * } * RedisModule_ScanCursorDestroy(c); * - * The function will return 1 if there are more elements to scan and 0 otherwise, - * possibly setting errno if the call failed. - * It is also possible to restart and existing cursor using RM_CursorRestart. */ + * The function will return 1 if there are more elements to scan and + * 0 otherwise, possibly setting errno if the call failed. + * + * It is also possible to restart and existing cursor using RM_CursorRestart. + * + * IMPORTANT: This API is very similar to the Redis SCAN command from the + * point of view of the guarantees it provides. This means that the API + * may report duplicated keys, but guarantees to report at least one time + * every key that was there from the start to the end of the scanning process. + * + * NOTE: If you do database changes within the callback, you should be aware + * that the internal state of the database may change. For instance it is safe + * to delete or modify the current key, but may not be safe to delete any + * other key. + * Moreover playing with the Redis keyspace while iterating may have the + * effect of returning more duplicates. A safe pattern is to store the keys + * names you want to modify elsewhere, and perform the actions on the keys + * later when the iteration is complete. Howerver this can cost a lot of + * memory, so it may make sense to just operate on the current key when + * possible during the iteration, given that this is safe. */ int RM_Scan(RedisModuleCtx *ctx, RedisModuleScanCursor *cursor, RedisModuleScanCB fn, void *privdata) { if (cursor->done) { errno = ENOENT; @@ -6748,9 +6790,17 @@ static void moduleScanKeyCallback(void *privdata, const dictEntry *de) { * RedisModule_CloseKey(key); * RedisModule_ScanCursorDestroy(c); * - * The function will return 1 if there are more elements to scan and 0 otherwise, - * possibly setting errno if the call failed. - * It is also possible to restart and existing cursor using RM_CursorRestart. */ + * The function will return 1 if there are more elements to scan and 0 otherwise, + * possibly setting errno if the call failed. + * It is also possible to restart and existing cursor using RM_CursorRestart. + * + * NOTE: Certain operations are unsafe while iterating the object. For instance + * while the API guarantees to return at least one time all the elements that + * are present in the data structure consistently from the start to the end + * of the iteration (see HSCAN and similar commands documentation), the more + * you play with the elements, the more duplicates you may get. In general + * deleting the current element of the data structure is safe, while removing + * the key you are iterating is not safe. */ int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleScanKeyCB fn, void *privdata) { if (key == NULL || key->value == NULL) { errno = EINVAL; @@ -6856,7 +6906,7 @@ int RM_Fork(RedisModuleForkDoneHandler cb, void *user_data) { g_pserver->module_child_pid = childpid; moduleForkInfo.done_handler = cb; moduleForkInfo.done_handler_user_data = user_data; - serverLog(LL_NOTICE, "Module fork started pid: %d ", childpid); + serverLog(LL_VERBOSE, "Module fork started pid: %d ", childpid); } return childpid; } @@ -6879,7 +6929,7 @@ int TerminateModuleForkChild(int child_pid, int wait) { g_pserver->module_child_pid != child_pid) return C_ERR; int statloc; - serverLog(LL_NOTICE,"Killing running module fork child: %ld", + serverLog(LL_VERBOSE,"Killing running module fork child: %ld", (long) g_pserver->module_child_pid); if (kill(g_pserver->module_child_pid,SIGUSR1) != -1 && wait) { while(wait4(g_pserver->module_child_pid,&statloc,0,NULL) != @@ -7806,6 +7856,7 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(CreateStringFromCallReply); REGISTER_API(CreateString); REGISTER_API(CreateStringFromLongLong); + REGISTER_API(CreateStringFromDouble); REGISTER_API(CreateStringFromLongDouble); REGISTER_API(CreateStringFromString); REGISTER_API(CreateStringPrintf); diff --git a/src/rdb.cpp b/src/rdb.cpp index cac5a1f08..9a3cdf103 100644 --- a/src/rdb.cpp +++ b/src/rdb.cpp @@ -2340,7 +2340,7 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { io.ver = 2; /* Call the rdb_load method of the module providing the 10 bit * encoding version in the lower 10 bits of the module ID. */ - if (mt->aux_load(&io,moduleid&1023, when) || io.error) { + if (mt->aux_load(&io,moduleid&1023, when) != REDISMODULE_OK || io.error) { moduleTypeNameByID(name,moduleid); serverLog(LL_WARNING,"The RDB file contains module AUX data for the module type '%s', that the responsible module is not able to load. Check for modules log above for additional clues.", name); exit(1); diff --git a/src/redismodule.h b/src/redismodule.h index f73bb772a..77b18b456 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -471,6 +471,7 @@ size_t REDISMODULE_API_FUNC(RedisModule_CallReplyLength)(RedisModuleCallReply *r RedisModuleCallReply *REDISMODULE_API_FUNC(RedisModule_CallReplyArrayElement)(RedisModuleCallReply *reply, size_t idx); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateString)(RedisModuleCtx *ctx, const char *ptr, size_t len); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromLongLong)(RedisModuleCtx *ctx, long long ll); +RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromDouble)(RedisModuleCtx *ctx, double d); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromLongDouble)(RedisModuleCtx *ctx, long double ld, int humanfriendly); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringFromString)(RedisModuleCtx *ctx, const RedisModuleString *str); RedisModuleString *REDISMODULE_API_FUNC(RedisModule_CreateStringPrintf)(RedisModuleCtx *ctx, const char *fmt, ...); @@ -730,6 +731,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(CreateStringFromCallReply); REDISMODULE_GET_API(CreateString); REDISMODULE_GET_API(CreateStringFromLongLong); + REDISMODULE_GET_API(CreateStringFromDouble); REDISMODULE_GET_API(CreateStringFromLongDouble); REDISMODULE_GET_API(CreateStringFromString); REDISMODULE_GET_API(CreateStringPrintf); diff --git a/src/replication.cpp b/src/replication.cpp index 2d38ab006..564d46fa1 100644 --- a/src/replication.cpp +++ b/src/replication.cpp @@ -3767,7 +3767,6 @@ void replicationCron(void) { propagateMasterStaleKeys(); - /* Remove the RDB file used for replication if Redis is not running * with any persistence. */ removeRDBUsedToSyncReplicas(); diff --git a/src/server.cpp b/src/server.cpp index d4426bbe4..217b15851 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -4020,6 +4020,7 @@ void addReplyCommand(client *c, struct redisCommand *cmd) { flagcount += addReplyCommandFlag(c,cmd,CMD_SKIP_SLOWLOG, "skip_slowlog"); flagcount += addReplyCommandFlag(c,cmd,CMD_ASKING, "asking"); flagcount += addReplyCommandFlag(c,cmd,CMD_FAST, "fast"); + flagcount += addReplyCommandFlag(c,cmd,CMD_NO_AUTH, "no_auth"); if ((cmd->getkeys_proc && !(cmd->flags & CMD_MODULE)) || cmd->flags & CMD_MODULE_GETKEYS) { diff --git a/src/t_set.cpp b/src/t_set.cpp index 0b7323c80..aab5d9c51 100644 --- a/src/t_set.cpp +++ b/src/t_set.cpp @@ -420,7 +420,7 @@ void spopWithCountCommand(client *c) { /* Make sure a key with the name inputted exists, and that it's type is * indeed a set. Otherwise, return nil */ - if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.null[c->resp])) + if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.emptyset[c->resp])) == NULL || checkType(c,set,OBJ_SET)) return; /* If count is zero, serve an empty set ASAP to avoid special diff --git a/src/t_stream.cpp b/src/t_stream.cpp index 51b5f55cf..4026a6d0f 100644 --- a/src/t_stream.cpp +++ b/src/t_stream.cpp @@ -1073,9 +1073,7 @@ size_t streamReplyWithRangeFromConsumerPEL(client *c, stream *s, streamID *start * by the user by other means. In that case we signal it emitting * the ID but then a NULL entry for the fields. */ addReplyArrayLen(c,2); - streamID id; - streamDecodeID(ri.key,&id); - addReplyStreamID(c,&id); + addReplyStreamID(c,&thisid); addReplyNullArray(c); } else { streamNACK *nack = (streamNACK*)ri.data;