forked from redis/redis
-
Notifications
You must be signed in to change notification settings - Fork 0
rebase from redis #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or 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
This makes it possible to add tests that generate assertions, and run them with valgrind, making sure that there are no memory violations prior to the assertion. New config options: - crash-log-enabled - can be disabled for cleaner core dumps - crash-memcheck-enabled - useful for faster termination after a crash - use-exit-on-panic - to be used by the test suite so that valgrind can detect leaks and memory corruptions Other changes: - Crash log is printed even on system that dont HAVE_BACKTRACE, i.e. in both SIGSEGV and assert / panic - Assertion and panic won't print registers and code around EIP (which was useless), but will do fast memory test (which may still indicate that the assertion was due to memory corrpution) I had to reshuffle code in order to re-use it, so i extracted come code into function without actually doing any changes to the code: - logServerInfo - logModulesInfo - doFastMemoryTest (with the exception of it being conditional) - dumpCodeAroundEIP changes to the crash report on segfault: - logRegisters is called right after the stack trace (before info) done just in order to have more re-usable code - stack trace skips the first two items on the stack (the crash log and signal handler functions)
this race would only happen when two threads paniced at the same time, and even then the only consequence is some extra log lines. race reported in #7391
…6271) Diskless master has some inherent latencies. 1) fork starts with delay from cron rather than immediately 2) replica is put online only after an ACK. but the ACK was sent only once a second. 3) but even if it would arrive immediately, it will not register in case cron didn't yet detect that the fork is done. Besides that, when a replica disconnects, it doesn't immediately attempts to re-connect, it waits for replication cron (one per second). in case it was already online, it may be important to try to re-connect as soon as possible, so that the backlog at the master doesn't vanish. In case it disconnected during rdb transfer, one can argue that it's not very important to re-connect immediately, but this is needed for the "diskless loading short read" test to be able to run 100 iterations in 5 seconds, rather than 3 (waiting for replication cron re-connection) changes in this commit: 1) sync command starts a fork immediately if no sync_delay is configured 2) replica sends REPLCONF ACK when done reading the rdb (rather than on 1s cron) 3) when a replica unexpectedly disconnets, it immediately tries to re-connect rather than waiting 1s 4) when when a child exits, if there is another replica waiting, we spawn a new one right away, instead of waiting for 1s replicationCron. 5) added a call to connectWithMaster from replicationSetMaster. which is called from the REPLICAOF command but also in 3 places in cluster.c, in all of these the connection attempt will now be immediate instead of delayed by 1 second. side note: we can add a call to rdbPipeReadHandler in replconfCommand when getting a REPLCONF ACK from the replica to solve a race where the replica got the entire rdb and EOF marker before we detected that the pipe was closed. in the test i did see this race happens in one about of some 300 runs, but i concluded that this race is unlikely in real life (where the replica is on another host and we're more likely to first detect the pipe was closed. the test runs 100 iterations in 3 seconds, so in some cases it'll take 4 seconds instead (waiting for another REPLCONF ACK). Removing unneeded startBgsaveForReplication from updateSlavesWaitingForBgsave Now that CheckChildrenDone is calling the new replicationStartPendingFork (extracted from serverCron) there's actually no need to call startBgsaveForReplication from updateSlavesWaitingForBgsave anymore, since as soon as updateSlavesWaitingForBgsave returns, CheckChildrenDone is calling replicationStartPendingFork that handles that anyway. The code in updateSlavesWaitingForBgsave had a bug in which it ignored repl-diskless-sync-delay, but removing that code shows that this bug was hiding another bug, which is that the max_idle should have used >= and not >, this one second delay has a big impact on my new test.
git-subtree-dir: deps/hiredis git-subtree-split: 39de5267c092859b4cab4bdf79081e9634b70e39
The else block would be executed when newlen == 0 and in the case memmove won't be called, so there's no need to set start.
…extReusedClient (#7323) Before this fix we where attempting to select a db before creating db the DB, see: #7323 This issue doesn't seem to have any implications, since the selected DB index is 0, the db pointer remains NULL, and will later be correctly set before using this dummy client for the first time. As we know, we call 'moduleInitModulesSystem()' before 'initServer()'. We will allocate memory for server.db in 'initServer', but we call 'createClient()' that will call 'selectDb()' in 'moduleInitModulesSystem()', before the databases where created. Instead, we should call 'createClient()' for moduleFreeContextReusedClient after 'initServer()'.
Add logic to redis-cli to display RESP3 PUSH messages when we detect STDOUT is a tty, with an optional command-line argument to override the default behavior. The new argument: --show-pushes <yn> Examples: $ redis-cli -3 --show-pushes no $ echo "client tracking on\nget k1\nset k1 v1"| redis-cli -3 --show-pushes y
…7554 (#7635) When runtest-cluster, at first, we need to create a cluster use spawn_instance, a port which is not used is choosen, however sometimes we can't run server on the port. possibley due to a race with another process taking it first. such as redis/redis/runs/896537490. It may be due to the machine problem or In order to reduce the probability of failure when start redis in runtest-cluster, we attemp to use another port when find server do not start up. Co-authored-by: Oran Agra <oran@redislabs.com> Co-authored-by: yanhui13 <yanhui13@meituan.com>
Added RedisModule_HoldString that either returns a shallow copy of the given String (by increasing the String ref count) or a new deep copy of String in case its not possible to get a shallow copy. Co-authored-by: Itamar Haber <itamar@redislabs.com>
Add redis-cli RESP3 Push support
…7645) In redismodule.h, RedisModule_DeauthenticateAndCloseClient returns void `void REDISMODULE_API_FUNC(RedisModule_DeauthenticateAndCloseClient)(RedisModuleCtx *ctx, uint64_t client_id);` But in module.c, RM_DeauthenticateAndCloseClient returns int `int RM_DeauthenticateAndCloseClient(RedisModuleCtx *ctx, uint64_t client_id)` It it safe to change return value from `void` to `int` from the user's perspective.
Appears to be handled by server.stream_node_max_bytes in reality.
signalKeyAsReady has some overhead (namely dictFind) so we should only call it when there are clients blocked on the relevant type (BLOCKED_*)
This is a rebased version of #3078 originally by shaharmor with the following patches by TysonAndre made after rebasing to work with the updated C API: 1. Add 2 more unit tests (wrong argument count error message, integer over 64 bits) 2. Use addReplyArrayLen instead of addReplyMultiBulkLen. 3. Undo changes to src/help.h - for the ZMSCORE PR, I heard those should instead be automatically generated from the redis-doc repo if it gets updated Motivations: - Example use case: Client code to efficiently check if each element of a set of 1000 items is a member of a set of 10 million items. (Similar to reasons for working on #7593) - HMGET and ZMSCORE already exist. This may lead to developers deciding to implement functionality that's best suited to a regular set with a data type of sorted set or hash map instead, for the multi-get support. Currently, multi commands or lua scripting to call sismember multiple times would almost definitely be less efficient than a native smismember for the following reasons: - Need to fetch the set from the string every time instead of reusing the C pointer. - Using pipelining or multi-commands would result in more bytes sent and received by the client for the repeated SISMEMBER KEY sections. - Need to specially encode the data and decode it from the client for lua-based solutions. - Proposed solutions using Lua or SADD/SDIFF could trigger writes to memory, which is undesirable on a redis replica server or when commands get replicated to replicas. Co-Authored-By: Shahar Mor <shahar@peer5.com> Co-Authored-By: Tyson Andre <tysonandre775@hotmail.com>
The two lines allow systemd to start redis.service after the network is online. Only after the network is online that Redis could bind to IP address other than 127.0.0.1 during initial boot up process.
Update adds a general source for retrieving a monotonic time. In addition, AE has been updated to utilize the new monotonic clock for timer processing. This performance improvement is **not** enabled in a default build due to various H/W compatibility concerns, see README.md for details. It does however change the default use of gettimeofday with clock_gettime and somewhat improves performance. This update provides the following 1. An interface for retrieving a monotonic clock. getMonotonicUs returns a uint64_t (aka monotime) with the number of micro-seconds from an arbitrary point. No more messing with tv_sec/tv_usec. Simple routines are provided for measuring elapsed milli-seconds or elapsed micro-seconds (the most common use case for a monotonic timer). No worries about time moving backwards. 2. High-speed assembler implementation for x86 and ARM. The standard method for retrieving the monotonic clock is POSIX.1b (1993): clock_gettime(CLOCK_MONOTONIC, timespec*). However, most modern processors provide a constant speed instruction clock which can be retrieved in a fraction of the time that it takes to call clock_gettime. For x86, this is provided by the RDTSC instruction. For ARM, this is provided by the CNTVCT_EL0 instruction. As a compile-time option, these high-speed timers can be chosen. (Default is POSIX clock_gettime.) 3. Refactor of event loop timers. The timer processing in ae.c has been refactored to use the new monotonic clock interface. This results in simpler/cleaner logic and improved performance.
The previous algorithm is of O(n^2) time complexity. It would have run through the ziplist entries one by one, each time doing a `realloc` and a `memmove` (moving the entire tail of the ziplist). The new algorithm is O(n), it runs over all the records once, computing the size of the `realloc` needed, then does one `realloc`, and run thought the records again doing many smaller `memmove`s, each time moving just one record. So this change reduces many reallocs, and moves each record just once. Co-authored-by: zhumaohua <zhumaohua@megvii.com> Co-authored-by: Oran Agra <oran@redislabs.com>
Don't assume `ps` handles `-h` to display output without headers and manually trim headers line from output.
This fixes the issue described in CVE-2014-5461. At this time we cannot confirm that the original issue has a real impact on Redis, but it is included as an extra safety measure.
All user-supplied variables that affect the build should be explicitly persisted. Fixes #7254
BUILD_WITH_SYSTEMD is an internal variable. Users should use USE_SYSTEMD=yes.
* Add master/slave option in --cluster call command * Update src/redis-cli.c * Update src/redis-cli.c Co-authored-by: Itamar Haber <itamar@redislabs.com>
When redis isn't configured to have a log file, having these prints before damonization puts them in the calling process stdout rather than /dev/null
DEBUG ZIPLIST <key> currently returns the following error string if the key is not a ziplist: "ERR Not an sds encoded string.". This looks like an accidental copy/paste error from the error returned in the else if branch above where this string is returned if the key is not an sds string. The command was added in ac61f90 and looking at the commit, nothing indicates that it is not an accidental typo. The error string now returns a correct error: "Not a ziplist encoded object", which accurately describes the error.
During long running scripts or loading RDB/AOF, we may need to do some defragging. Since processEventsWhileBlocked is called periodically at unknown intervals, and many cron jobs either depend on run_with_period (including active defrag), or rely on being called at server.hz rate (i.e. active defrag knows ho much time to run by looking at server.hz), the whileBlockedCron may have to run a loop triggering the cron jobs in it (currently only active defrag) several times. Other changes: - Adding a test for defrag during aof loading. - Changing key-load-delay config to take negative values for fractions of a microsecond sleep
Fix issues with writeConn() which resulted with corruption of the stream by leaving an extra byte in the buffer. The trigger for this is partial writes or write errors which were not experienced on Linux but reported on macOS.
this is important when running a test with --loop
- skip full units - skip a single test (not just a list of tests) - when skipping tag, skip spinning up servers, not just the tests - skip tags when running against an external server too - allow using multiple tags (split them)
reduce code duplication in aof.tcl. move creation of clients into the test so that it can be skipped
in some cases a command that returns an error possibly due to a timing issue causes the tcl code to crash and thus prevents the rest of the tests from running. this adds an option to make the test proceed despite the crash. maybe it should be the default mode some day.
- redirect valgrind reports to a dedicated file rather than console - try to avoid killing instances with SIGKILL so that we get the memory leak report (killing with SIGTERM before resorting to SIGKILL) - search for valgrind reports when done, print them and fail the tests - add --dont-clean option to keep the logs on exit - fix exit error code when crash is found (would have exited with 0) changes that affect the normal redis test suite: - refactor check_valgrind_errors into two functions one to search and one to report - move the search half into util.tcl to serve the cluster tests too - ignore "address range perms" valgrind warnings which seem non relevant.
Starting redis 6.0 and the changes we made to the diskless master to be suitable for TLS, I made the master avoid reaping (wait3) the pid of the child until we know all replicas are done reading their rdb. I did that in order to avoid a state where the rdb_child_pid is -1 but we don't yet want to start another fork (still busy serving that data to replicas). It turns out that the solution used so far was problematic in case the fork child was being killed (e.g. by the kernel OOM killer), in that case there's a chance that we currently disabled the read event on the rdb pipe, since we're waiting for a replica to become writable again. and in that scenario the master would have never realized the child exited, and the replica will remain hung too. Note that there's no mechanism to detect a hung replica while it's in rdb transfer state. The solution here is to add another pipe which is used by the parent to tell the child it is safe to exit. this mean that when the child exits, for whatever reason, it is safe to reap it. Besides that, i'm re-introducing an adjustment to REPLCONF ACK which was part of #6271 (Accelerate diskless master connections) but was dropped when that PR was rebased after the TLS fork/pipe changes (5a47794). Now that RdbPipeCleanup no longer calls checkChildrenDone, and the ACK has chance to detect that the child exited, it should be the one to call it so that we don't have to wait for cron (server.hz) to do that.
2b998de added a file for stderr to keep valgrind log but i forgot to add a similar thing when valgrind isn't being used. the result is that `glob */err.txt` fails.
There is an inherent race condition in port allocation for spawned servers. If a server fails to start because a port is taken, a new port is allocated. This fixes a problem where the logs are not truncated and as a result a large number of unmonitored servers are started.
This test was failing from time to time see discussion at the bottom of #7635 This was probably due to timing, the DEBUG SLEEP executed by redis-cli didn't sleep for enough time. This commit changes: 1) use SET-ACTIVE-EXPIRE instead of DEBUG SLEEP 2) reduce many `after` sleeps with retry loops to speed up the test. 3) add many comment explaining the different steps of the test and it's purpose. 4) config appendonly before populating the volatile keys, so that they'll be part of the AOF command stream rather than the preamble RDB portion. other complications: recently kill_instance switched from SIGKILL to SIGTERM, and this would sometimes fail since there was an AOFRW running in the background. now we wait for it to end before attempting the kill.
1) cur_test: when restart_server, "no such variable" error occurs
./runtest --single integration/rdb
test {client freed during loading}
SET ::cur_test
restart_server
kill_server
test "Check for memory leaks (pid $pid)"
SET ::cur_test
UNSET ::cur_test
UNSET ::cur_test // This global variable has been unset.
2) `ps --ppid` not available on macOS platform, can be replaced with
`pgrep -P pid`.
if there are nested tests and nested servers, we need to restore the
previous value of cur_test when a test exist.
example:
```
test{test 1} {
start_server {
test{test 1.1 - master only} {
}
start_server {
test{test 1.2 - with replication} {
}
}
}
}
```
when `test 1.1 - master only exists`, we're still inside `test 1`
* Tests: Some fixes for macOS
1) cur_test: when restart_server, "no such variable" error occurs
./runtest --single integration/rdb
test {client freed during loading}
SET ::cur_test
restart_server
kill_server
test "Check for memory leaks (pid $pid)"
SET ::cur_test
UNSET ::cur_test
UNSET ::cur_test // This global variable has been unset.
2) `ps --ppid` not available on macOS platform, can be replaced with
`pgrep -P pid`.
* handle cur_test for nested tests
if there are nested tests and nested servers, we need to restore the
previous value of cur_test when a test exist.
example:
```
test{test 1} {
start_server {
test{test 1.1 - master only} {
}
start_server {
test{test 1.2 - with replication} {
}
}
}
}
```
when `test 1.1 - master only exists`, we're still inside `test 1`
Co-authored-by: Oran Agra <oran@redislabs.com>
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.
rebase from redis