Modules that use background threads with thread safe contexts are likely
to use RM_BlockClient() without a timeout function, because they do not
set up a timeout.
Before this commit, `CLIENT UNBLOCK` would result with a crash as the
`NULL` timeout callback is called. Beyond just crashing, this is also
logically wrong as it may throw the module into an unexpected client
state.
This commits makes `CLIENT UNBLOCK` on such clients behave the same as
any other client that is not in a blocked state and therefore cannot be
unblocked.
For the sdscatfmt function in sds.c, when the parameter fmt ended up with '%',
the behavior is undefined. This commit fix this bug.
Co-authored-by: stafuc <stafuc@gmail.com>
Before this commit, redis-server starts in sentinel mode if the first startup
argument has the string redis-sentinel, so redis also starts in sentinel mode
if the directory it was started from contains the string redis-sentinel.
Now we check the executable name instead of directory.
Some examples:
1. Execute ./redis-sentinel/redis/src/redis-sentinel, starts in sentinel mode.
2. Execute ./redis-sentinel/redis/src/redis-server, starts in server mode,
but before, redis will start in sentinel mode.
3. Execute ./redis-sentinel/redis/src/redis-server --sentinel, of course, like
before, starts in sentinel mode.
This seems to be an unimportant bug that was accidentally generated. If the user does not specify limit in streamParseAddOrTrimArgsOrReply, the initial value of args->limit is 100 * server.stream_node_max_entries, which may lead to out of bounds, and then the default function of limit in xadd becomes invalid (this failure occurs in streamTrim).
Additionally, provide sane default for args->limit in case stream_node_max_entries is set to 0.
Co-authored-by: lizhaolong.lzl <lizhaolong.lzl@B-54MPMD6R-0221.local>
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: guybe7 <guy.benoish@redislabs.com>
A change in redis 6.2 caused redis-cli --rdb that's directed to stdout to fail because fsync fails.
This commit avoids doing ftruncate (fails with a warning) and fsync (fails with an error) when the
output file is `-`, and adds the missing documentation that `-` means stdout.
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Wang Yuan <wangyuancode@163.com>
1. Add one key-value pair to myhash, which the length of key and value both less than hash-max-ziplist-value, for example:
>hset myhash key value
2. Then execute the following command
>hsetnx myhash key value1 (the length greater than hash-max-ziplist-value)
3. This will add nothing, but the code type of "myhash" changed from ziplist to dict even there are only one key-value pair in "myhash", and both of them less than hash-max-ziplist-value.
In the original version, the operation of traversing the stack only seems to
reconstruct the key that does not contain the current node.
But in fact We have got the matched length and splitpos in the key in the
raxlowwalk, so I think we can simplify the logic of this part.
Co-authored-by: lizhaolong.lzl <lizhaolong.lzl@B-54MPMD6R-0221.local>
Return a bad score when used with negative count (or count of 1), and non-ziplist encoded zset.
Also add test to validate the return value and cover the issue.
in the past, the reply list was a list of sds objects, so this didn't have any overhead,
but now addReplySds just copies the data from the sds and frees it, so there's no
need to make a copy of the buffer before copying again.
this reduces an excessive allocation and free and a memcpy.
In the past, the first bind address that was explicitly specified was
also used to bind outgoing connections. This could result with some
problems. For example: on some systems using `bind 127.0.0.1` would
result with outgoing connections also binding to `127.0.0.1` and failing
to connect to remote addresses.
With the recent change to the way `bind` is handled, this presented
other issues:
* The default first bind address is '*' which is not a valid address.
* We make no distinction between user-supplied config that is identical
to the default, and the default config.
This commit addresses both these issues by introducing an explicit
configuration parameter to control the bind address on outgoing
connections.
The call to raxNext didn't really progress in the rax, since we were already on the last item.
instead, all it does is check that it is indeed a valid item, so the new code clearer.
- Introduce a new sdssubstr api as a building block for sdsrange.
The API of sdsrange is many times hard to work with and also has
corner case that cause bugs. sdsrange is easy to work with and also
simplifies the implementation of sdsrange.
- Revert the fix to RM_StringTruncate and just use sdssubstr instead of
sdsrange.
- Solve valgrind warnings from the new tests introduced by the previous
PR.
* Specifying an empty `bind ""` configuration prevents Redis from listening on any TCP port. Before this commit, such configuration was not accepted.
* Using `CONFIG GET bind` will always return an explicit configuration value. Before this commit, if a bind address was not specified the returned value was empty (which was an anomaly).
Another behavior change is that modifying the `bind` configuration to a non-default value will NO LONGER DISABLE protected-mode implicitly.
Previously, passing 0 for newlen would not truncate the string at all.
This adds handling of this case, freeing the old string and creating a new empty string.
Other changes:
- Move `src/modules/testmodule.c` to `tests/modules/basics.c`
- Introduce that basic test into the test suite
- Add tests to cover StringTruncate
- Add `test-modules` build target for the main makefile
- Extend `distclean` build target to clean modules too
The `Tracking gets notification of expired keys` test in tracking.tcl
used to hung in valgrind CI quite a lot.
It turns out the reason is that with valgrind and a busy machine, the
server cron active expire cycle could easily run in the same event loop
as the command that created `mykey`, so that when they key got expired,
there were two change events to broadcast, one that set the key and one
that expired it, but since we used raxTryInsert, the client that was
associated with the "last" change was the one that created the key, so
the NOLOOP filtered that event.
This commit adds a test that reproduces the problem by using lazy expire
in a multi-exec which makes sure the key expires in the same event loop
as the one that added it.
Fixes#6792. Added support of REDIS_REPLY_SET in raw and csv output of `./redis-cli`
Test:
run commands to test:
./redis-cli -3 --csv COMMAND
./redis-cli -3 --raw COMMAND
Now they are returning resuts, were failing with: "Unknown reply type: 10" before the change.
Open the log file only after parsing the entire config file, so that it's
location isn't dependent on the order of configs (`dir` and `logfile`).
Also solves the problem of creating multiple log files if the `logfile`
directive appears many times in the config file.
cleanups:
1: Re-introduce debug leak subcommand in help text.
Mistankenly deleted in https://github.com/redis/redis/pull/5531
2: Formatted the text.
Some text lacks commas resulting in no line breaks.
3: Supplementary debug restart command descriptions of delay arg.
Due to the change in #9003, a long-standing bug was raised under `valgrind`.
This bug can cause the master-slave sync to take a very long time, causing the `pendingquerybuf.tcl` test to fail.
This problem does not only occur in master-slave sync, it is triggered when the big arg is greater than 32k.
step:
```sh
dd if=/dev/zero of=bigfile bs=1M count=32
./src/redis-cli -x hset a a < bigfile
```
1) Make room for querybuf in processMultibulkBuffer, now the alloc of querybuf will be more than 32k.
2) If this happens to trigger the `clientsCronResizeQueryBuffer`, querybuf will be resized to 0.
3) Finally, in readQueryFromClient, we expand the querybuf non-greedily, from 0 to 32k.
Old code, make room for querybuf is greedy, so it only needs 11 times to expand to 32M(16k*(2^11)),
but now we need 2048(32*1024/16) times to reach it, due to the slow allocation under valgrind that exposed the problem.
The fix for the excessive shrinking of the query buf to 0, will be handled in #5013 (that other change on it's own can fix failing test too), but the fix in this PR will also fix the failing test.
The fix in this PR will makes the reading in `readQueryFromClient` more aggressive when working on a big arg (so that it is in par with the same code in `processMultibulkBuffer` (i.e. the two calls to `sdsMakeRoomForNonGreedy` should both use the bulk size).
In the code before this fix the one in readQueryFromClient always has `readlen = PROTO_IOBUF_LEN`
This commit improve MEMORY USAGE command to include internal fragmentation overheads of:
1. EMBSTR encoded strings
2. ziplist encoded zsets and hashes
3. List type nodes
This will allow distros to use an "include conf.d/*.conf" statement in the default configuration file
which will facilitate customization across upgrades/downgrades.
The change itself is trivial: instead of opening an individual file, the glob call creates a vector of files to open, and each file is opened in turn, and its content is added to the configuration.
Gopher support was added mainly because it was simple (trivial to add).
But apparently even something that was trivial at the time, does cause complications
down the line when adding more features.
We recently ran into a few issues with io-threads conflicting with the gopher support.
We had to either complicate the code further in order to solve them, or drop gopher.
AFAIK it's completely unused, so we wanna chuck it, rather than keep supporting it.
Create new module type enhanced callbacks: mem_usage2, free_effort2, unlink2, copy2.
These will be given a context point from which the module can obtain the key name and database id.
In addition the digest and defrag context can now be used to obtain the key name and database id.
When using RESP3, ZPOPMAX/ZPOPMIN should return nested arrays for consistency
with other commands (e.g. ZRANGE).
We do that only when COUNT argument is present (similarly to how LPOP behaves).
for reasoning see https://github.com/redis/redis/issues/8824#issuecomment-855427955
This is a breaking change only when RESP3 is used, and COUNT argument is present!
* Cleaning up the cluster interface by moving almost all related declarations into cluster.h
(no logic change -- just moving declarations/definitions around)
This initial effort leaves two items out of scope - the configuration parsing into the server
struct and the internals exposed by the clusterNode struct.
* Remove unneeded declarations of dictSds*
Ideally all the dictSds functionality would move from server.c into a dedicated module
so we can avoid the duplication in redis-benchmark/cli
* Move crc16 back into server.h, will be moved out once we create a seperate header file for
hashing functions
The initialize memory of `querybuf` is `PROTO_IOBUF_LEN(1024*16) * 2` (due to sdsMakeRoomFor being greedy), under `jemalloc`, the allocated memory will be 40k.
This will most likely result in the `querybuf` being resized when call `clientsCronResizeQueryBuffer` unless the client requests it fast enough.
Note that this bug existed even before #7875, since the condition for resizing includes the sds headers (32k+6).
## Changes
1. Use non-greedy sdsMakeRoomFor when allocating the initial query buffer (of 16k).
1. Also use non-greedy allocation when working with BIG_ARG (we won't use that extra space anyway)
2. in case we did use a greedy allocation, read as much as we can into the buffer we got (including internal frag), to reduce system calls.
3. introduce a dedicated constant for the shrinking (same value as before)
3. Add test for querybuf.
4. improve a maxmemory test by ignoring the effect of replica query buffers (can accumulate many ACKs on slow env)
5. improve a maxmemory by disabling slowlog (it will cause slight memory growth on slow env).
Do not queue command in an already aborted MULTI state.
We can detect an error (watched key).
So in queueMultiCommand, we also can return early.
Like we deal with `CLIENT_DIRTY_EXEC`.
Fix crash when using io-threads-do-reads and issuing CLIENT PAUSE and
CLIENT UNPAUSE.
This issue was introduced in redis 6.2 together with the FAILOVER command.
Today when we load the AOF on startup, the loadAppendOnlyFile checks if
the file is openning for reading.
This check is redundent (dead code) as we open the AOF file for writing at initServer,
and the file will always be existing for the loadAppendOnlyFile.
In this commit:
- remove all the exit(1) from loadAppendOnlyFile, as it is the caller
responsibility to decide what to do in case of failure.
- move the opening of the AOF file for writing, to be after we loading it.
- avoid return -ERR in DEBUG LOADAOF, when the AOF is existing but empty
SINTERSTORE would have deleted the dest key right away,
even when later on it is bound to fail on an (WRONGTYPE) error.
With this change it first picks up all the input keys, and only later
delete the dest key if one is empty.
Also add more tests for some commands.
Mainly focus on
- `wrong type error`:
expand test case (base on sinter bug) in non-store variant
add tests for store variant (although it exists in non-store variant, i think it would be better to have same tests)
- the dstkey result when we meet `non-exist key (empty set)` in *store
sdiff:
- improve test case about wrong type error (the one we found in sinter, although it is safe in sdiff)
- add test about using non-exist key (treat it like an empty set)
sdiffstore:
- according to sdiff test case, also add some tests about `wrong type error` and `non-exist key`
- the different is that in sdiffstore, we will consider the `dstkey` result
sunion/sunionstore add more tests (same as above)
sinter/sinterstore also same as above ...
