Previously invalid configuration errors were not very specific and in some cases hard to understand.
Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
* Adds ASYNC and SYNC arguments to SCRIPT FLUSH
* Adds SYNC argument to FLUSHDB and FLUSHALL
* Adds new config to control the default behavior of FLUSHDB, FLUSHALL and SCRIPT FLUASH.
the new behavior is as follows:
* FLUSH[ALL|DB],SCRIPT FLUSH: Determine sync or async according to the
value of lazyfree-lazy-user-flush.
* FLUSH[ALL|DB],SCRIPT FLUSH ASYNC: Always flushes the database in an async manner.
* FLUSH[ALL|DB],SCRIPT FLUSH SYNC: Always flushes the database in a sync manner.
The prefix is changed from `RM_` to `module` on the following
internal functions, to prevent them from appearing in the API docs:
RM_LogRaw -> moduleLogRaw
RM_FreeCallReplyRec -> moduleFreeCallReplyRec
RM_ZsetAddFlagsToCoreFlags -> moduleZsetAddFlagsToCoreFlags
RM_ZsetAddFlagsFromCoreFlags -> moduleZsetAddFlagsFromCoreFlags
Fixes markdown formatting errors and some functions not showing
up in the generated documentation at all.
Ruby script (gendoc.rb) fixes:
* Modified automatic instertion of backquotes:
* Don't add backquotes around names which are already preceded by a
backquote. Fixes for example \`RedisModule_Reply\*\` which turning
into \`\`RedisModule_Reply\`\*\` messes up the formatting.
* Add backquotes around types such as RedisModuleString (in addition
to function names `RedisModule_[A-z()]*` and macro names
`REDISMODULE_[A-z]*`).
* Require 4 spaces indentation for disabling automatic backquotes, i.e.
code blocks. Fixes continuations of list items (indented 2 spaces).
* More permissive extraction of doc comments:
* Allow doc comments starting with `/**`.
* Make space before `*` on each line optional.
* Make space after `/*` and `/**` optional (needed when appearing on
its own line).
Markdown fixes in module.c:
* Fix code blocks not indented enough (4 spaces needed).
* Add black line before code blocks and lists where missing (needed).
* Enclose special markdown characters `_*^<>` in backticks to prevent them
from messing up formatting.
* Lists with `1)` changed to `1.` for proper markdown lists.
* Remove excessive indentation which causes text to be unintentionally
rendered as code blocks.
* Other minor formatting fixes.
Other fixes in module.c:
* Remove blank lines between doc comment and function definition. A blank
line here makes the Ruby script exclude the function in docs.
* Change zunionInterDiffGenericCommand to use lookupKeyRead if dstkey is null
* Change zrangeGenericCommand to use lookupKey Write if dstkey isn't null
ZRANGESTORE and UNION, ZINTER, ZDIFF are all new commands (6.2 RC1 and RC2).
In redis 6.0 the ZRANGE was using lookupKeyRead, and ZUNIONSTORE / ZINTERSTORE were using lookupKeyWrite.
So there bugs are introduced in 6.2 and will be resolved before it is released.
the implications of this bug are also not big:
The sole difference between LookupKeyRead and LookupKeyWrite is for command executed on a replica, which are not received from its master client. (for the master, and for the master client on the replica, these two functions behave the same)!
This fixes three issues:
1. Using debug SLEEP was impacting the subsequent test, and causing it to pass reliably even though it should have failed. There was exactly 5 seconds of artificial pause (after 1000, wait 3000, wait 1000) between the debug sleep 5 and when we needed to unblock the client in the subsequent test. Now the test properly makes sure the client is unblocked, and the subsequent test is fixed.
2. Minor, the client pause types were using & comparisons instead of ==, since it was previously a flag.
3. Test is faster now that some of the hand wavy time is removed.
When the server state changes and blocked clients are being dropped, the
paused clients should not be dropped, they're safe to keep since unlike
other blocked types, these commands are not half way though processing,
and the commands they sent may get rejected according to the new server
state.
- the last COW report wasn't always read from the pipe
(receiveLastChildInfo wasn't used)
- but in fact, there's no reason we won't always try to drain that pipe
so i'm unifying receiveLastChildInfo with receiveChildInfo
- adjust threshold of the COW test when run in accurate mode
- add some prints in case this test fails again
- fix indentation, page size, and PID! in MacOS proc info
p.s. it seems that pri_pages_dirtied is always 0
This PR adds another trimming strategy to XADD and XTRIM named MINID
(complements the existing MAXLEN).
It also adds a new LIMIT argument that allows incremental trimming by repeated
calls (rather than all at once).
This provides the ability to trim all records older than a certain ID (which makes it
possible for the user to trim by age too).
Example:
XTRIM mystream MINID ~ 1608540753 will trim entries with id < 1608540753,
but might not trim all (because of the ~ modifier)
The purpose is to ease the use of streams. many users use streams as logs and
the common case is wanting a log
of the last X seconds rather than a log that contains maximum X entries (new
MINID vs existing MAXLEN)
The new LIMIT modifier is only supported when the trim strategy uses ~.
i.e. when the user asked for exact trimming, it all happens in one go (no
possibility for incremental trimming).
However, when ~ is provided, we trim full rax nodes, up to the limit number
of records.
The default limit is 100*stream_node_max_entries (used when LIMIT is not
provided).
I.e. this is a behavior change (even if the existing MAXLEN strategy is used).
An explicit limit of 0 means unlimited (but note that it's not the default).
Other changes:
Refactor arg parsing code for XADD and XTRIM to use common code.
Older arm64 Linux kernels have a bug that could lead to data corruption during
background save under the following scenario:
1) jemalloc uses MADV_FREE on a page,
2) jemalloc reuses and writes the page,
3) Redis forks the background save process, and
4) Linux performs page reclamation.
Under these conditions, Linux will reclaim the page wrongfully and the
background save process will read zeros when it tries to read the page.
The bug has been fixed in Linux with commit:
ff1712f953e27f0b0718762ec17d0adb15c9fd0b ("arm64: pgtable: Ensure dirty bit is
preserved across pte_wrprotect()")
This Commit adds an ignore-warnings config, when not found, redis will
print a warning and exit on startup (default behavior).
Co-authored-by: Oran Agra <oran@redislabs.com>
Add INFO field, rdb_active_cow_size, to report COW of a live fork child while
it's active.
- once in 1024 keys check the time, and if there's more than one second since
the last report send a report to the parent via the pipe.
- refactor the child_info_data struct, it's an implementation detail that
shouldn't be in the server struct, and not used to communicate data between
caller and callee
- remove the magic value from that struct (not sure what it was good for), and
instead add handling of short reads.
- add another value to the structure, cow_type, to indicate if the report is
for the new rdb_active_cow_size field, or it's the last report of a
successful operation
- add new Module API to report the active COW
- add more asserts variants to test.tcl
This is a refactory commit, isn't suppose to have any actual impact.
it does the following:
- keep just one server struct fork child pid variable instead of 3
- have one server struct variable indicating the purpose of the current fork
child.
- redisFork is now responsible of updating the server struct with the pid,
which means it can be the one that calls updateDictResizePolicy
- move child info pipe handling into redisFork instead of having them
repeated outside
- there are two classes of fork purposes, mutually exclusive group (AOF, RDB,
Module), and one that can create several forks to coexist in parallel (LDB,
but maybe Modules some day too, Module API allows for that).
- minor fix to killRDBChild:
unlike killAppendOnlyChild and TerminateModuleForkChild, the killRDBChild
doesn't clear the pid variable or call wait4, so checkChildrenDone does
the cleanup for it.
This commit removes the explicit calls to rdbRemoveTempFile, closeChildInfoPipe,
updateDictResizePolicy, which didn't do any harm, but where unnecessary.
Add ZRANGESTORE command, and improve ZSTORE command to deprecated Z[REV]RANGE[BYSCORE|BYLEX].
Syntax for the new ZRANGESTORE command:
ZRANGESTORE [BYSCORE | BYLEX] [REV] [LIMIT offset count]
New syntax for ZRANGE:
ZRANGE [BYSCORE | BYLEX] [REV] [WITHSCORES] [LIMIT offset count]
Old syntax for ZRANGE:
ZRANGE [WITHSCORES]
Other ZRANGE commands remain unchanged.
The implementation uses common code for all of these, by utilizing a consumer interface that in one
command response to the client, and in the other command stores a zset key.
Co-authored-by: Oran Agra <oran@redislabs.com>
New command: XAUTOCLAIM <key> <group> <consumer> <min-idle-time> <start> [COUNT <count>] [JUSTID]
The purpose is to claim entries from a stale consumer without the usual
XPENDING+XCLAIM combo which takes two round trips.
The syntax for XAUTOCLAIM is similar to scan: A cursor is returned (streamID)
by each call and should be used as start for the next call. 0-0 means the scan is complete.
This PR extends the deferred reply mechanism for any bulk string (not just counts)
This PR carries some unrelated test code changes:
- Renames the term "client" into "consumer" in the stream-cgroups test
- And also changes DEBUG SLEEP into "after"
Co-authored-by: Oran Agra <oran@redislabs.com>
instead of asking for the extra new space it wanted, it asked to grow the
string by the size it already has too.
i.e. a string of 1000 bytes, needing to grow by 10 bytes, would have been
asking for an **additional** 1010 bytes.
Turns out the RDB checksum in Redis 6.0 on bigendian is broken.
It always returned 0, so the RDB files are generated as if checksum is
disabled, and will be loaded ok on littleendian, and on bigendian.
But it'll not be able to load RDB files generated on littleendian or older versions.
Similarly DUMP and RESTORE will work on the same version (0==0),
but will be unable to exchange dump payloads with littleendian or old versions.
When a Lua script returns a map to redis (a feature which was added in
redis 6 together with RESP3), it would have returned the value first and
the key second.
If the client was using RESP2, it was getting them out of order, and if
the client was in RESP3, it was getting a map of value => key.
This was happening regardless of the Lua script using redis.setresp(3)
or not.
This also affects a case where the script was returning a map which it got
from from redis by doing something like: redis.setresp(3); return redis.call()
This fix is a breaking change for redis 6.0 users who happened to rely
on the wrong order (either ones that used redis.setresp(3), or ones that
returned a map explicitly).
This commit also includes other two changes in the tests:
1. The test suite now handles RESP3 maps as dicts rather than nested
lists
2. Remove some redundant (duplicate) tests from tracking.tcl
* man-like consistent long formatting
* Uppercases commands, subcommands and options
* Adds 'HELP' to HELP for all
* Lexicographical order
* Uses value notation and other .md likeness
* Moves const char *help to top
* Keeps it under 80 chars
* Misc help typos, consistent conjuctioning (i.e return and not returns)
* Uses addReplySubcommandSyntaxError(c) all over
Signed-off-by: Itamar Haber <itamar@redislabs.com>
This PR not only fixes the problem that swapdb does not make the
transaction fail, but also optimizes the FLUSHALL and FLUSHDB command to
set the CLIENT_DIRTY_CAS flag to avoid unnecessary traversal of clients.
FLUSHDB was changed to first iterate on all watched keys, and then on the
clients watching each key.
Instead of iterating though all clients, and for each iterate on watched keys.
Co-authored-by: Oran Agra <oran@redislabs.com>
If AOF file contains a long Lua script that timed out, then the `evalCommand` calls
`blockingOperationEnds` which sets `server.blocked_last_cron` to 0. later on,
the AOF `whileBlockedCron` function asserts that this value is not 0.
The fix allows nesting call to `blockingOperationStarts` and `blockingOperationEnds`.
The issue was first introduce in this commit: 9ef8d2f67 (Redis 6.2 RC1)
This is a recent problem, introduced by 7471743 (redis 6.0)
The implications are:
The sole difference between LookupKeyRead and LookupKeyWrite is for command
executed on a replica, which are not received from its master client. (for the master,
and for the master client on the replica, these two functions behave the same)!
Since SORT is a write command, this bug only implicates a writable-replica.
And these are its implications:
- SORT STORE will behave as it did before the above mentioned commit (like before
redis 6.0). on a writable-replica an already logically expired the key would have
appeared missing. (store dest key would be deleted, instead of being populated
with the data from the already logically expired key)
- SORT (the non store variant, which in theory could have been executed on
read-only-replica if it weren't for the write flag), will (in redis 6.0) have a new bug
and return the data from the already logically expired key instead of empty response.
New command flags similar to what SADD already has.
Co-authored-by: huangwei03 <huangwei03@kuaishou.com>
Co-authored-by: Itamar Haber <itamar@redislabs.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
In #7726 (part of 6.2), we added a mechanism for whileBlockedCron, this
mechanism has an assertion to make sure the timestamp in
whileBlockedCron was always set correctly before the blocking operation
starts.
I now found (thanks to our CI) two bugs in that area:
1) CONFIG RESETSTAT (if it was allowed during loading) would have
cleared this var
2) the call stopLoading (which calls whileBlockedCron) was made too
early, while the rio is still in use, in which case the update_cksum
(rdbLoadProgressCallback) may still be called and whileBlockedCron
can assert.
getRDB is "designed" to work in two modes: one for redis-cli --rdb and
one for redis-cli --cluster backup.
in the later case it uses the hiredis connection from the cluster nodes
and it used to free it without nullifying the context, so a later
attempt to free the context would crash.
I suppose the reason it seems to want to free the hiredis context ASAP
is that it wants to disconnect the replica link, so that replication
buffers will not be accumulated.
The crash log attempts to print the current client info, and when it
does that it attempts to check if the first argument happens to be a key
but it did so for commands with no arguments too, which caused the crash
log to crash half way and not reach its end.
This Commit pushes forward the observability on overall error statistics and command statistics within redis-server:
It extends INFO COMMANDSTATS to have
- failed_calls in - so we can keep track of errors that happen from the command itself, broken by command.
- rejected_calls - so we can keep track of errors that were triggered outside the commmand processing per se
Adds a new section to INFO, named ERRORSTATS that enables keeping track of the different errors that
occur within redis ( within processCommand and call ) based on the reply Error Prefix ( The first word
after the "-", up to the first space ).
This commit also fixes RM_ReplyWithError so that it can be correctly identified as an error reply.
Recently efaf09ee4 started using addReplyErrorSds in place of
addReplySds the later takes ownership of the string but the former did
not.
This introduced memory leaks when a script returns an error to redis,
and also in clusterRedirectClient (two new usages of
addReplyErrorSds which was mostly unused till now.
This commit chagnes two thanks.
1. change addReplyErrorSds to take ownership of the error string.
2. scripting.c doesn't actually need to use addReplyErrorSds, it's a
perfect match for addReplyErrorFormat (replaces newlines with spaces)
Adds: `L/RPOP <key> [count]`
Implements no. 2 of the following strategies:
1. Loop on listTypePop - this would result in multiple calls for memory freeing and allocating (see 769167a079)
2. Iterate the range to build the reply, then call quickListDelRange - this requires two iterations and **is the current choice**
3. Refactor quicklist to have a pop variant of quickListDelRange - probably optimal but more complex
Also:
* There's a historical check for NULL after calling listTypePop that was converted to an assert.
* This refactors common logic shared between LRANGE and the new form of LPOP/RPOP into addListRangeReply (adds test for b/w compat)
* Consequently, it may have made sense to have `LRANGE l -1 -2` and `LRANGE l 9 0` be legit and return a reverse reply. Due to historical reasons that would be, however, a breaking change.
* Added minimal comments to existing commands to adhere to the style, make core dev life easier and get commit karma, naturally.
The commit deals with the syncWithMaster and the ugly state machine in it.
It attempts to clean it a bit, but more importantly it uses pipeline for
part of the work (rather than 7 round trips, we now have 4).
i.e. the connect and PING are separate, then AUTH + 3 REPLCONF in one pipeline,
and finally the PSYNC (must be separate since the master has to have an empty
output buffer).
This is just a refactoring commit.
This function was never actually used as a synchronous (do both send or
receive), it was always used only ine one of the two modes, which meant it
has to take extra arguments that are not relevant for the other.
Besides that, a tool that sends a synchronous command, it not something
we want in our toolbox (synchronous IO in single threaded app is evil).
sendSynchronousCommand was now refactored into separate sending and
receiving APIs, and the sending part has two variants, one taking vaargs,
and the other taking argc+argv (and an optional length array which means
you can use binary sds strings).
As discussed in https://github.com/antirez/redis/issues/7364, it is good
to have a HELLO command variant, which does not switch the current proto
version of a redis server.
While `HELLO` will work, it introduced a certain difficulty on parsing
options of the command. We will need to offset the index of authentication
and setname option by -1.
So 0 is marked a special version meaning non-switching. And we do not
need to change the code much.
When a database on a 64 bit build grows past 2^31 keys, the underlying hash table expands to 2^32 buckets. After this point, the algorithms for selecting random elements only return elements from half of the available buckets because they use random() which has a range of 0 to 2^31 - 1. This causes problems for eviction policies which use dictGetSomeKeys or dictGetRandomKey. Over time they cause the hash table to become unbalanced because, while new keys are spread out evenly across all buckets, evictions come from only half of the available buckets. Eventually this half of the table starts to run out of keys and it takes longer and longer to find candidates for eviction. This continues until no more evictions can happen.
This solution addresses this by using a 64 bit PRNG instead of libc random().
Co-authored-by: Greg Femec <gfemec@google.com>
Turns out that when the fork child crashes, the crash log was deleting
the pidfile from the disk (although the parent is still running.
Now we set the pidfile of the fork process to NULL so the fork process
will never deletes it.
Normally IO threads should simply read data from the socket into the
buffer and attempt to parse it.
If a protocol error is detected, a reply is generated which may result
with installing a write handler which is not thread safe. This fix
delays that until the client is processed back in the main thread.
Fixes#8220
In the distant history there was only the read flag for commands, and whatever
command that didn't have the read flag was a write one.
Then we added the write flag, but some portions of the code still used !read
Also some commands that don't work on the keyspace at all, still have the read
flag.
Changes in this commit:
1. remove the read-only flag from TIME, ECHO, ROLE and LASTSAVE
2. EXEC command used to decides if it should propagate a MULTI by looking at
the command flags (!read & !admin).
When i was about to change it to look at the write flag instead, i realized
that this would cause it not to propagate a MULTI for PUBLISH, EVAL, and
SCRIPT, all 3 are not marked as either a read command or a write one (as
they should), but all 3 are calling forceCommandPropagation.
So instead of introducing a new flag to denote a command that "writes" but
not into the keyspace, and still needs propagation, i decided to rely on
the forceCommandPropagation, and just fix the code to propagate MULTI when
needed rather than depending on the command flags at all.
The implication of my change then is that now it won't decide to propagate
MULTI when it sees one of these: SELECT, PING, INFO, COMMAND, TIME and
other commands which are neither read nor write.
3. Changing getNodeByQuery and clusterRedirectBlockedClientIfNeeded in
cluster.c to look at !write rather than read flag.
This should have no implications, since these code paths are only reachable
for commands which access keys, and these are always marked as either read
or write.
This commit improve MULTI propagation tests, for modules and a bunch of
other special cases, all of which used to pass already before that commit.
the only one that test change that uncovered a change of behavior is the
one that DELs a non-existing key, it used to propagate an empty
multi-exec block, and no longer does.
In response to large client query buffer optimization introduced in 1898e6c. The calculation of the amount of
remaining bytes we need to write to the query buffer was calculated wrong, as a result we are unnecessarily
growing the client query buffer by sdslen(c->querybuf) always. This fix corrects that behavior.
Please note the previous behavior prior to the before-mentioned change was correctly calculating the remaining
additional bytes, and this change makes that calculate to be consistent.
Useful context, the argument of size `ll` starts at qb_pos (which is now the beginning of the sds), but much of it
may have already been read from the socket, so we only need to grow the sds for the remainder of it.
If we only has one node in cluster or before 8fdc857, we don't know myself ip, so we should use config.hostip for myself.
However, we should use the IP from the command response to update node->ip if it exists and is different from config.hostip
otherwise, when there's more than one node in cluster, if we use -h with virtual IP or DNS, benchmark doesn't show node real ip and port of myself even though it could get right IP and port by CLUSTER NODES command.
Fix wrong server dirty increment in
* spopWithCountCommand
* hsetCommand
* ltrimCommand
* pfaddCommand
Some didn't increment the amount of fields (just one per command).
Others had excessive increments.
Exposes the main thread CPU info via info modules ( linux specific only )
(used_cpu_sys_main_thread and used_cpu_user_main_thread). This is important for:
- distinguish between main thread and io-threads cpu time total cpu time consumed ( check
what is the first bottleneck on the used config )
- distinguish between main thread and modules threads total cpu time consumed
Apart from it, this commit also exposes the server_time_usec within the Server section so that we can
properly differentiate consecutive collection and calculate for example the CPU% and or / cpu time vs
wall time, etc...
* Allow runtest-moduleapi use a different 'make', for systems where GNU Make is 'gmake'.
* Fix issue with builds on Solaris re-building everything from scratch due to CFLAGS/LDFLAGS not stored.
* Fix compile failure on Solaris due to atomicvar and a bunch of warnings.
* Fix garbled log timestamps on Solaris.
When a replica uses the diskless-load swapdb approach, it backs up the old database,
then attempts to load a new one, and in case of failure, it restores the backup.
this means that modules with global out of keyspace data, must have an option to
subscribe to events and backup/restore/discard their global data too.
Add a new set of defrag functions that take a defrag context and allow
defragmenting memory blocks and RedisModuleStrings.
Modules can register a defrag callback which will be invoked when the
defrag process handles globals.
Modules with custom data types can also register a datatype-specific
defrag callback which is invoked for keys that require defragmentation.
The callback and associated functions support both one-step and
multi-step options, depending on the complexity of the key as exposed by
the free_effort callback.
The pid of the benchmark process is used to randomize the random number generator's
seed. This ensures that when multiple benchmark processes are started at the same time
to generate load on a server, they use different seeds. This will ensure randomness in
the keys generated by different benchmark processes.
Add commands to query geospatial data with bounding box.
Two new commands that replace the existing 4 GEORADIUS* commands.
GEOSEARCH key [FROMMEMBER member] [FROMLOC long lat] [BYRADIUS radius
unit] [BYBOX width height unit] [WITHCORD] [WITHDIST] [WITHASH] [COUNT
count] [ASC|DESC]
GEOSEARCHSTORE dest_key src_key [FROMMEMBER member] [FROMLOC long lat]
[BYRADIUS radius unit] [BYBOX width height unit] [WITHCORD] [WITHDIST]
[WITHASH] [COUNT count] [ASC|DESC] [STOREDIST]
- Add two types of CIRCULAR_TYPE and RECTANGLE_TYPE to achieve different searches
- Judge whether the point is within the rectangle, refer to:
geohashGetDistanceIfInRectangle
This adds a new `tls-client-cert-file` and `tls-client-key-file`
configuration directives which make it possible to use different
certificates for the TLS-server and TLS-client functions of Redis.
This is an optional directive. If it is not specified the `tls-cert-file`
and `tls-key-file` directives are used for TLS client functions as well.
Also, `utils/gen-test-certs.sh` now creates additional server-only and client-only certs and will skip intensive operations if target files already exist.
This adds a copy callback for module data types, in order to make
modules compatible with the new COPY command.
The callback is optional and COPY will fail for keys with data types
that do not implement it.
Module blocked clients cache the response in a temporary client,
the reply list in this client would be affected by the recent fix
in #7202, but when the reply is later copied into the real client,
it would have bypassed all the checks for output buffer limit, which
would have resulted in both: responding with a partial response to
the client, and also not disconnecting it at all.
Makes spt_init more careful with assumptions about what memory regions
may be overwritten. It will now only consider a contiguous block of argv
and envp elements and mind any gaps.
when the same consumer re-claim an entry that it already has, there's
no need to remove-and-insert if it's the same rax.
we do need to update the idle time though.
this commit only improves efficiency (doesn't change behavior).
* Add CLIENT INFO subcommand.
The output is identical to CLIENT LIST but provides a single line for
the current client only.
* Add CLIENT LIST ID [id...].
Co-authored-by: Itamar Haber <itamar@redislabs.com>
First, if the ziplist header is surely inside the ziplist, do fast path
decoding rather than the careful one.
In that case, streamline the encoding if-else chain to be executed only
once, and the encoding validity tested at the end.
encourage inlining
likely / unlikely hints for speculative execution
Assertion used _exit(1) to tell the compiler that the code after them is
not reachable and get rid of warnings.
But in some cases assertions are placed inside tight loops, and any
piece of code in them can slow down execution (code cache and other
reasons), instead using either abort() or better yet, unreachable
builtin.
If RESTORE passes successfully with full sanitization, we can't affort
to crash later on assertion due to duplicate records in a hash when
converting it form ziplist to dict.
This means that when doing full sanitization, we must make sure there
are no duplicate records in any of the collections.
The test creates keys with various encodings, DUMP them, corrupt the payload
and RESTORES it.
It utilizes the recently added use-exit-on-panic config to distinguish between
asserts and segfaults.
If the restore succeeds, it runs random commands on the key to attempt to
trigger a crash.
It runs in two modes, one with deep sanitation enabled and one without.
In the first one we don't expect any assertions or segfaults, in the second one
we expect assertions, but no segfaults.
We also check for leaks and invalid reads using valgrind, and if we find them
we print the commands that lead to that issue.
Changes in the code (other than the test):
- Replace a few NPD (null pointer deference) flows and division by zero with an
assertion, so that it doesn't fail the test. (since we set the server to use
`exit` rather than `abort` on assertion).
- Fix quite a lot of flows in rdb.c that could have lead to memory leaks in
RESTORE command (since it now responds with an error rather than panic)
- Add a DEBUG flag for SET-SKIP-CHECKSUM-VALIDATION so that the test don't need
to bother with faking a valid checksum
- Remove a pile of code in serverLogObjectDebugInfo which is actually unsafe to
run in the crash report (see comments in the code)
- fix a missing boundary check in lzf_decompress
test suite infra improvements:
- be able to run valgrind checks before the process terminates
- rotate log files when restarting servers
When loading an encoded payload we will at least do a shallow validation to
check that the size that's encoded in the payload matches the size of the
allocation.
This let's us later use this encoded size to make sure the various offsets
inside encoded payload don't reach outside the allocation, if they do, we'll
assert/panic, but at least we won't segfault or smear memory.
We can also do 'deep' validation which runs on all the records of the encoded
payload and validates that they don't contain invalid offsets. This lets us
detect corruptions early and reject a RESTORE command rather than accepting
it and asserting (crashing) later when accessing that payload via some command.
configuration:
- adding ACL flag skip-sanitize-payload
- adding config sanitize-dump-payload [yes/no/clients]
For now, we don't have a good way to ensure MIGRATE in cluster resharding isn't
being slowed down by these sanitation, so i'm setting the default value to `no`,
but later on it should be set to `clients` by default.
changes:
- changing rdbReportError not to `exit` in RESTORE command
- adding a new stat to be able to later check if cluster MIGRATE isn't being
slowed down by sanitation.
When client tracking is enabled signalModifiedKey can increase memory usage,
this can cause the loop in performEvictions to keep running since it was measuring
the memory usage impact of signalModifiedKey.
The section that measures the memory impact of the eviction should be just on dbDelete,
excluding keyspace notification, client tracking, and propagation to AOF and replicas.
This resolves part of the problem described in #8069
p.s. fix took 1 minute, test took about 3 hours to write.
One way this was happening is when a module issued an RM_Call which would inject MULTI.
If the module command that does that was itself issued by something else that already did
added MULTI (e.g. another module, or a Lua script), it would have caused nested MULTI.
In fact the MULTI state in the client or the MULTI_EMITTED flag in the context isn't
the right indication that we need to propagate MULTI or not, because on a nested calls
(possibly a module action called by a keyspace event of another module action), these
flags aren't retained / reflected.
instead there's now a global propagate_in_transaction flag for that.
in addition to that, we now have a global in_eval and in_exec flags, to serve the flags
of RM_GetContextFlags, since their dependence on the current client is wrong for the same
reasons mentioned above.
As we know, redis may reject user's requests or evict some keys if
used memory is over maxmemory. Dictionaries expanding may make
things worse, some big dictionaries, such as main db and expires dict,
may eat huge memory at once for allocating a new big hash table and be
far more than maxmemory after expanding.
There are related issues: #4213#4583
More details, when expand dict in redis, we will allocate a new big
ht[1] that generally is double of ht[0], The size of ht[1] will be
very big if ht[0] already is big. For db dict, if we have more than
64 million keys, we need to cost 1GB for ht[1] when dict expands.
If the sum of used memory and new hash table of dict needed exceeds
maxmemory, we shouldn't allow the dict to expand. Because, if we
enable keys eviction, we still couldn't add much more keys after
eviction and rehashing, what's worse, redis will keep less keys when
redis only remains a little memory for storing new hash table instead
of users' data. Moreover users can't write data in redis if disable
keys eviction.
What this commit changed ?
Add a new member function expandAllowed for dict type, it provide a way
for caller to allow expand or not. We expose two parameters for this
function: more memory needed for expanding and dict current load factor,
users can implement a function to make a decision by them.
For main db dict and expires dict type, these dictionaries may be very
big and cost huge memory for expanding, so we implement a judgement
function: we can stop dict to expand provisionally if used memory will
be over maxmemory after dict expands, but to guarantee the performance
of redis, we still allow dict to expand if dict load factor exceeds the
safe load factor.
Add test cases to verify we don't allow main db to expand when left
memory is not enough, so that avoid keys eviction.
Other changes:
For new hash table size when expand. Before this commit, the size is
that double used of dict and later _dictNextPower. Actually we aim to
control a dict load factor between 0.5 and 1.0. Now we replace *2 with
+1, since the first check is that used >= size, the outcome of before
will usually be the same as _dictNextPower(used+1). The only case where
it'll differ is when dict_can_resize is false during fork, so that later
the _dictNextPower(used*2) will cause the dict to jump to *4 (i.e.
_dictNextPower(1025*2) will return 4096).
Fix rehash test cases due to changing algorithm of new hash table size
when expand.
In the iterator for these functions, we'll traverse the sorted sets
in a reversed way so that largest elements come first. We prefer
this order because it's optimized for insertion in a skiplist, which
is the destination of the elements being iterated in there functions.
When replica diskless-load type is swapdb in cluster mode, we didn't backup
keys to slots map, so we will lose keys to slots map if fail to sync.
Now we backup keys to slots map at first, and restore it properly when fail.
This commit includes a refactory/cleanup of the backups mechanism (moving it to db.c and re-structuring it a bit).
Co-authored-by: Oran Agra <oran@redislabs.com>
As described in redis-benchamrk help message 'The test names are the same as the ones produced as output.', In redis-benchmark output, we can only see PING_BULK, but the cmd `redis-benchmark -t ping_bulk` is not supported. We have to run it with ping_mbulk which is not user friendly.
SELECT used to read the index into a `long` variable, and then pass it to a function
that takes an `int`, possibly causing an overflow before the range check.
Now all these commands use better and cleaner range check, and that also results in
a slight change of the error response in case of an invalid database index.
SELECT:
in the past it would have returned either `-ERR invalid DB index` (if not a number),
or `-ERR DB index is out of range` (if not between 1..16 or alike).
now it'll return either `-ERR value is out of range` (if not a number), or
`-ERR value is out of range, value must between -2147483648 and 2147483647`
(if not in the range for an int), or `-ERR DB index is out of range`
(if not between 0..16 or alike)
MOVE:
in the past it would only fail with `-ERR index out of range` no matter the reason.
now return the same errors as the new ones for SELECT mentioned above.
(i.e. unlike for SELECT even for a value like 17 we changed the error message)
COPY:
doesn't really matter how it behaved in the past (new command), new behavior is
like the above two.
Fixes#7923.
This PR appropriates the special `&` symbol (because `@` and `*` are taken),
followed by a literal value or pattern for describing the Pub/Sub patterns that
an ACL user can interact with. It is similar to the existing key patterns
mechanism in function (additive) and implementation (copy-pasta). It also adds
the allchannels and resetchannels ACL keywords, naturally.
The default user is given allchannels permissions, whereas new users get
whatever is defined by the acl-pubsub-default configuration directive. For
backward compatibility in 6.2, the default of this directive is allchannels but
this is likely to be changed to resetchannels in the next major version for
stronger default security settings.
Unless allchannels is set for the user, channel access permissions are checked
as follows :
* Calls to both PUBLISH and SUBSCRIBE will fail unless a pattern matching the
argumentative channel name(s) exists for the user.
* Calls to PSUBSCRIBE will fail unless the pattern(s) provided as an argument
literally exist(s) in the user's list.
Such failures are logged to the ACL log.
Runtime changes to channel permissions for a user with existing subscribing
clients cause said clients to disconnect unless the new permissions permit the
connections to continue. Note, however, that PSUBSCRIBErs' patterns are matched
literally, so given the change bar:* -> b*, pattern subscribers to bar:* will be
disconnected.
Notes/questions:
* UNSUBSCRIBE, PUNSUBSCRIBE and PUBSUB remain unprotected due to lack of reasons
for touching them.
when performing the and operation, if the output is 0, we can jump out of the loop.
when performing an or operation, if the output is 0xff, we can jump out of the loop.
this metric already includes the argv bytes, like what clientsCronTrackClientsMemUsage does, but it's missing the array itself.
p.s. For the purpose of tracking expensive clients we don't need to include the size of the client struct and the static reply buffer in it.
Seems to have gone unnoticed for a long time, because at least with
glibc it will only be triggered if setenv() was called before spt_init,
which Redis doesn't.
Fixes#8064.
When USE_SYSTEMD=yes is specified, try to use pkg-config to determine
libsystemd linker flags. If not found, silently fall back to simply
using "-lsystemd".
We now use a LIBSYSTEMD_LIBS variable so users can explicitly override
it and specify their own library.
If USE_SYSTEMD is unspecified the old behavior of auto-enabling it if
both pkg-config and libsystemd are available is retained.
If we enable diskless replication, set repl-diskless-sync-delay to 0,
and master has non-rdb child process such as rewrite aof child, master
will try to start to a new BGSAVE but fails immediately (before fork)
when replicas ask for full synchronization, and master always fails
to start a new BGSAVE and disconnects with replicas until non-rdb
child process exists.
this bug was introduced in #6271 (not yet released in 6.0.x)
This is hopefully usually harmles.
The server.ready_keys will usually be empty so the code after releasing
the GIL will soon be done.
The only case where it'll actually process things is when a module
releases a client (or module) blocked on a key, by triggering this NOT
from within a command (e.g. a timer event).
This bug was introduced in redis 6.0.9, see #7903
Fix: When oom-score-adj-values is provided in the config file after
oom-score-adj yes, it'll take an immediate action, before
readOOMScoreAdj was acquired, resulting in an error (out of range score
due to uninitialized value. delay the reaction the real call is made by
main().
Since the values are clamped to -1000..1000, and they're
applied as an offset from the value at startup (which may be -1000), we
need to allow the offsets to reach to +2000 so that a value of +1000 is
achievable in case the value at startup was -1000.
Adding an option for absolute values rather than relative ones.
The bug was introduced by #5021 which only attempted avoid EXIST on an
already expired key from returning 1 on a replica.
Before that commit, dbExists was used instead of
lookupKeyRead (which had an undesired effect to "touch" the LRU/LFU)
Other than that, this commit fixes OBJECT to also come empty handed on
expired keys in replica.
And DEBUG DIGEST-VALUE to behave like DEBUG OBJECT (get the data from
the key regardless of it's expired state)
Blocking command should not be used with MULTI, LUA, and RM_Call. This is because,
the caller, who executes the command in this context, expects a reply.
Today, LUA and MULTI have a special (and different) treatment to blocking commands:
LUA - Most commands are marked with no-script flag which are checked when executing
and command from LUA, commands that are not marked (like XREAD) verify that their
blocking mode is not used inside LUA (by checking the CLIENT_LUA client flag).
MULTI - Command that is going to block, first verify that the client is not inside
multi (by checking the CLIENT_MULTI client flag). If the client is inside multi, they
return a result which is a match to the empty key with no timeout (for example blpop
inside MULTI will act as lpop)
For modules that perform RM_Call with blocking command, the returned results type is
REDISMODULE_REPLY_UNKNOWN and the caller can not really know what happened.
Disadvantages of the current state are:
No unified approach, LUA, MULTI, and RM_Call, each has a different treatment
Module can not safely execute blocking command (and get reply or error).
Though It is true that modules are not like LUA or MULTI and should be smarter not
to execute blocking commands on RM_Call, sometimes you want to execute a command base
on client input (for example if you create a module that provides a new scripting
language like javascript or python).
While modules (on modules command) can check for REDISMODULE_CTX_FLAGS_LUA or
REDISMODULE_CTX_FLAGS_MULTI to know not to block the client, there is no way to
check if the command came from another module using RM_Call. So there is no way
for a module to know not to block another module RM_Call execution.
This commit adds a way to unify the treatment for blocking clients by introducing
a new CLIENT_DENY_BLOCKING client flag. On LUA, MULTI, and RM_Call the new flag
turned on to signify that the client should not be blocked. A blocking command
verifies that the flag is turned off before blocking. If a blocking command sees
that the CLIENT_DENY_BLOCKING flag is on, it's not blocking and return results
which are matches to empty key with no timeout (as MULTI does today).
The new flag is checked on the following commands:
List blocking commands: BLPOP, BRPOP, BRPOPLPUSH, BLMOVE,
Zset blocking commands: BZPOPMIN, BZPOPMAX
Stream blocking commands: XREAD, XREADGROUP
SUBSCRIBE, PSUBSCRIBE, MONITOR
In addition, the new flag is turned on inside the AOF client, we do not want to
block the AOF client to prevent deadlocks and commands ordering issues (and there
is also an existing assert in the code that verifies it).
To keep backward compatibility on LUA, all the no-script flags on existing commands
were kept untouched. In addition, a LUA special treatment on XREAD and XREADGROUP was kept.
To keep backward compatibility on MULTI (which today allows SUBSCRIBE, and PSUBSCRIBE).
We added a special treatment on those commands to allow executing them on MULTI.
The only backward compatibility issue that this PR introduces is that now MONITOR
is not allowed inside MULTI.
Tests were added to verify blocking commands are not blocking the client on LUA, MULTI,
or RM_Call. Tests were added to verify the module can check for CLIENT_DENY_BLOCKING flag.
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Itamar Haber <itamar@redislabs.com>
ZREVRANGEBYSCORE key max min [WITHSCORES] [LIMIT offset count]
When the offset is too large, the query is very slow. Especially when the offset is greater than the length of zset it is easy to determine whether the offset is greater than the length of zset at first, and If it exceed the length of zset, then return directly.
Co-authored-by: Oran Agra <oran@redislabs.com>
* Configuration file default should now be "auto".
* Expose "process_supervised" as an info field.
* Log messages improvements (clarify required systemd config, report
auto-detected supervision mode, etc.)
* Set server.supervised properly, so it can take precedence of
"daemonize" configuration.
* Produce clear warning if systemd is detected/requested but executable
is compiled without support for it, instead of silently ignoring.
* Handle systemd notification error on startup, and turn off supervised
mode if it failed.
Syntax:
COPY <key> <new-key> [DB <dest-db>] [REPLACE]
No support for module keys yet.
Co-authored-by: tmgauss
Co-authored-by: Itamar Haber <itamar@redislabs.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
Add two optional callbacks to the RedisModuleTypeMethods structure, which is `free_effort`
and `unlink`. the `free_effort` callback indicates the effort required to free a module memory.
Currently, if the effort exceeds LAZYFREE_THRESHOLD, the module memory may be released
asynchronously. the `unlink` callback indicates the key has been removed from the DB by redis, and
may soon be freed by a background thread.
Add `lazyfreed_objects` info field, which represents the number of objects that have been
lazyfreed since redis was started.
Add `RM_GetTypeMethodVersion` API, which return the current redis-server runtime value of
`REDISMODULE_TYPE_METHOD_VERSION`. You can use that when calling `RM_CreateDataType` to know
which fields of RedisModuleTypeMethods are gonna be supported and which will be ignored.
- Add ZDIFF and ZDIFFSTORE which work similarly to SDIFF and SDIFFSTORE
- Make sure the new WITHSCORES argument that was added for ZUNION isn't considered valid for ZUNIONSTORE
Co-authored-by: Oran Agra <oran@redislabs.com>
BLPOP when there are elements in the list works in the same way as LPOP
does. Due to this they also does the same repetitive action and logic
for the same is written at two different places. This is a bad code
practice as the one needs the context to change the BLPOP list pop code
as well when the LPOP code gets changed.
Separated the generic logic from LPOP to a function that is being used
by the BLPOP code as well.
* Debug Populate: Add checks for count and keysize to avoid crash
* provide getRangeLongFromObjectOrReply and getPositiveLongFromObjectOrReply for range check
Expose new `loading_rdb_used_mem` showing the used memory of the server
that saved the RDB file we're currently using.
This is useful in diskless replication when the total size of the rdb is
unkown, and can be used as a rought estimation of progres.
Use that new field to calculate the "user friendly"
`loading_loaded_perc` and `loading_eta_seconds`.
Expose `master_sync_total_bytes` and `master_sync_total_bytes` to complement
on the existing `master_sync_total_bytes` (which cannot be used on its own
to calculate progress).
Add "user friendly" field for `master_sync_perc`
Perform full reset of all client connection states, is if the client was
disconnected and re-connected. This affects:
* MULTI state
* Watched keys
* MONITOR mode
* Pub/Sub subscription
* ACL/Authenticated state
* Client tracking state
* Cluster read-only/asking state
* RESP version (reset to 2)
* Selected database
* CLIENT REPLY state
The response is +RESET to make it easily distinguishable from other
responses.
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Itamar Haber <itamar@redislabs.com>
- rdbSaveSingleModuleAux() used RedisModuleIO's "bytes" field for
tracking written bytes before calling moduleInitIOContext() which sets
"bytes" to zero
- rdbSaveObject() re-initialized RedisModuleIO too late
This return value is not used at the moment since it's only tested
against -1, and the actual byte count isn't used yet.
Co-authored-by: Tomasz Poradowski <tomasz.poradowski@generiscorp.com>
This will allow to use: RedisModule_CreateStringPrintf(ctx, "%s %c %s", "string1", 0, "string2");
On large string, the previous code would incrementally retry to double the output buffer.
now it uses the the return value of snprintf and grows to the right size in one step.
and also avoids an excessive strlen in sdscat at the end.
Make it possible for redis-cli cluster import to work with source and
target that require AUTH.
Adding two different flags --cluster-from-user, --cluster-from-pass
and --cluster-askpass for source node authentication.
Also for target authentication, using existing --user and --pass flag.
Example:
./redis-cli --cluster import 127.0.0.1:7000 --cluster-from 127.0.0.1:6379 --pass 1234 --user default --cluster-from-user default --cluster-from-pass 123456
./redis-cli --cluster import 127.0.0.1:7000 --cluster-from 127.0.0.1:6379 --askpass --cluster-from-user default --cluster-from-askpass
Few config settings are also reflected by the INFO command.
these are mainly ones that are important for either an instant view of
the server status (to compare a metric to it's limit config),
Important configurations that are necessary in the crash log (which
currently doesn't print the config),
And things that are important for monitoring solutions (such as
Prometheus), which rely on INFO to collect their data.
Add cluster_connections to INFO CLUSTER:
This makes it possible to be combined together with connected_clients
and connected_slaves and be matched against maxclients
In redisFork(), we don't set child pid, so updateDictResizePolicy()
doesn't take effect, that isn't friendly for copy-on-write.
The bug was introduced this in redis 6.0: 56258c6
The reason that we want to get a full crash report on SIGABRT
is that the jmalloc, when detecting a corruption, calls abort().
This will cause the Redis to exist silently without any report
and without any way to analyze what happened.
The bug occurs when 'callback' re-registers itself to a point
in the future and the execution time in non-negligible:
'now' refers to time BEFORE callback was executed and is used
to calculate 'next_period'.
We must get the actual current time when calculating 'next_period'
Useful when you want to know through which bind address the client connected to
the server in case of multiple bind addresses.
- Adding `laddr` field to CLIENT list showing the local (bind) address.
- Adding `LADDR` option to CLIENT KILL to kill all the clients connected
to a specific local address.
- Refactoring to share code.
Background:
#3467 (redis 4.0.0), started ignoring ENOPROTOOPT, but did that only for
the default bind (in case bind config wasn't explicitly set).
#5598 (redis 5.0.3), added that for bind addresses explicitly set
(following bug reports in Debian for redis 4.0.9 and 5.0.1), it
also ignored a bunch of other errors like EPROTONOSUPPORT which was
requested in #3894, and also added EADDRNOTAVAIL (wasn't clear why).
This (ignoring EADDRNOTAVAIL) makes redis start successfully, even if a
certain network interface isn't up yet , in which case we rather redis
fail and will be re-tried when the NIC is up, see #7933.
However, it turns out that when IPv6 is disabled (supported but unused),
the error we're getting is EADDRNOTAVAIL. and in many systems the
default config file tries to bind to localhost for both v4 and v6 and
would like to silently ignore the error on v6 if disabled.
This means that we sometimes want to ignore EADDRNOTAVAIL and other times
we wanna fail.
So this commit changes these main things:
1. Ignore all the errors we ignore for both explicitly requested bind
address and a default implicit one.
2. Add a '-' prefix to allow EADDRNOTAVAIL be ignored (by default that's
different than the previous behavior).
3. Restructure that function in a more readable and maintainable way see
below.
4. Make the default behavior of listening to all achievable by setting
a bind config directive to * (previously only possible by omitting
it)
5. document everything.
The old structure of this function was that even if there are no bind
addresses requested, the loop that runs though the bind addresses runs
at least once anyway!
In that one iteration of the loop it binds to both v4 and v6 addresses,
handles errors for each of them separately, and then eventually at the
if-else chain, handles the error of the last bind attempt again!
This was very hard to read and very error prone to maintain, instead now
when the bind info is missing we create one with two entries, and run
the simple loop twice.
Turns out this was broken since version 4.0 when we added sds size
classes.
The cluster code uses sds for the receive buffer, and then casts it to a
struct and accesses a 64 bit variable.
This commit replaces the use of sds with a simple reallocated buffer.
In case redis starts and find that THP is enabled ("always"), instead
of printing a log message, which might go unnoticed, redis will try to
disable it (just for the redis process).
Note: it looks like on self-bulit kernels THP is likely be set to "always" by default.
Some discuss about THP side effect on Linux:
according to http://www.antirez.com/news/84, we can see that
redis latency spikes are caused by linux kernel THP feature.
I have tested on E3-2650 v3, and found that 2M huge page costs
about 0.25ms to fix COW page fault.
Add a new config 'disable-thp', the recommended setting is 'yes',
(default) the redis tries to disable THP by prctl syscall. But
users who really want THP can set it to "no"
Thanks to Oran & Yossi for suggestions.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
This commit deals with manual failover as well as non-manual failover.
We did tests with manual failover as follows:
1, Setup redis cluster which holds 16 partions, each having only
1 corresponding replica.
2, Write a batch of data to redis cluster and make sure the redis is doing
a active expire in serverCron.
3, Do a manual failover sequentially to each partions with a time interval
of 3 minutes.
4, Collect logs and do some computaiton work.
The result:
case avgTime maxTime minTime
C1 95.8ms 227ms 25ms
C2 47.9ms 96ms 12ms
C3 12.6ms 27ms 7ms
Explanation
case C1: All nodes use the version before optimization
case C2: Masters use the elder version while replicas use the optimized version
case C3: All nodes use the optimized version
failover time: The time between when replica got a `manual failover request` and
when it `won the failover election`.
avgTime: average failover time
maxTime: maximum failover time
minTime: mimimum failover time
ms: millisecond
Co-authored-by: chendq8 <c.d_q@163.com>
When using a system with no malloc_usable_size(), zmalloc_size() assumed
that the heap allocator always returns blocks that are long-padded.
This may not always be the case, and will result with zmalloc_size()
returning a size that is bigger than allocated. At least in one case
this leads to out of bound write, process crash and a potential security
vulnerability.
Effectively this does not affect the vast majority of users, who use
jemalloc or glibc.
This problem along with a (different) fix was reported by Drew DeVault.
- add test suite coverage for redis-benchmark
- add --version (similar to what redis-cli has)
- fix bug sending more requests than intended when pipeline > 1.
- when done sending requests, avoid freeing client in the write handler, in theory before
responses are received (probably dead code since the read handler will call clientDone first)
Co-authored-by: Oran Agra <oran@redislabs.com>
`info Persistence` will include correct (updated) rdb_last_bgsave_time_sec
For diskless bgsave (sockets) too (like a few other persistence info fields).
Refactor code to reduce duplicate code.
Previous code would have added default redis save parameters
to the config file on rewrite, which would have been silently ignored
when the config file is loaded.
The new code avoids adding this, and also actively removes these lines
If added by a previous config rewrite.
This wrong behavior was backed by a test, and also documentation, and dates back to 2010.
But it makes no sense to anyone involved so it was decided to change that.
Note that 20eeddf (invalidate watch on expire on access) was released in 6.0 RC2
and 2d1968f released in in 6.0.0 GA (invalidate watch when key is evicted).
both of which do similar changes.
Adding the ":{tag}" only if --cluster is used, so that when used against
a proxy it generates traffic to all shards.
Co-authored-by: Oran Agra <oran@redislabs.com>
minor fix for a bug which happen on error handling code
and doesn't look like it could have caused any real harm
(fd number wouldn't have been reused yet)
This commit implements ACL for Sentinel mode, main work of this PR includes:
- Update Sentinel command table in order to better support ACLs.
- Fix couple of things which currently blocks the support for ACL on sentinel mode.
- Provide "sentinel sentinel-user" and "sentinel sentinel-pass " configuration in order to let sentinel authenticate with a specific user in other sentinels.
- requirepass is kept just for compatibility with old config files
Co-authored-by: Oran Agra <oran@redislabs.com>
In some cases one command added a very big bulk of memory, and this
would be "resolved" by the eviction before the next command.
Seeing an unexplained mass eviction we would wish to
know the highest momentary usage too.
Tracking it in call() and beforeSleep() adds some hooks in AOF and RDB
loading.
The fix in clientsCronTrackExpansiveClients is related to #7874
introduces a NOMKSTREAM option for xadd command, this would be useful for some
use cases when we do not want to create new stream by default:
XADD key [MAXLEN [~|=] <count>] [NOMKSTREAM] <ID or *> [field value] [field value]
Reference the correct "case", case 4, in the comment explaining the need
for case 3, when the number of request items is too close to the
cardinality of the set. Case 4 is indeed the "natural approach"
referenced earlier in that sentence.
This is a compatibility issue with redis 5.0 that was introduced by ACL.
Before this commit, setting requirepass to an empty string will result
in a server that needs an empty AUTH, unlike redis 5.0 which would
accept connections without an AUTH.
If 'delta' is negative 'mem_freed' may underflow and cause
the while loop to exit prematurely (And not evicting enough
memory)
mem_freed can be negative when:
1. We use lazy free (consuming memory by appending to a list)
2. Thread doing an allocation between the two calls to zmalloc_used_memory.
If we fail or stop to rewrite aof, we need to remove temporary aof.
We also remove temporary rdb when replicas abort to receive rdb.
But currently we delete them in main thread, to avoid blocking,
we should use bg_unlink to remove them in a background thread.
Btw, we have already used this way to removed child process temporary rdb.
- Clarify some documentation comments
- Make sure blocked-on-keys client privdata is accessible
from withing the timeout callback
- Handle blocked clients in beforeSleep - In case a key
becomes "ready" outside of processCommand
See #7879#7880
This cleans up and simplifies the API by passing the command name as the
first argument. Previously the command name was specified explicitly,
but was still included in the argv.
* Introduce a new API's: RM_GetContextFlagsAll, and
RM_GetKeyspaceNotificationFlagsAll that will return the
full flags mask of each feature. The module writer can
check base on this value if the Flags he needs are
supported or not.
* For each flag, introduce a new value on redismodule.h,
this value represents the LAST value and should be there
as a reminder to update it when a new value is added,
also it will be used in the code to calculate the full
flags mask (assuming flags are incrementally increasing).
In addition, stated that the module writer should not use
the LAST flag directly and he should use the GetFlagAll API's.
* Introduce a new API: RM_IsSubEventSupported, that returns for a given
event and subevent, whether or not the subevent supported.
* Introduce a new macro RMAPI_FUNC_SUPPORTED(func) that returns whether
or not a function API is supported by comparing it to NULL.
* Introduce a new API: int RM_GetServerVersion();, that will return the
current Redis version in the format 0x00MMmmpp; e.g. 0x00060008;
* Changed unstable version from 999.999.999 to 255.255.255
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
The main motivation here is to provide a way for modules to create a
single, global context that can be used for logging.
Currently, it is possible to obtain a thread-safe context that is not
attached to any blocked client by using `RM_GetThreadSafeContext`.
However, the attached context is not linked to the module identity so
log messages produced are not tagged with the module name.
Ideally we'd fix this in `RM_GetThreadSafeContext` itself but as it
doesn't accept the current context as an argument there's no way to do
that in a backwards compatible manner.
This is essentially the same as calling COMMAND GETKEYS but provides a
more efficient interface that can be used in every context (i.e. not a
Redis command).
Avoid using a static buffer for short key index responses, and make it
caller's responsibility to stack-allocate a result type. Responses that
don't fit are still allocated on the heap.
This allows supplying secret configuration (for example - masterauth) via a secure channel
instead of having it in a plaintext file / command line param, while still allowing for most
of the configuration to reside there.
Also, remove 'special' case handling for --check-rdb which hasn't been relevant
since 4.0.0.
Adding [B]LMOVE <src> <dst> RIGHT|LEFT RIGHT|LEFT. deprecating [B]RPOPLPUSH.
Note that when receiving a BRPOPLPUSH we'll still propagate an RPOPLPUSH,
but on BLMOVE RIGHT LEFT we'll propagate an LMOVE
improvement to existing tests
- Replace "after 1000" with "wait_for_condition" when wait for
clients to block/unblock.
- Add a pre-existing element to target list on basic tests so
that we can check if the new element was added to the correct
side of the list.
- check command stats on the replica to make sure the right
command was replicated
Co-authored-by: Oran Agra <oran@redislabs.com>
I suppose that it was overlooked, since till recently none of the blocked commands were readonly.
other changes:
- add test for the above.
- add better support for additional (and deferring) clients for
cluster tests
- improve a test which left the client in MULTI state.
track and report memory used by clients argv.
this is very usaful in case clients started sending a command and didn't
complete it. in which case the first args of the command are already
trimmed from the query buffer.
in an effort to avoid cache misses and overheads while keeping track of
these, i avoid calling sdsZmallocSize and instead use the sdslen /
bulk-len which can at least give some insight into the problem.
This memory is now added to the total clients memory usage, as well as
the client list.
Adding -D option for redis-cli to control newline between command
responses in raw mode.
Also removing cleanup code before calling exit, just in order
to avoid adding more adding more cleanup code (redis doesn't
bother to release allocations before exit anyway)
Co-authored-by: Oran Agra <oran@redislabs.com>
Add optional GET parameter to SET command in order to set a new value to
a key and retrieve the old key value. With this change we can deprecate
`GETSET` command and use only the SET command with the GET parameter.
This commit has two aspects:
1) improve memory reporting for all the places that use sdsAllocSize to compute
memory used by a string, in this case it'll include the internal fragmentation.
2) reduce the need for realloc calls by making the sds implicitly take over
the internal fragmentation of the block it allocated.
When REDISMODULE_EVENT_CLIENT_CHANGE events are delivered, modules may
want to mutate the client state (e.g. perform authentication).
This change links the module context with the real client rather than a
fake client for these events.
The client pointed to by the module context may in some cases be a fake
client. RM_Authenticate*() calls in this case would be ineffective but
appear to succeed, and this change fails them to make it easier to catch
such cases.
The tls-ca-cert or tls-ca-cert-dir configuration parameters are only
used when Redis needs to authenticate peer certificates, in one of these
scenarios:
1. Incoming clients or replicas, with `tls-auth-clients` enabled.
2. A replica authenticating the master's peer certificate.
3. Cluster nodes authenticating other nodes when establishing the bus
protocol connection.
when slaveof config is "no one", reset any pre-existing config and resume.
also solve a memory leak if slaveof appears twice.
and fail loading if port number is out of range or not an integer.
Co-authored-by: caozhengbin <caozb@yidingyun.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
We may access and modify these two variables in signal handler function,
to guarantee them async-signal-safe, so we should set them to volatile
sig_atomic_t type.
It doesn't look like this could have caused any real issue, and it seems that
signals are handled in main thread on most platforms. But we want to follow C
and POSIX standard in signal handler function.
Make sure we handle short writes correctly, sync to disk after writing and use
rename to make sure the replacement is actually atomic.
In any case of failure old configuration will remain in place.
Also, add some additional logging to make it easier to diagnose rewrite problems.
We should sync temp DB file before renaming as rdb_fsync_range does not use
flag `SYNC_FILE_RANGE_WAIT_AFTER`.
Refer to `Linux Programmer's Manual`:
SYNC_FILE_RANGE_WAIT_AFTER
Wait upon write-out of all pages in the range after performing any write.
When fclose would fail, the previous implementation would have attempted to do fclose again
this can in theory lead to segfault.
other changes:
check for non-zero return value as failure rather than a specific error code.
this doesn't fix a real bug, just a minor cleanup.
Before this commit, we would have continued to add replies to the reply buffer even if client
output buffer limit is reached, so the used memory would keep increasing over the configured limit.
What's more, we shouldn’t write any reply to the client if it is set 'CLIENT_CLOSE_ASAP' flag
because that doesn't conform to its definition and we will close all clients flagged with
'CLIENT_CLOSE_ASAP' in ‘beforeSleep’.
Because of code execution order, before this, we may firstly write to part of the replies to
the socket before disconnecting it, but in fact, we may can’t send the full replies to clients
since OS socket buffer is limited. But this unexpected behavior makes some commands work well,
for instance ACL DELUSER, if the client deletes the current user, we need to send reply to client
and close the connection, but before, we close the client firstly and write the reply to reply
buffer. secondly, we shouldn't do this despite the fact it works well in most cases.
We add a flag 'CLIENT_CLOSE_AFTER_COMMAND' to mark clients, this flag means we will close the
client after executing commands and send all entire replies, so that we can write replies to
reply buffer during executing commands, send replies to clients, and close them later.
We also fix some implicit problems. If client output buffer limit is enforced in 'multi/exec',
all commands will be executed completely in redis and clients will not read any reply instead of
partial replies. Even more, if the client executes 'ACL deluser' the using user in 'multi/exec',
it will not read the replies after 'ACL deluser' just like before executing 'client kill' itself
in 'multi/exec'.
We added some tests for output buffer limit breach during multi-exec and using a pipeline of
many small commands rather than one with big response.
Co-authored-by: Oran Agra <oran@redislabs.com>
