- 19 Apr, 2021 3 commits
-
-
guybe7 authored
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.
-
Harkrishn Patro authored
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>
-
sundb authored
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).
-
- 18 Apr, 2021 3 commits
-
-
Yossi Gottlieb authored
Disables #8649 and subsequent attempts to stabilize the test.
-
Oran Agra authored
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.
-
Oran Agra authored
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
-
- 15 Apr, 2021 1 commit
-
-
guybe7 authored
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.
-
- 14 Apr, 2021 1 commit
-
-
YaacovHazan authored
replace the hardcoded after 2000, with waiting for the sync and wait for condition
-
- 13 Apr, 2021 2 commits
-
-
Oran Agra authored
- 12 Apr, 2021 1 commit
-
-
Yossi Gottlieb authored
Disable replica migration to avoid a race condition where the migrated-from node turns into a replica. Long term, this test should probably be improved to handle multiple slots and accept such auto migrations but this is a quick fix to stabilize the CI without completely dropping this test.
-
- 07 Apr, 2021 1 commit
-
-
Yang Bodong authored
Fix out of range error messages to be clearer (avoid mentioning 9223372036854775807) * Fix XAUTOCLAIM COUNT option confusing error msg * Fix other RPOP and alike error message to mention positive
-
- 06 Apr, 2021 2 commits
-
-
Bonsai authored
With this fix, module data type registration will fail if the load or save callbacks are not defined, or the optional aux load and save callbacks are not either both defined or both missing.
-
Yossi Gottlieb authored
This is work in progress, focusing on two main areas: * Avoiding race conditions with cluster configuration propagation. * Ignoring limitations with redis-cli --cluster fix which makes it hard to distinguish real errors (e.g. failure to fix) from expected conditions in this test (e.g. nodes not agreeing on configuration).
-
- 05 Apr, 2021 1 commit
-
-
Huang Zhw authored
Fix "default" and overwritten / reset users will not have pubsub channels permissions by default. (#8723) Background: Redis 6.2 added ACL control for pubsub channels (#7993), which were supposed to be permissive by default to retain compatibility with redis 6.0 ACL. But due to a bug, only newly created users got this `acl-pubsub-default` applied, while overwritten (updated) users got reset to `resetchannels` (denied). Since the "default" user exists before loading the config file, any ACL change to it, results in an update / overwrite. So when a "default" user is loaded from config file or include ACL file with no channels related rules, the user will not have any permissions to any channels. But other users will have default permissions to any channels. When upgraded from 6.0 with config rewrite, this will lead to "default" user channels permissions lost. When users are loaded from include file, then call "acl load", users will also lost channels permissions. Similarly, the `reset` ACL rule, would have reset the user to be denied access to any channels, ignoring `acl-pubsub-default` and breaking compatibility with redis 6.0. The implication of this fix is that it regains compatibility with redis 6.0, but breaks compatibility with redis 6.2.0 and 2.0.1. e.g. after the upgrade, the default user will regain access to pubsub channels. Other changes: Additionally this commit rename server.acl_pubusub_default to server.acl_pubsub_default and fix typo in acl tests.
-
- 04 Apr, 2021 1 commit
-
-
Sokolov Yura authored
Previously (and by default after commit) when master loose its last slot (due to migration, for example), its replicas will migrate to new last slot holder. There are cases where this is not desired: * Consolidation that results with removed nodes (including the replica, eventually). * Manually configured cluster topologies, which the admin wishes to preserve. Needlessly migrating a replica triggers a full synchronization and can have a negative impact, so we prefer to be able to avoid it where possible. This commit adds 'cluster-allow-replica-migration' configuration option that is enabled by default to preserve existed behavior. When disabled, replicas will not be auto-migrated. Fixes #4896 Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 01 Apr, 2021 4 commits
-
-
Valentino Geron authored
This command used to return the last scanned entry id as the cursor, instead of the next one to be scanned. so in the next call, the user could / should have sent `(cursor` and not just `cursor` if he wanted to avoid scanning the same record twice. Scanning the record twice would look odd if someone is checking what exactly was scanned, but it also has a side effect of incrementing the delivery count twice.
-
Oran Agra authored
5629dbe7 added a change that configures the tcp (plaintext) port alongside the tls port, this causes the INFO command for tcp_port to return that instead of the tls port when running in tls, and that broke the sentinel tests that query it. the fix is to add a method that gets the right port from CONFIG instead of relying on the tcp_port info field.
-
guybe7 authored
If GT/LT fails the operation we need to reply with nill (like failure due to NX). Other changes: Add the missing $encoding suffix to many zset tests Note: there's a behavior change just in case of INCR + GT/LT that fails. The old code was replying with the wrong (rejected) score, and now it'll reply with nil. Note that that's anyway a corner case so this "behavior change" shouldn't have too much affect. Using GT/LT with INCR has a predictable result even before we run the command (INCR GT will only only / always fail if the increment is negative).
-
sundb authored
Problem: Currently, when performing random distribution verification, we determine the probability of each element occurring in the sum, but the probability is only an estimate, these tests had rare sporadic failures, and we cannot verify what the probability of failure will be. Solution: Using the chi-square distribution instead of the original random distribution validation makes the test more reasonable and easier to find problems.
-
- 30 Mar, 2021 5 commits
-
-
Jérôme Loyet authored
The 'sentinel replicas <master>' command will ignore replicas with `replica-announced` set to no. The goal of disabling the config setting replica-announced is to allow ghost replicas. The replica is in the cluster, synchronize with its master, can be promoted to master and is not exposed to sentinel clients. This way, it is acting as a live backup or living ghost. In addition, to prevent the replica to be promoted as master, set replica-priority to 0.
-
Yossi Gottlieb authored
* Dump more output on error (always, cluster tests currently have no verbose flag). * Slow down redis-cli check iteration.
-
Viktor Söderqvist authored
The cluster bus is established over TLS or non-TLS depending on the configuration tls-cluster. The client ports distributed in the cluster and sent to clients are assumed to be TLS or non-TLS also depending on tls-cluster. The cluster bus is now extended to also contain the non-TLS port of clients in a TLS cluster, when available. The non-TLS port of a cluster node, when available, is sent to clients connected without TLS in responses to CLUSTER SLOTS, CLUSTER NODES, CLUSTER SLAVES and MOVED and ASK redirects, instead of the TLS port. The user was able to override the client port by defining cluster-announce-port. Now cluster-announce-tls-port is added, so the user can define an alternative announce port for both TLS and non-TLS clients. Fixes #8134
-
JunhuaY authored
the bug was also discussed in #8716, and was solved in #8719, but incompletely: when the server is started, and the save option is default, if you issue the " config set save "" " to change the save option, and then issue the “config rewrite” command, the " save "" " won't be saved.
-
Oran Agra authored
Another test race condition in the macos tests. the test was waiting for PINGs to be generated and put on the replication stream, but waiting for 1 or 2 seconds doesn't really guarantee that. then the test that expected 6 full syncs, found only 4
-
- 29 Mar, 2021 3 commits
-
-
Yossi Gottlieb authored
-
Sokolov Yura authored
Add tests for fixing migrating slot at all stages: 1. when migration is half inited on "migrating" node 2. when migration is half inited on "importing" node 3. migration inited, but not finished 4. migration is half finished on "migrating" node 5. migration is half finished on "importing" node Also add tests for many simultaneous slot migrations. Co-authored-by:
Yossi Gottlieb <yossigo@gmail.com>
-
Meir Shpilraien (Spielrein) authored
'processCommandAndResetClient' returns 1 if client is dead. It does it by checking if serve.current_client is NULL. On script timeout, Redis will re-enter 'processCommandAndResetClient' and when finish we will set server.current_client to NULL. This will cause later to falsely return 1 and think that the client that sent the timed-out script is dead (Redis to stop reading from the client buffer).
-
- 26 Mar, 2021 1 commit
-
-
Huang Zhw authored
Add publish channel permissions check in processCommand. processCommand didn't check publish channel permissions, so we can queue a publish command in a transaction. But when exec the transaction, it will fail with -NOPERM. We also union keys/commands/channels permissions check togegher in ACLCheckAllPerm. Remove pubsubCheckACLPermissionsOrReply in publishCommand/subscribeCommand/psubscribeCommand. Always check permissions in processCommand/execCommand/ luaRedisGenericCommand.
-
- 25 Mar, 2021 1 commit
-
-
Oran Agra authored
* SLOWLOG didn't record anything for blocked commands because the client was reset and argv was already empty. there was a fix for this issue specifically for modules, now it works for all blocked clients. * The original command argv (before being re-written) was also reset before adding the slowlog on behalf of the blocked command. * Latency monitor is now updated regardless of the slowlog flags of the command or its execution (their purpose is to hide sensitive info from the slowlog, not hide the fact the latency happened). * Latency monitor now uses real_cmd rather than c->cmd (which may be different if the command got re-written, e.g. GEOADD) Changes: * Unify shared code between slowlog insertion in call() and updateStatsOnUnblock(), hopefully prevent future bugs from happening due to the later being overlooked. * Reset CLIENT_PREVENT_LOGGING in resetClient rather than after command processing. * Add a test for SLOWLOG and BLPOP Notes: - real_cmd == c->lastcmd, except inside MULTI and Lua. - blocked commands never happen in these cases (MULTI / Lua) - real_cmd == c->cmd, except for when the command is rewritten (e.g. GEOADD) - blocked commands (currently) are never rewritten - other than the command's CLIENT_PREVENT_LOGGING, and the execution flag CLIENT_PREVENT_LOGGING, other cases that we want to avoid slowlog are on AOF loading (specifically CMD_CALL_SLOWLOG will be off when executed from execCommand that runs from an AOF)
-
- 24 Mar, 2021 3 commits
-
-
Qu Chen authored
Properly initialize variable to make valgrind happy in checkChildrenDone(). Removed usage for the obsolete wait3() and wait4() in favor of waitpid(), and properly check for the exit status code. (#8666)
-
yoav-steinberg authored
This fixes a race where a bgsave can start during the test after we verified no bgsave is running.
-
Oran Agra authored
the corrupt-dump-fuzzer test found a case where an access to a corrupt stream would have caused accessing to uninitialized memory. now it'll panic instead. The issue was that there was a stream that says it has more than 0 records, but looking for the max ID came back empty handed. p.s. when sanitize-dump-payload is used, this corruption is detected, and the RESTORE command is gracefully rejected.
-
- 22 Mar, 2021 3 commits
-
-
Yossi Gottlieb authored
-
Oran Agra authored
-
Oran Agra authored
Since redis 6.2, redis immediately tries to connect to the master, not waiting for replication cron. in the slow freebsd CI, this test failed and master_link_status was already "up" when INFO was called.
-
- 17 Mar, 2021 1 commit
-
-
Meir Shpilraien (Spielrein) authored
pcall function runs another LUA function in protected mode, this means that any error will be caught by this function and will not stop the LUA execution. The script kill mechanism uses error to stop the running script. Scripts that uses pcall can catch the error raise by the script kill mechanism, this will cause a script like this to be unkillable: local f = function() while 1 do redis.call('ping') end end while 1 do pcall(f) end The fix is, when we want to kill the script, we set the hook function to be invoked after each line. This will promise that the execution will get another error before it is able to enter the pcall function again.
-
- 16 Mar, 2021 2 commits
-
-
Huang Zhw authored
In certain scenario start_server may think it failed to start a redis server although it started successfully. in these cases, it'll not terminate it, and it'll remain running when the test is over. In start_server if config doesn't have bind (the minimal.conf in introspection.tcl), it will try to bind ipv4 and ipv6. One may success while other fails. It will output "Could not create server TCP listening socket". wait_server_started uses this message to check whether instance started successfully. So it will consider that it failed even though redis started successfully. Additionally, in some cases it wasn't clear to users why the server exited, since the warning message printed to the log, could in some cases be harmless, and in some cases fatal. This PR adds makes a clear distinction between a warning log message and a fatal one, and changes the test suite to look for the fatal message.
-
Madelyn Olson authored
Redact config set requirepass/masterauth/masteruser from slowlog in addition to showing ACL commands without sensitive values.
-
- 15 Mar, 2021 1 commit
-
-
guybe7 authored
1. moduleReplicateMultiIfNeeded should use server.in_eval like moduleHandlePropagationAfterCommandCallback 2. server.in_eval could have been set to 1 and not reset back to 0 (a lot of missed early-exits after in_eval is already 1) Note: The new assertions in processCommand cover (2) and I added two module tests to cover (1) Implications: If an EVAL that failed (and thus left server.in_eval=1) runs before a module command that replicates, the replication stream will contain MULTI (because moduleReplicateMultiIfNeeded used to check server.lua_caller which is NULL at this point) but not EXEC (because server.in_eval==1) This only affects modules as module.c the only user of server.in_eval. Affects versions 6.2.0, 6.2.1
-