summaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAgeFilesLines
...
* Added fields to ACL LOG error entries for precise time logging (#11477)Roshan Khatri2023-02-023-2/+25
| | | | | | | | | | | | | Added 3 fields to the ACL LOG - adds entry_id, timestamp_created and timestamp_last_updated, which updates similar existing log error entries. The pair - entry_id, timestamp_created is a unique identifier of this entry, in case the node dies and is restarted, it can detect that if it's a new series. The primary use case of Unique id is to uniquely identify the error messages and not to detect if the server has restarted. entry-id is the sequence number of the entry (starting at 0) since the server process started. Can also be used to check if items were "lost" if they fell between periods. timestamp-created is the unix-time in ms at the time the entry was first created. timestamp-last-updated is the unix-time in ms at the time the entry was last updated Time_created gives the absolute time which better accounts for network time as compared to time since. It can also be older than 60 secs and presently there is no field that can display the original time of creation once the error entry is updated. The reason of timestamp_last_updated field is that it provides a more precise value for the “last time” an error was seen where as, presently it is only in the 60 second period. Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
* adding the ability to add streams to the pre-defined redis-benchmark tests ↵vanguard_space2023-02-021-0/+6
| | | | | (#11762) Added standard way to support xadd as one of the commands that can be run via redis-benchmarking tool
* Propagate message to a node only if the cluster link is healthy. (#11752)Harkrishn Patro2023-02-023-4/+36
| | | | | | | | Currently while a sharded pubsub message publish tries to propagate the message across the cluster, a NULL check is missing for clusterLink. clusterLink could be NULL if the link is causing memory beyond the set threshold cluster-link-sendbuf-limit and server terminates the link. This change introduces two things: Avoids the engine crashes on the publishing node if a message is tried to be sent to a node and the link is NULL. Adds a debugging tool CLUSTERLINK KILL to terminate the clusterLink between two nodes.
* Document some fields history of CLIENT LIST command (#11729)Binbin2023-02-012-2/+17
| | | | | | | | | | Change history: - `user` added in 6.0.0, 0f42447a0ec841f0b3e83328ac16a573012e2880 - `argv-mem` and `tot-mem` added in 6.2.0, bea40e6a41e31a52e9e6efee77ea5a4bd873b759 - `redir` added in 6.2.0, dd1f20edc5ecda7848c31601782c5e9d7bce4788 - `resp` added in 7.0.0, 7c376398b1cd827282e17804e230c41cbb48a89c - `multi-mem` added in 7.0.0, 2753429c99425e3d0216cba79e0e61192975f252 - `rbs` and `rbp` added in 7.0.0, 47c51d0c7858dc8ce7747b78b73cf8cec2e59ff3 - `ssub` added in 7.0.3, 35c2ee8716dc9b1d4edbbb409815a585af491335
* Optimization: sdsRemoveFreeSpace to avoid realloc on noop (#11766)uriyage2023-01-316-55/+42
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In #7875 (Redis 6.2), we changed the sds alloc to be the usable allocation size in order to: > reduce the need for realloc calls by making the sds implicitly take over the internal fragmentation This change was done most sds functions, excluding `sdsRemoveFreeSpace` and `sdsResize`, the reason is that in some places (e.g. clientsCronResizeQueryBuffer) we call sdsRemoveFreeSpace when we see excessive free space and want to trim it. so if we don't trim it exactly to size, the caller may still see excessive free space and call it again and again. However, this resulted in some excessive calls to realloc, even when there's no need and it's gonna be a no-op (e.g. when reducing 15 bytes allocation to 13). It turns out that a call for realloc with jemalloc can be expensive even if it ends up doing nothing, so this PR adds a check using `je_nallocx`, which is cheap to avoid the call for realloc. in addition to that this PR unifies sdsResize and sdsRemoveFreeSpace into common code. the difference between them was that sdsResize would avoid using SDS_TYPE_5, since it want to keep the string ready to be resized again, while sdsRemoveFreeSpace would permit using SDS_TYPE_5 and get an optimal memory consumption. now both methods take a `would_regrow` argument that makes it more explicit. the only actual impact of that is that in clientsCronResizeQueryBuffer we call both sdsResize and sdsRemoveFreeSpace for in different cases, and we now prevent the use of SDS_TYPE_5 in both. The new test that was added to cover this concern used to pass before this PR as well, this PR is just a performance optimization and cleanup. Benchmark: `redis-benchmark -c 100 -t set -d 512 -P 10 -n 100000000` on i7-9850H with jemalloc, shows improvement from 1021k ops/sec to 1067k (average of 3 runs). some 4.5% improvement. Co-authored-by: Oran Agra <oran@redislabs.com>
* Optimize the performance of cluster slots for non-continuous slots (#11745)Madelyn Olson2023-01-291-10/+25
| | | | | | | | | | | | | | This change improves the performance of cluster slots by removing the deferring lengths that are used. Deferring lengths are used in two contexts, the first is for determining the number of replicas that serve a slot (Added in 6.2 as part of a different performance improvement) and the second is for determining the extra networking options for each node (Added in 7.0). For continuous slots, (e.g. 0-8196) this improvement is very negligible, however it becomes more significant when slots are not continuous (e.g. 0 2 4 6 etc) which can happen in production for various users. The `cluster slots` command is deprecated in favor of `cluster shards`, but since most clients don't support the new command yet I think it's important to not degrade performance here. Benchmarking shows about 2x improvement, however I wasn't able to get a coherent TPS number since the benchmark process was being saturated long before Redis was, so had to run with multiple benchmarks and merge results. If needed I can add this to our memtier framework. Instead the next section shows the number of usec per call from the benchmark results, which shows significant improvement as well as having a more coherent response in the CoB. | | New Code | Old Code | % Improvements |----|----|----- |----- | Uniform slots| usec_per_call=10.46 | usec_per_call=11.03 | 5.7% | Worst case (Only even slots)| usec_per_call=963.80 | usec_per_call=2950.99 | 307% This change also removes some extra white space that I added a when making a code change for adding hostnames.
* Fix master client check in expireIfNeeded() for read only replica (#11761)Qu Chen2023-01-291-1/+1
| | | Redis 7.0 introduced new logic in expireIfNeeded() where a read-only replica would never consider a key as expired when replicating commands from the master. See acf3495. This was done by checking server.current_client with server.master. However, we should instead check for CLIENT_MASTER flag for this logic to be more robust and consistent with the rest of the Redis code base.
* update sentinel config condition (#11751)Wen Hui2023-01-263-3/+3
| | | | | | | | | | The command: sentinel config set option value and sentinel config get option They should include at least 4 arguments instead of 3, This PR fixes this issue. the only impact on the client is a different error message
* fix format for evalsha_ro.json file (#11756)Wen Hui2023-01-251-2/+2
| | | We should always use space instead of Tab, this PR fix the wrong code format
* Fix EVAL_RO json command format (#11755)Wen Hui2023-01-251-2/+2
| | | We should always use space instead of Tab, this PR fix the wrong code format
* fix typos in syscheck (#11710)artikell2023-01-221-1/+1
| | | replace "clokcsource" with "clocksource"
* Optimize the performance of sdscatrepr in printable characters (#11725)judeng2023-01-221-1/+2
| | | sdscatrepr is not the hot path in redis, but it's still useful to have make it less wasteful.
* Remove duplicate code in listAddNodeTail (#11733)王卿2023-01-201-10/+1
| | | Remove duplicate code that removes a node from the tail of a list.
* Key as dict entry - memory optimization for sets (#11595)Viktor Söderqvist2023-01-206-168/+340
| | | | | | | | | | | | | | | | | | | | | | | | | | | If a dict has only keys, and no use of values, then a key can be stored directly in a dict's hashtable. The key replaces the dictEntry. To distinguish between a key and a dictEntry, we only use this optimization if the key is odd, i.e. if the key has the least significant bit set. This is true for sds strings, since the sds header is always an odd number of bytes. Dict entries are used as a fallback when there is a hash collision. A special dict entry without a value (only key and next) is used so we save one word in this case too. This saves 24 bytes per set element for larges sets, and also gains some speed improvement as a side effect (less allocations and cache misses). A quick test adding 1M elements to a set using the command below resulted in memory usage of 28.83M, compared to 46.29M on unstable. That's 18 bytes per set element on average. eval 'for i=1,1000000,1 do redis.call("sadd", "myset", "x"..i) end' 0 Other changes: Allocations are ensured to have at least 8 bits alignment on all systems. This affects 32-bit builds compiled without HAVE_MALLOC_SIZE (not jemalloc or glibc) in which Redis stores the size of each allocation, after this change in 8 bytes instead of previously 4 bytes per allocation. This is done so we can reliably use the 3 least significant bits in a pointer to encode stuff.
* Obuf limit, exit during loop in *RAND* commands and KEYS (#11676)Oran Agra2023-01-164-0/+14
| | | | | | | | | | | | | | Related to the hang reported in #11671 Currently, redis can disconnect a client due to reaching output buffer limit, it'll also avoid feeding that output buffer with more data, but it will keep running the loop in the command (despite the client already being marked for disconnection) This PR is an attempt to mitigate the problem, specifically for commands that are easy to abuse, specifically: KEYS, HRANDFIELD, SRANDMEMBER, ZRANDMEMBER. The RAND family of commands can take a negative COUNT argument (which is not bound to the number of elements in the key), so it's enough to create a key with one field, and then these commands can be used to hang redis. For KEYS the caller can use the existing keyspace in redis (if big enough).
* Fix range issues in ZRANDMEMBER and HRANDFIELD (CVE-2023-22458) (#11674)Oran Agra2023-01-162-2/+12
| | | | missing range check in ZRANDMEMBER and HRANDIFLD leading to panic due to protocol limitations
* Avoid integer overflows in SETRANGE and SORT (CVE-2022-35977) (#11720)Oran Agra2023-01-162-8/+15
| | | | | Authenticated users issuing specially crafted SETRANGE and SORT(_RO) commands can trigger an integer overflow, resulting with Redis attempting to allocate impossible amounts of memory and abort with an OOM panic.
* Increase frequency of failover log and emit the status of the election to ↵harrylhl2023-01-112-2/+9
| | | | | | | help debugging (#11665) This change increase the frequency of the failover log from 5 minutes to 10 seconds. This log is only emitted when a replica has an outstanding election is progress, and waiting 5 minutes for the next log makes debugging and alarming on the log messages too slow. It also now prints out the number of votes the replica has currently received as well as the number of votes it needs to achieve quorum so that we can track the progress if it's running slowly. Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>
* Move stat_active_defrag_hits increment to activeDefragAllocViktor Söderqvist2023-01-113-184/+122
| | | | instead of passing it around to every defrag function
* Remove the bucket-cb from dictScan and move dictEntry defrag to dictScanDefragViktor Söderqvist2023-01-1110-109/+158
| | | | | | | | | | | | | | | | | | | This change deletes the dictGetNext and dictGetNextRef functions, so the dict API doesn't expose the next field at all. The bucket function in dictScan is deleted. A separate dictScanDefrag function is added which takes a defrag alloc function to defrag-reallocate the dict entries. "Dirty" code accessing the dict internals in active defrag is removed. An 'afterReplaceEntry' is added to dictType, which allows the dict user to keep the dictEntry metadata up to date after reallocation/defrag/move. Additionally, for updating the cluster slot-to-key mapping, after a dictEntry has been reallocated, we need to know which db a dict belongs to, so we store a pointer to the db in a new metadata section in the dict struct, which is a new mechanism similar to dictEntry metadata. This adds some complexity but provides better isolation.
* activeDefragSdsDict use scan instead of iterator and drop dictSetNextViktor Söderqvist2023-01-113-121/+41
| | | | Also delete unused function activeDefragSdsListAndDict
* Let active expire cycle use dictScan instead of messing with internalsViktor Söderqvist2023-01-111-49/+46
|
* Make dictEntry opaqueViktor Söderqvist2023-01-1114-119/+195
| | | | | Use functions for all accesses to dictEntry (except in dict.c). Dict abuses e.g. in defrag.c have been replaced by support functions provided by dict.
* Fixes typo (double word) in memory overcommit message (#11675)Krunoslav Husak2023-01-101-1/+1
| | | Fixes small typo in memory overcommit message in syscheck.c (double word can).
* Add minimum version information to new xsetid arguments (#11694)knggk2023-01-102-4/+6
| | | | | the metadata for the new arguments of XSETID, entries-added and max-deleted-id, which have been added in Redis 7.0 was missing.
* Make sure that fork child doesn't do incremental rehashing (#11692)Oran Agra2023-01-103-21/+32
| | | | | | Turns out that a fork child calling getExpire while persisting keys (and possibly also a result of some module fork tasks) could cause dictFind to do incremental rehashing in the child process, which is both a waste of time, and also causes COW harm.
* Blocking command with a 0.001 seconds timeout blocks indefinitely (#11688)Gabi Ganam2023-01-081-1/+3
| | | Any value in the range of [0-1) turns to 0 when being cast from double to long long. This change rounds up instead of down for values that can't be stored precisely as long doubles.
* Fix issues with listpack encoded set (#11685)Oran Agra2023-01-052-7/+20
| | | | | | | | PR #11290 added listpack encoding for sets, but was missing two things: 1. Correct handling of MEMORY USAGE (leading to an assertion). 2. Had an uncontrolled scratch buffer size in SRANDMEMBER leading to OOM panic (reported in #11668). Fixed by copying logic from ZRANDMEMBER. note that both issues didn't exist in any redis release.
* In cluster-mode enabled, override the databases config at startup to 1 (#11555)llahav-amzn2023-01-042-6/+7
| | | | | | | In cluster-mode, only DB0 is supported so all data must reside in that database. There is a single check that validates that data loaded from an RDB all resides in DB0. This check is performed after all the data is loaded which makes it difficult to identify where the non DB0 data resides as well as does a bunch of unnecessary work to load incompatible data. This change override the database config at startup to 1 to throw an error when attempting to add data to a database other than DB0. Co-authored-by: Eran Liberty <eranl@amazon.com> Co-authored-by: Oran Agra <oran@redislabs.com> Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
* Include peer-addr:port and local-addr:port when logging accept errors (#11622)Viktor Söderqvist2023-01-041-12/+17
| | | | The logged errors include these on the same format as in CLIENT INFO, e.g. "addr=127.0.0.1:12345 laddr=127.0.0.1:6379".
* Make redis-cli support PSYNC command (#11647)Binbin2023-01-041-22/+74
| | | | | | | | | | | | | | | | | | | | | | | | The current redis-cli does not support the real PSYNC command, the older version of redis-cli can support PSYNC is because that we actually issue the SYNC command instead of PSYNC, so it act like SYNC (always full-sync). Noted that in this case we will send the SYNC first (triggered by sendSync), then send the PSYNC (the one in redis-cli input). Didn't bother to find which version that the order changed, we send PSYNC first (the one in redis-cli input), and then send the SYNC (the one triggered by sendSync). So even full-sync is not working anymore, and it will result this output (mentioned in issue #11246): ``` psync dummy 0 Entering replica output mode... (press Ctrl-C to quit) SYNC with master, discarding bytes of bulk transfer until EOF marker... Error reading RDB payload while SYNCing ``` This PR adds PSYNC support to redis-cli, which can handle +FULLRESYNC and +CONTINUE responses, and some examples will follow. Co-authored-by: Oran Agra <oran@redislabs.com>
* Fix potential issue with Lua argv caching, module command filter and libc ↵Oran Agra2023-01-042-12/+18
| | | | | | | | | | | | | | | | | realloc (#11652) TLDR: solve a problem introduced in Redis 7.0.6 (#11541) with RM_CommandFilterArgInsert being called from scripts, which can lead to memory corruption. Libc realloc can return the same pointer even if the size was changed. The code in freeLuaRedisArgv had an assumption that if the pointer didn't change, then the allocation didn't change, and the cache can still be reused. However, if rewriteClientCommandArgument or RM_CommandFilterArgInsert were used, it could be that we realloced the argv array, and the pointer didn't change, then a consecutive command being executed from Lua can use that argv cache reaching beyond its size. This was actually only possible with modules, since the decision to realloc was based on argc, rather than argv_len.
* Introduce .is_local method for connection layer (#11672)zhenwei pi2023-01-047-15/+35
| | | | | | | | | | Introduce .is_local method to connection, and implement for TCP/TLS/ Unix socket, also drop 'int islocalClient(client *c)'. Then we can hide the detail into the specific connection types. Uplayer tests a connection is local or not by abstract method only. Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
* Optimize the performance of msetnx command by call lookupkey only once (#11594)judeng2023-01-031-1/+3
| | | | This is a small addition to #9640 It improves performance by avoiding double lookup of the the key.
* Add cluster info and cluster nodes to bug report (#11656)aradz442023-01-013-73/+95
| | | | | Adds the CLUSTER INFO and CLUSTER NODES to the bug report string. Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>
* reprocess command when client is unblocked on keys (#11012)ranshid2023-01-0115-760/+492
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | *TL;DR* --------------------------------------- Following the discussion over the issue [#7551](https://github.com/redis/redis/issues/7551) We decided to refactor the client blocking code to eliminate some of the code duplications and to rebuild the infrastructure better for future key blocking cases. *In this PR* --------------------------------------- 1. reprocess the command once a client becomes unblocked on key (instead of running custom code for the unblocked path that's different than the one that would have run if blocking wasn't needed) 2. eliminate some (now) irrelevant code for handling unblocking lists/zsets/streams etc... 3. modify some tests to intercept the error in cases of error on reprocess after unblock (see details in the notes section below) 4. replace '$' on the client argv with current stream id. Since once we reprocess the stream XREAD we need to read from the last msg and not wait for new msg in order to prevent endless block loop. 5. Added statistics to the info "Clients" section to report the: * `total_blocking_keys` - number of blocking keys * `total_blocking_keys_on_nokey` - number of blocking keys which have at least 1 client which would like to be unblocked on when the key is deleted. 6. Avoid expiring unblocked key during unblock. Previously we used to lookup the unblocked key which might have been expired during the lookup. Now we lookup the key using NOTOUCH and NOEXPIRE to avoid deleting it at this point, so propagating commands in blocked.c is no longer needed. 7. deprecated command flags. We decided to remove the CMD_CALL_STATS and CMD_CALL_SLOWLOG and make an explicit verification in the call() function in order to decide if stats update should take place. This should simplify the logic and also mitigate existing issues: for example module calls which are triggered as part of AOF loading might still report stats even though they are called during AOF loading. *Behavior changes* --------------------------------------------------- 1. As this implementation prevents writing dedicated code handling unblocked streams/lists/zsets, since we now re-process the command once the client is unblocked some errors will be reported differently. The old implementation used to issue ``UNBLOCKED the stream key no longer exists`` in the following cases: - The stream key has been deleted (ie. calling DEL) - The stream and group existed but the key type was changed by overriding it (ie. with set command) - The key not longer exists after we swapdb with a db which does not contains this key - After swapdb when the new db has this key but with different type. In the new implementation the reported errors will be the same as if the command was processed after effect: **NOGROUP** - in case key no longer exists, or **WRONGTYPE** in case the key was overridden with a different type. 2. Reprocessing the command means that some checks will be reevaluated once the client is unblocked. For example, ACL rules might change since the command originally was executed and will fail once the client is unblocked. Another example is OOM condition checks which might enable the command to run and block but fail the command reprocess once the client is unblocked. 3. One of the changes in this PR is that no command stats are being updated once the command is blocked (all stats will be updated once the client is unblocked). This implies that when we have many clients blocked, users will no longer be able to get that information from the command stats. However the information can still be gathered from the client list. **Client blocking** --------------------------------------------------- the blocking on key will still be triggered the same way as it is done today. in order to block the current client on list of keys, the call to blockForKeys will still need to be made which will perform the same as it is today: * add the client to the list of blocked clients on each key * keep the key with a matching list node (position in the global blocking clients list for that key) in the client private blocking key dict. * flag the client with CLIENT_BLOCKED * update blocking statistics * register the client on the timeout table **Key Unblock** --------------------------------------------------- Unblocking a specific key will be triggered (same as today) by calling signalKeyAsReady. the implementation in that part will stay the same as today - adding the key to the global readyList. The reason to maintain the readyList (as apposed to iterating over all clients blocked on the specific key) is in order to keep the signal operation as short as possible, since it is called during the command processing. The main change is that instead of going through a dedicated code path that operates the blocked command we will just call processPendingCommandsAndResetClient. **ClientUnblock (keys)** --------------------------------------------------- 1. Unblocking clients on keys will be triggered after command is processed and during the beforeSleep 8. the general schema is: 9. For each key *k* in the readyList: ``` For each client *c* which is blocked on *k*: in case either: 1. *k* exists AND the *k* type matches the current client blocking type OR 2. *k* exists and *c* is blocked on module command OR 3. *k* does not exists and *c* was blocked with the flag unblock_on_deleted_key do: 1. remove the client from the list of clients blocked on this key 2. remove the blocking list node from the client blocking key dict 3. remove the client from the timeout list 10. queue the client on the unblocked_clients list 11. *NEW*: call processCommandAndResetClient(c); ``` *NOTE:* for module blocked clients we will still call the moduleUnblockClientByHandle which will queue the client for processing in moduleUnblockedClients list. **Process Unblocked clients** --------------------------------------------------- The process of all unblocked clients is done in the beforeSleep and no change is planned in that part. The general schema will be: For each client *c* in server.unblocked_clients: * remove client from the server.unblocked_clients * set back the client readHandler * continue processing the pending command and input buffer. *Some notes regarding the new implementation* --------------------------------------------------- 1. Although it was proposed, it is currently difficult to remove the read handler from the client while it is blocked. The reason is that a blocked client should be unblocked when it is disconnected, or we might consume data into void. 2. While this PR mainly keep the current blocking logic as-is, there might be some future additions to the infrastructure that we would like to have: - allow non-preemptive blocking of client - sometimes we can think that a new kind of blocking can be expected to not be preempt. for example lets imagine we hold some keys on disk and when a command needs to process them it will block until the keys are uploaded. in this case we will want the client to not disconnect or be unblocked until the process is completed (remove the client read handler, prevent client timeout, disable unblock via debug command etc...). - allow generic blocking based on command declared keys - we might want to add a hook before command processing to check if any of the declared keys require the command to block. this way it would be easier to add new kinds of key-based blocking mechanisms. Co-authored-by: Oran Agra <oran@redislabs.com> Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
* Remove unnecessary updateClientMemUsageAndBucket() when feeding monitors ↵sundb2022-12-282-1/+6
| | | | | | | | (#11657) This call is introduced in #8687, but became irrelevant in #11348, and is currently a no-op. The fact is that #11348 an unintended side effect, which is that even if the client eviction config is enabled, there are certain types of clients for which memory consumption is not accurately tracked, and so unlike normal clients, their memory isn't reported correctly in INFO.
* Cleanup: Get rid of server.core_propagates (#11572)guybe72022-12-209-107/+88
| | | | | | | | 1. Get rid of server.core_propagates - we can just rely on module/call nesting levels 2. Rename in_nested_call to execution_nesting and update the comment 3. Remove module_ctx_nesting (redundant, we can use execution_nesting) 4. Modify postExecutionUnitOperations according to the comment (The main purpose of this PR) 5. trackingHandlePendingKeyInvalidations: Check the nesting level inside this function
* Fixed small distance replies on GEODIST and GEO commands WITHDIST (#11631)filipe oliveira2022-12-151-7/+17
| | | | | | Fixes a regression introduced by #11552 in 7.0.6. it causes replies in the GEO commands to contain garbage when the result is a very small distance (less than 1) Includes test to confirm indeed with junk in buffer now we properly reply
* Call postExecutionUnitOperations in active-expire of writable replicas (#11615)guybe72022-12-151-0/+2
| | | | | | | | We need to honor the post-execution-unit API and call it after each KSN Note that this is an edge case that only happens in case volatile keys were created directly on a writable replica, and that anyway nothing is propagated to sub-replicas Co-authored-by: Oran Agra <oran@redislabs.com>
* Fix zuiFind crash / RM_ScanKey hang on SET object listpack encoding (#11581)Binbin2022-12-098-58/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | In #11290, we added listpack encoding for SET object. But forgot to support it in zuiFind, causes ZINTER, ZINTERSTORE, ZINTERCARD, ZIDFF, ZDIFFSTORE to crash. And forgot to support it in RM_ScanKey, causes it hang. This PR add support SET listpack in zuiFind, and in RM_ScanKey. And add tests for related commands to cover this case. Other changes: - There is no reason for zuiFind to go into the internals of the SET. It can simply use setTypeIsMember and don't care about encoding. - Remove the `#include "intset.h"` from server.h reduce the chance of accidental intset API use. - Move setTypeAddAux, setTypeRemoveAux and setTypeIsMemberAux interfaces to the header. - In scanGenericCommand, use setTypeInitIterator and setTypeNext to handle OBJ_SET scan. - In RM_ScanKey, improve hash scan mode, use lpGetValue like zset, they can share code and better performance. The zuiFind part fixes #11578 Co-authored-by: Oran Agra <oran@redislabs.com> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
* Reduce rewriteClientCommandVector usage on EXPIRE command (#11602)filipe oliveira2022-12-091-4/+13
| | | | | | | | | | | | | | | There is overhead on Redis 7.0 EXPIRE command that is not present on 6.2.7. We could see that on the unstable profile there are around 7% of CPU cycles spent on rewriteClientCommandVector that are not present on 6.2.7. This was introduced in #8474. This PR reduces the overhead by using 2X rewriteClientCommandArgument instead of rewriteClientCommandVector. In this scenario rewriteClientCommandVector creates 4 arguments. the above usage of rewriteClientCommandArgument reduces the overhead in half. This PR should also improve PEXPIREAT performance by avoiding at all rewriteClientCommandArgument usage. Co-authored-by: Oran Agra <oran@redislabs.com>
* Correct RM_GetSharedAPI documentation. (#11601)Yossi Gottlieb2022-12-081-1/+1
| | | Fix wrong API name i example doc
* Normalize NAN to a single nan type, like we do with inf (#11597)Binbin2022-12-082-1/+29
| | | | | | | | | | | From https://en.wikipedia.org/wiki/NaN#Display, it says that apart from nan and -nan, we can also get NAN and even nan(char-sequence) from libc. In #11482, our conclusion was that we wanna normalize it in Redis to a single nan type, like we already normalized inf. For this, we also reverted the assert_match part of the test added in #11506, using assert_equal to validate the changes.
* Fix sentinel issue if replica changes IP (#11590)Moti Cohen2022-12-081-8/+3
| | | | | | | As Sentinel supports dynamic IP only when using hostnames, there are few leftover addess comparison logic that doesn't take into account that the IP might get change. Co-authored-by: moticless <moticless@github.com>
* Use SNI on outgoing TLS connections (#11458)CatboxParadox2022-12-071-0/+7
| | | When establishing an outgoing TLS connection using a hostname as a target, use TLS SNI extensions to include the hostname in use.
* Optimize client memory usage tracking operation while client eviction is ↵Harkrishn Patro2022-12-079-59/+147
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | disabled (#11348) ## Issue During the client input/output buffer processing, the memory usage is incrementally updated to keep track of clients going beyond a certain threshold `maxmemory-clients` to be evicted. However, this additional tracking activity leads to unnecessary CPU cycles wasted when no client-eviction is required. It is applicable in two cases. * `maxmemory-clients` is set to `0` which equates to no client eviction (applicable to all clients) * `CLIENT NO-EVICT` flag is set to `ON` which equates to a particular client not applicable for eviction. ## Solution * Disable client memory usage tracking during the read/write flow when `maxmemory-clients` is set to `0` or `client no-evict` is `on`. The memory usage is tracked only during the `clientCron` i.e. it gets periodically updated. * Cleanup the clients from the memory usage bucket when client eviction is disabled. * When the maxmemory-clients config is enabled or disabled at runtime, we immediately update the memory usage buckets for all clients (tested scanning 80000 took some 20ms) Benchmark shown that this can improve performance by about 5% in certain situations. Co-authored-by: Oran Agra <oran@redislabs.com>
* When converting a set to dict, presize for one more element to be added (#11559)Viktor Söderqvist2022-12-065-27/+63
| | | | | | | | | | | | | | | | | | | In most cases when a listpack or intset is converted to a dict, the conversion is trigged when adding an element. The extra element is added after conversion to dict (in all cases except when the conversion is triggered by set-max-intset-entries being reached). If set-max-listpack-entries is set to a power of two, let's say 128, when adding the 129th element, the 128 element listpack is first converted to a dict with a hashtable presized for 128 elements. After converting to dict, the 129th element is added to the dict which immediately triggers incremental rehashing to size 256. This commit instead presizes the dict to one more element, with the assumption that conversion to dict is followed by adding another element, so the dict doesn't immediately need rehashing. Co-authored-by: sundb <sundbcn@gmail.com> Co-authored-by: Oran Agra <oran@redislabs.com>
* Fix command line startup --sentinel problem (#11591)Binbin2022-12-061-0/+18
| | | | | | | | | | | | | | | There is a issue with --sentinel: ``` [root]# src/redis-server sentinel.conf --sentinel --loglevel verbose *** FATAL CONFIG FILE ERROR (Redis 255.255.255) *** Reading the configuration file, at line 352 >>> 'sentinel "--loglevel" "verbose"' Unrecognized sentinel configuration statement ``` This is because in #10660 (Redis 7.0.1), `--` prefix change break it. In this PR, we will handle `--sentinel` the same as we did for `--save` in #10866. i.e. it's a pseudo config option with no value.
* GEOSEARCH BYBOX: Simplified haversine distance formula when longitude diff ↵filipe oliveira2022-12-051-10/+18
| | | | | | | | | | | | | | is 0 (#11579) This is take 2 of `GEOSEARCH BYBOX` optimizations based on haversine distance formula when longitude diff is 0. The first one was in #11535 . - Given longitude diff is 0 the asin(sqrt(a)) on the haversine is asin(sin(abs(u))). - arcsin(sin(x)) equal to x when x ∈[−𝜋/2,𝜋/2]. - Given latitude is between [−𝜋/2,𝜋/2] we can simplifiy arcsin(sin(x)) to x. On the sample dataset with 60M datapoints, we've measured 55% increase in the achievable ops/sec.