| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
| |
The docs state that there is a new and an old argument format.
The current state of the arguments allows mixing the old and new format,
thus the need for two additional oneof blocks.
One for differentiating the new from the old format and then one to
allow setting multiple filters using the new format.
|
|
|
|
|
|
|
|
|
|
|
|
| |
plain (#11050)
Before this change, if the module has an embedded string, then uses RedisModule_SaveString
and RedisModule_LoadString, the result would be a raw string instead of an embedded string.
Now the `RDB_LOAD_ENC` flag to `moduleLoadString` only affects integer encoding, but not
embedded strings (which still hold an sds in the robj ptr, so they're actually still raw strings for
anyone who reads them).
Co-authored-by: Valentino Geron <valentino@redis.com>
|
|
|
| |
trackingHandlePendingKeyInvalidations should use proto.
|
|
|
|
|
|
| |
In clusterMsgPingExtForgottenNode, sizeof(name) is CLUSTER_NAMELEN,
and sizeof(clusterMsgPingExtForgottenNode) is > CLUSTER_NAMELEN.
Doing a (name + sizeof(clusterMsgPingExtForgottenNode)) sanitizer
generates an out-of-bounds error which is a false positive in here
|
|
|
|
|
|
|
|
|
|
|
|
| |
RM_Microseconds
Return the wall-clock Unix time, in microseconds
RM_CachedMicroseconds
Returns a cached copy of the Unix time, in microseconds.
It is updated in the server cron job and before executing a command.
It is useful for complex call stacks, such as a command causing a
key space notification, causing a module to execute a RedisModule_Call,
causing another notification, etc.
It makes sense that all these callbacks would use the same clock.
|
|
|
|
|
|
|
|
|
|
| |
The reason we do this is because in #11036, we added error
log message when failing to open RDB file for reading.
In loadDdataFromDisk we call rdbLoad and also check errno,
now the logging corrupts errno (reported in alpine daily).
It is not safe to rely on errno as we do today, so we change
the return value of rdbLoad function to enums, like we have
when loading an AOF.
|
|
|
|
|
|
| |
When FLUSHDB / FLUSHALL / SWAPDB is inside MULTI / EXEC, the
client side tracking invalidation message was interleaved with transaction response.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fix #11030, use lua_rawget to avoid triggering metatables.
#11030 shows how return `_G` from the Lua script (either function or eval), cause the
Lua interpreter to Panic and the Redis processes to exit with error code 1.
Though return `_G` only panic on Redis 7 and 6.2.7, the underline issue exists on older
versions as well (6.0 and 6.2). The underline issue is returning a table with a metatable
such that the metatable raises an error.
The following example demonstrate the issue:
```
127.0.0.1:6379> eval "local a = {}; setmetatable(a,{__index=function() foo() end}) return a" 0
Error: Server closed the connection
```
```
PANIC: unprotected error in call to Lua API (user_script:1: Script attempted to access nonexistent global variable 'foo')
```
The Lua panic happened because when returning the result to the client, Redis needs to
introspect the returning table and transform the table into a resp. In order to scan the table,
Redis uses `lua_gettable` api which might trigger the metatable (if exists) and might raise an error.
This code is not running inside `pcall` (Lua protected call), so raising an error causes the
Lua to panic and exit. Notice that this is not a crash, its a Lua panic that exit with error code 1.
Returning `_G` panics on Redis 7 and 6.2.7 because on those versions `_G` has a metatable
that raises error when trying to fetch a none existing key.
### Solution
Instead of using `lua_gettable` that might raise error and cause the issue, use `lua_rawget`
that simply return the value from the table without triggering any metatable logic.
This is promised not to raise and error.
The downside of this solution is that it might be considered as breaking change, if someone
rely on metatable in the returned value. An alternative solution is to wrap this entire logic
with `pcall` (Lua protected call), this alternative require a much bigger refactoring.
### Back Porting
The same fix will work on older versions as well (6.2, 6.0). Notice that on those version,
the issue can cause Redis to crash if inside the metatable logic there is an attempt to accesses
Redis (`redis.call`). On 7.0, there is not crash and the `redis.call` is executed as if it was done
from inside the script itself.
### Tests
Tests was added the verify the fix
|
|
|
|
|
|
|
|
| |
Gossip the cluster node blacklist in ping and pong messages.
This means that CLUSTER FORGET doesn't need to be sent to all nodes in a cluster.
It can be sent to one or more nodes and then be propagated to the rest of them.
For each blacklisted node, its node id and its remaining blacklist TTL is gossiped in a
cluster bus ping extension (introduced in #9530).
|
|
|
|
| |
When failing to open the rdb file, there was no specific error printed (unlike a
corrupt file), so it was not clear what failed and why.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In rewriteAppendOnlyFileBackground, after flushAppendOnlyFile(1),
and before openNewIncrAofForAppend, we should call redis_fsync
to fsync the aof file.
Because we may open a new INCR AOF in openNewIncrAofForAppend,
in the case of using everysec policy, the old AOF file may not
be fsynced in time (or even at all).
When using everysec, we don't want to pay the disk latency from
the main thread, so we will do a background fsync.
Adding a argument for bioCreateCloseJob, a `need_fsync` flag to
indicate that a fsync is required before the file is closed. So we will
fsync the old AOF file before we close it.
A cleanup, we make union become a union, since the free_* args and
the fd / fsync args are never used together.
Co-authored-by: Oran Agra <oran@redislabs.com>
|
|
|
|
| |
RM_SetAbsExpire and RM_GetAbsExpire were not actually operational since
they were introduced, due to omission in API registration.
|
|
|
|
|
| |
Swap M and N in the complexity formula of [B]ZMPOP
Co-authored-by: Pavel Krush <neon@pushwoosh.com>
|
| |
|
|
|
| |
It seems it was overlooked when we first created the json files
|
|
|
|
|
|
|
|
|
|
| |
As an outstanding part mentioned in #10737, we could just make the cluster config file and
ACL file saving done with a more safe and atomic pattern (write to temp file, fsync, rename, fsync dir).
The cluster config file uses an in-place overwrite and truncation (which was also used by the
main config file before #7824).
The ACL file is using the temp file and rename approach, but was missing an fsync.
Co-authored-by: 朱天 <zhutian03@meituan.com>
|
|
|
|
|
| |
these are missing from the RO_ commands, present in the other ones.
Co-authored-by: Ubuntu <lucas.guang.yang1@huawei.com>
|
|
|
|
|
|
| |
Following #10996, it forgot to modify RM_StringCompare in module.c
Modified RM_StringCompare, compareStringObjectsWithFlags,
compareStringObjects and collateStringObjects.
|
|
|
|
|
|
|
|
|
| |
The temporary array for deleted entries reply of XAUTOCLAIM was
insufficient, but also in fact the COUNT argument should be used to
control the size of the reply, so instead of terminating the loop by
only counting the claimed entries, we'll count deleted entries as well.
Fix #10968
Addresses CVE-2022-31144
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
replace use of:
sprintf --> snprintf
strcpy/strncpy --> redis_strlcpy
strcat/strncat --> redis_strlcat
**why are we making this change?**
Much of the code uses some unsafe variants or deprecated buffer handling
functions.
While most cases are probably not presenting any issue on the known path
programming errors and unterminated strings might lead to potential
buffer overflows which are not covered by tests.
**As part of this PR we change**
1. added implementation for redis_strlcpy and redis_strlcat based on the strl implementation: https://linux.die.net/man/3/strl
2. change all occurrences of use of sprintf with use of snprintf
3. change occurrences of use of strcpy/strncpy with redis_strlcpy
4. change occurrences of use of strcat/strncat with redis_strlcat
5. change the behavior of ll2string/ull2string/ld2string so that it will always place null
termination ('\0') on the output buffer in the first index. this was done in order to make
the use of these functions more safe in cases were the user will not check the output
returned by them (for example in rdbRemoveTempFile)
6. we added a compiler directive to issue a deprecation error in case a use of
sprintf/strcpy/strcat is found during compilation which will result in error during compile time.
However keep in mind that since the deprecation attribute is not supported on all compilers,
this is expected to fail during push workflows.
**NOTE:** while this is only an initial milestone. We might also consider
using the *_s implementation provided by the C11 Extensions (however not
yet widly supported). I would also suggest to start
looking at static code analyzers to track unsafe use cases.
For example LLVM clang checker supports security.insecureAPI.DeprecatedOrUnsafeBufferHandling
which can help locate unsafe function usage.
https://clang.llvm.org/docs/analyzer/checkers.html#security-insecureapi-deprecatedorunsafebufferhandling-c
The main reason not to onboard it at this stage is that the alternative
excepted by clang is to use the C11 extensions which are not always
supported by stdlib.
|
|
|
|
|
|
| |
If we do not use jemalloc (mostly with valgrind) and use an old compiler that does not support C11
we will get compilation error
Co-authored-by: Valentino Geron <valentino@redis.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
According to the Redis functions documentation, FCALL command format could be
FCALL function_name numberOfKeys [key1, key2, key3.....] [arg1, arg2, arg3.....]
So in the json file of fcall and fcall_ro, we should add optional for key and arg part.
Just like EVAL...
Co-authored-by: Binbin <binloveplay1314@qq.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The corrupt dump fuzzer uncovered a valgrind warning saying:
```
==76370== Argument 'size' of function malloc has a fishy (possibly negative) value: -3744781444216323815
```
This allocation would have failed (returning NULL) and being handled properly by redis (even before this change), but we also want to silence the valgrind warnings (which are checking that casting to ssize_t produces a non-negative value).
The solution i opted for is to explicitly fail these allocations (returning NULL), before even reaching `malloc` (which would have failed and return NULL too).
The implication is that we will not be able to support a single allocation of more than 2GB on a 32bit system (which i don't think is a realistic scenario).
i.e. i do think we could be facing cases were redis consumes more than 2gb on a 32bit system, but not in a single allocation.
The byproduct of this, is that i dropped the overflow assertions, since these will now lead to the same OOM panic we have for failed allocations.
|
|
|
| |
Validating inputs ahead of time, to give the end user a slightly more useful error.
|
| |
|
|
|
| |
Remove unnecessary variable name.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In #9389, we add a new `cluster-port` config and make cluster bus port configurable,
and currently redis-cli --cluster create/add-node doesn't support with a configurable `cluster-port` instance.
Because redis-cli uses the old way (port + 10000) to send the `CLUSTER MEET` command.
Now we add this support on redis-cli `--cluster`, note we don't need to explicitly pass in the
`cluster-port` parameter, we can get the real `cluster-port` of the node in `clusterManagerNodeLoadInfo`,
so the `--cluster create` and `--cluster add-node` interfaces have not changed.
We will use the `cluster-port` when we are doing `CLUSTER MEET`, also note that `CLUSTER MEET` bus-port
parameter was added in 4.0, so if the bus_port (the one in redis-cli) is 0, or equal (port + 10000),
we just call `CLUSTER MEET` with 2 arguments, using the old form.
Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>
|
| |
|
|
|
| |
* Fix an engine crash when there are nodes in handshaking and a user calls cluster shards
|
|
|
| |
* Only print ACL syntax errors once and include command names in errors
|
|
|
| |
* Fix some outdated comments and some typo
|
|
|
|
|
|
| |
We already have `pubsub_channels` and `pubsub_patterns`
in INFO stats, now add `pubsubshard_channels` (symmetry).
Sharded pubsub was added in #8621
|
|
|
|
|
| |
Use SSL_shutdown(), in a best-effort manner, when closing a TLS
connection. This change better supports OpenSSL 3.x clients that will
not silently ignore the socket-level EOF.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
## Issue
During the MULTI/EXEC flow, each command gets queued until the `EXEC`
command is received and during this phase on every command queue, a
`realloc` is being invoked. This could be expensive based on the realloc
behavior (if copy to a new memory location).
## Solution
In order to reduce the no. of syscall, couple of optimization I've used.
1. By default, reserve memory for atleast two commands. `MULTI/EXEC` for a
single command doesn't have any significance. Hence, I believe customer wouldn't use it.
2. For further reservation, increase the memory allocation in exponent growth (power of 2).
This reduces the no. of `realloc` call from `N` to `log(N)` times.
## Other changes:
* Include multi exec queued command array in client memory consumption calculation
(affects client eviction too)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently in cluster mode, Redis process locks the cluster config file when
starting up and holds the lock for the entire lifetime of the process.
When the server shuts down, it doesn't explicitly release the lock on the
cluster config file. We noticed a problem with restart testing that if you shut down
a very large redis-server process (i.e. with several hundred GB of data stored),
it takes the OS a while to free the resources and unlock the cluster config file.
So if we immediately try to restart the redis server process, it might fail to acquire
the lock on the cluster config file and fail to come up.
This fix explicitly releases the lock on the cluster config file upon a shutdown rather
than relying on the OS to release the lock, which is a cleaner and safer approach to
free up resources acquired.
|
|
|
|
| |
Account sharded pubsub channels memory consumption in client memory usage
computation to accurately evict client based on the set threshold for `maxmemory-clients`.
|
|
|
|
|
| |
We should also set aof_lastbgrewrite_status to C_ERR on these
errors. Because aof rewrite did fail, and we did not finish the
manifest update. Also maintain the stat_aofrw_consecutive_failures.
|
|
|
| |
This problem was introduced by 496375f and seems to more easily reproduce on macOS since OpenSSL writes more frequently return with EAGAIN.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The `can_log` variable prevents us from outputting too
many error logs. But it should not include the modification
of server.aof_last_write_errno.
We are doing this because:
1. In the short write case, we always set aof_last_write_errno
to ENOSPC, we don't care the `can_log` flag.
2. And we always set aof_last_write_status to C_ERR in aof write
error (except for FSYNC_ALWAYS, we exit). So there may be a chance
that `aof_last_write_errno` is not right.
An innocent bug or just a code cleanup.
|
|
|
|
|
|
|
|
|
|
|
| |
This was harmless because we marked the parent command
with SENTINEL flag. So the populateCommandTable was ok.
And we also don't show the flag (SENTINEL and ONLY-SENTNEL)
in COMMAND INFO.
In this PR, we also add the same CMD_SENTINEL and CMD_ONLY_SENTINEL
flags check when populating the sub-commands.
so that in the future it'll be possible to add some sub-commands to sentinel or sentinel-only but not others.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fix regression of CLUSTER RESET command in redis 7.0.
cluster reset command format is:
CLUSTER RESET [ HARD | SOFT]
According to the cluster reset command doc and codes, the third argument is optional, so
the arity in json file should be -2 instead of 3.
Add test to verify future regressions with RESET and RESET SOFT that were not covered.
Co-authored-by: Ubuntu <lucas.guang.yang1@huawei.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
|
|
|
|
|
|
|
|
| |
When calling CLIENT INFO/LIST, and in various debug prints, Redis is printing
the number of pubsub channels / patterns the client is subscribed to.
With the addition of sharded pubsub, it would be useful to print the number of
keychannels the client is subscribed to as well.
|
| |
|
|
|
|
|
|
|
| |
The module API docs mentions this macro, but it was not defined (so no one could have used it).
Instead of adding it as is, we decided to add a _V1 macro, so that if / when we some day extend this struct,
modules that use this API and don't need the extra fields, will still use the old version
and still be compatible with older redis version (despite being compiled with newer redismodule.h)
|
|
|
|
|
|
|
|
|
|
| |
Since the ranges of `unsigned long long` and `long long` are different, we cannot read an
`unsigned long long` integer from a `RedisModuleString` by `RedisModule_StringToLongLong` .
So I added two new Redis Module APIs to support the conversion between these two types:
* `RedisModule_StringToULongLong`
* `RedisModule_CreateStringFromULongLong`
Signed-off-by: RinChanNOWWW <hzy427@gmail.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
the same arg (#10866)
This commit has two topics.
## Passing config name and value in the same arg
In #10660 (Redis 7.0.1), when we supported the config values that can start with `--` prefix (one of the two topics of that PR),
we broke another pattern: `redis-server redis.config "name value"`, passing both config name
and it's value in the same arg, see #10865
This wasn't a intended change (i.e we didn't realize this pattern used to work).
Although this is a wrong usage, we still like to fix it.
Now we support something like:
```
src/redis-server redis.conf "--maxmemory '700mb'" "--maxmemory-policy volatile-lru" --proc-title-template --my--title--template --loglevel verbose
```
## Changes around --save
Also in this PR, we undo the breaking change we made in #10660 on purpose.
1. `redis-server redis.conf --save --loglevel verbose` (missing `save` argument before anotehr argument).
In 7.0.1, it was throwing an wrong arg error.
Now it will work and reset the save, similar to how it used to be in 7.0.0 and 6.2.x.
3. `redis-server redis.conf --loglevel verbose --save` (missing `save` argument as last argument).
In 6.2, it did not reset the save, which was a bug (inconsistent with the previous bullet).
Now we will make it work and reset the save as well (a bug fix).
|
|
|
| |
Adding Module APIs to let the module read and set the client name of an arbitrary connection.
|