Release 6.2.2 #8817
Merged
Release 6.2.2 #8817
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Oran Agra <oran@redislabs.com>
…clients drop. (#8588) When no connected clients variables stopped being updated instead of being zeroed over time. The other changes are cleanup and optimization (avoiding an unnecessary division per client)
This solves the problem of /dev/random and /dev/urandom open file descriptors leaking to childs with some versions of OpenSSL.
Since the API declared (as #define) in redismodule.h and uses the CLIENT_ID_AOF that declared in the server.h, when a module will want to make use of this API, it will get a compilation error (module doesn't include the server.h). The API was broken by d6eb3af (failed attempt for a cleanup). Revert to the original version of RedisModule_IsAOFClient that uses UINT64_MAX instead of CLIENT_ID_AOF
When the length of the quicklist is 1(only one zipmap), the rotate operation will cause memory overlap when moving an entity from the tail of the zipmap to the head. quicklistRotate is a dead code, so it has no impact on the existing code.
* The `redis-cli --scan` output should honor output mode (set explicitly or implicitly), and quote key names when not in raw mode. * Technically this is a breaking change, but it should be very minor since raw mode is by default on for non-tty output. * It should only affect TTY output (human users) or non-tty output if `--no-raw` is specified. * Added `--quoted-input` option to treat all arguments as potentially quoted strings. * Added `--quoted-pattern` option to accept a potentially quoted pattern. Unquoting is applied to potentially quoted input only if single or double quotes are used. Fixes #8561, #8563
Remove unused latency variable from redis-cli
The result of `sdscatprintf` is doubled when using argument twice.
Scenario: 1. A module client is blocked on keys with a timeout 2. Shortly before the timeout expires, the key is being populated and signaled as ready 3. Redis calls moduleTryServeClientBlockedOnKey (which replies to client) and then moduleUnblockClient 4. moduleUnblockClient doesn't really unblock the client, it writes to server.module_blocked_pipe and only marks the BC as unblocked. 5. beforeSleep kics in, by this time the client still exists and techincally timed-out. beforeSleep replies to the timeout client (double reply) and only then moduleHandleBlockedClients is called, reading from module_blocked_pipe and calling unblockClient The solution is similar to what was done in moduleTryServeClientBlockedOnKey: we should avoid re-processing an already-unblocked client
It seems like non-Linux sockets may be less greedy, resulting with more transient client output buffers. Haven't proven this but empirically when stressing this test on non-Linux tends to exhibit increased mem_clients_normal values.
When a quicklist has quicklist->compress * 2 nodes, then call __quicklistCompress, all nodes will be decompressed and the middle two nodes will be recompressed again. This violates the fact that quicklist->compress * 2 nodes are uncompressed. It's harmless because when visit a node, we always try to uncompress node first. This only happened when a quicklist has quicklist->compress * 2 + 1 nodes, then delete a node. For other scenarios like insert node and iterate this will not happen.
The obtained process_rss was incorrect (the OS reports pages, not bytes), resulting with many other fields getting corrupted. This has been tested on FreeBSD but not other platforms.
Co-authored-by: Harkrishn Patro <harkrisp@amazon.com>
1. Add `redis-server test all` support to run all tests. 2. Add redis test to daily ci. 3. Add `--accurate` option to run slow tests for more iterations (so that by default we run less cycles (shorter time, and less prints). 4. Move dict benchmark to REDIS_TEST. 5. fix some leaks in tests 6. make quicklist tests run on a specific fill set of options rather than huge ranges 7. move some prints in quicklist test outside their loops to reduce prints 8. removing sds.h from dict.c since it is now used in both redis-server and redis-cli (uses hiredis sds)
The implication is that OBJECT command would was not updating stat_keyspace_misses
`master_sync_perc` and `loading_loaded_perc` don't have that sign, and i think the info field should be a raw floating point number (the name suggests its units). we already have `used_memory_peak_perc` and `used_memory_dataset_perc` which do add the `%` sign, but: 1) i think it was a mistake but maybe too late to fix now, and maybe not too late to fix for `current_fork_perc` 2) it is more important to be consistent with the two other "progress" "prec" metrics, and not with the "utilization" metric.
Have a clear separation between in and out flags Other changes: delete dead code in RM_ZsetIncrby: if zsetAdd returned error (happens only if the result of the operation is NAN or if score is NAN) we return immediately so there is no way that zsetAdd succeeded and returned NAN in the out-flags
Bug 1: When a module ctx is freed moduleHandlePropagationAfterCommandCallback is called and handles propagation. We want to prevent it from propagating commands that were not replicated by the same context. Example: 1. module1.foo does: RM_Replicate(cmd1); RM_Call(cmd2); RM_Replicate(cmd3) 2. RM_Replicate(cmd1) propagates MULTI and adds cmd1 to also_propagagte 3. RM_Call(cmd2) create a new ctx, calls call() and destroys the ctx. 4. moduleHandlePropagationAfterCommandCallback is called, calling alsoPropagates EXEC (Note: EXEC is still not written to socket), setting server.in_trnsaction = 0 5. RM_Replicate(cmd3) is called, propagagting yet another MULTI (now we have nested MULTI calls, which is no good) and then cmd3 We must prevent RM_Call(cmd2) from resetting server.in_transaction. REDISMODULE_CTX_MULTI_EMITTED was revived for that purpose. Bug 2: Fix issues with nested RM_Call where some have '!' and some don't. Example: 1. module1.foo does RM_Call of module2.bar without replication (i.e. no '!') 2. module2.bar internally calls RM_Call of INCR with '!' 3. at the end of module1.foo we call RM_ReplicateVerbatim We want the replica/AOF to see only module1.foo and not the INCR from module2.bar Introduced a global replication_allowed flag inside RM_Call to determine whether we need to replicate or not (even if '!' was specified) Other changes: Split beforePropagateMultiOrExec to beforePropagateMulti afterPropagateExec just for better readability
…8633) Co-authored-by: Harkrishn Patro <harkrisp@amazon.com>
* Modules API docs: Link API function names to their definitions Occurrences of API functions are linked to their definition. A function index with links to all functions is added on the bottom of the page. Comment blocks in module.c starting with a markdown h2 heading are used as sections. A table of contents is generated from these headings. The functions names are changed from h2 to h3, since they are now rendered as sub-headings within each section. Existing sections in module.c are used with some minor changes. Some documentation text is added or sligtly modified. The markdown renderer will add IDs which may clash with our generated IDs. By prefixing section IDs with "section-" we make them different. Replace double dashes with a unicode long ndash
replace the hardcoded after 2000, with waiting for the sync and wait for condition
Starting redis 6.0 (part of the TLS feature), diskless master uses pipe from the fork child so that the parent is the one sending data to the replicas. This mechanism has an issue in which a hung replica will cause the master to wait for it to read the data sent to it forever, thus preventing the fork child from terminating and preventing the creations of any other forks. This PR adds a timeout mechanism, much like the ACK-based timeout, we disconnect replicas that aren't reading the RDB file fast enough.
1. the `dump_logs` option would have printed only logs of servers that were spawn before the test proc started, and not ones that the test proc started inside it. 2. when a server proc catches an exception it should normally forward the exception upwards, specifically when it's an assertion that should be caught by a test proc above. however, in `durable` mode, we caught all exceptions printed them to stdout and let the code continue, this was wrong to do for assertions, which should have still been propagated to the test function. 3. don't bother to search for crash log to print if we printed the the entire log anyway 4. if no crash log was found, no need to print anything (i.e. the fact it wasn't found) 5. rename warnings_from_file to crashlog_from_file
In github actions CI with valgrind, i saw that even the fast replica (one that wasn't paused), didn't get to complete the replication fast enough, and ended up getting disconnected by timeout. Additionally, due to a typo in uname, we didn't get to actually run the CPU efficiency part of the test.
Disables #8649 and subsequent attempts to stabilize the test.
When replica never successfully connect to master, server.repl_down_since will be initialized to 0, therefore, the info master_link_down_since_seconds was showing the current unix timestamp, which does not make much sense. This commit fixes the issue by showing master_link_down_since_seconds to -1. means the replica never connect to master before. This commit also resets this variable back to 0 when a replica is turned into a master, so that it'll behave the same if the master is later turned into a replica again. The implication of this change is that if some app is checking if the value is > 60 do something, like conclude the replica is stale, this could case harm (changing a big positive number with a small one).
The tail size of c->reply is 16kb, but in the test only publish a few chars each time, due to a change in #8699, the obuf limit is now checked a new memory allocation is made, so this test would have sometimes failed to trigger a soft limit disconnection in time. The solution is to write bigger payloads to the output buffer, but still limit their rate (not more than 100k/s).
In the initial release of Redis 6.2 setting a user to only allow pubsub access to a specific channel, and doing ACL SAVE, resulted in an assertion when ACL LOAD was used. This was later changed by #8723 (not yet released), but still not properly resolved (now it errors instead of crash). The problem is that the server that generates an ACL file, doesn't know what would be the setting of the acl-pubsub-default config in the server that will load it. so ACL SAVE needs to always start with resetchannels directive. This should still be compatible with old acl files (from redis 6.0), and ones from earlier versions of 6.2 that didn't mess with channels. Co-authored-by: Harkrishn Patro <harkrisp@amazon.com> Co-authored-by: Oran Agra <oran@redislabs.com>
) Before this commit using RM_Call without "!" could cause the master to lazy-expire a key (delete it) but without replicating to replicas. This could cause the replica's memory usage to gradually grow and could also cause consistency issues if the master and replica have a clock diff. This bug was introduced in #8617 Added a test which demonstrates that scenario.
yossigo
previously approved these changes
Apr 19, 2021
itamarhaber
previously approved these changes
Apr 19, 2021
madolson
previously approved these changes
Apr 19, 2021
Adding a new type mask for key space notification, REDISMODULE_NOTIFY_MODULE, to enable unique notifications from commands on REDISMODULE_KEYTYPE_MODULE type keys (which is currently unsupported). Modules can subscribe to a module key keyspace notification by RM_SubscribeToKeyspaceEvents, and clients by notify-keyspace-events of redis.conf or via the CONFIG SET, with the characters 'd' or 'A' (REDISMODULE_NOTIFY_MODULE type mask is part of the '**A**ll' notation for key space notifications). Refactor: move some pubsub test infra from pubsub.tcl to util.tcl to be re-used by other tests.
oranagra
dismissed stale reviews from madolson, itamarhaber, and yossigo
via
aa730ef
yossigo
approved these changes
Apr 19, 2021
itamarhaber
approved these changes
Apr 19, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upgrade urgency: HIGH, if you're using ACL and pub/sub, CONFIG REWRITE, or
suffering from performance regression. see below.
Bug fixes for regressions in previous releases of Redis 6.2:
saveconfig resulting in defaultsavevalues (Fix config rewrite with an empty "save" parameter. #8719)Several issues around nested calls and thread-safe contexts
Bug fixes that are only applicable to previous releases of Redis 6.2:
Bug fixes:
pcall(Fix script kill to work also on scripts that use pcall #8661)Command behavior changes:
It was responding with the incremented value rather than nil
Previous behavior was returning the last one which was already scanned
New config options:
Improvements:
Info fields and introspection changes:
Platform and deployment-related changes:
Modules: