* Support BUILD_TLS=module to be loaded as a module via config file or
command line. e.g. redis-server --loadmodule redis-tls.so
* Updates to redismodule.h to allow it to be used side by side with
server.h by defining REDISMODULE_CORE_MODULE
* Changes to server.h, redismodule.h and module.c to avoid repeated
type declarations (gcc 4.8 doesn't like these)
* Add a mechanism for non-ABI neutral modules (ones who include
server.h) to refuse loading if they detect not being built together with
redis (release.c)
* Fix wrong signature of RedisModuleDefragFunc, this could break
compilation of a module, but not the ABI
* Move initialization of listeners in server.c to be after loading
the modules
* Config TLS after initialization of listeners
* Init cluster after initialization of listeners
* Add TLS module to CI
* Fix a test suite race conditions:
Now that the listeners are initialized later, it's not sufficient to
wait for the PID message in the log, we need to wait for the "Server
Initialized" message.
* Fix issues with moduleconfigs test as a result from start_server
waiting for "Server Initialized"
* Fix issues with modules/infra test as a result of an additional module
present
Notes about Sentinel:
Sentinel can't really rely on the tls module, since it uses hiredis to
initiate connections and depends on OpenSSL (won't be able to use any
other connection modules for that), so it was decided that when TLS is
built as a module, sentinel does not support TLS at all.
This means that it keeps using redis_tls_ctx and redis_tls_client_ctx directly.
Example code of config in redis-tls.so(may be use in the future):
RedisModuleString *tls_cfg = NULL;
void tlsInfo(RedisModuleInfoCtx *ctx, int for_crash_report) {
UNUSED(for_crash_report);
RedisModule_InfoAddSection(ctx, "");
RedisModule_InfoAddFieldLongLong(ctx, "var", 42);
}
int tlsCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
{
if (argc != 2) return RedisModule_WrongArity(ctx);
return RedisModule_ReplyWithString(ctx, argv[1]);
}
RedisModuleString *getStringConfigCommand(const char *name, void *privdata) {
REDISMODULE_NOT_USED(name);
REDISMODULE_NOT_USED(privdata);
return tls_cfg;
}
int setStringConfigCommand(const char *name, RedisModuleString *new, void *privdata, RedisModuleString **err) {
REDISMODULE_NOT_USED(name);
REDISMODULE_NOT_USED(err);
REDISMODULE_NOT_USED(privdata);
if (tls_cfg) RedisModule_FreeString(NULL, tls_cfg);
RedisModule_RetainString(NULL, new);
tls_cfg = new;
return REDISMODULE_OK;
}
int RedisModule_OnLoad(void *ctx, RedisModuleString **argv, int argc)
{
....
if (RedisModule_CreateCommand(ctx,"tls",tlsCommand,"",0,0,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_RegisterStringConfig(ctx, "cfg", "", REDISMODULE_CONFIG_DEFAULT, getStringConfigCommand, setStringConfigCommand, NULL, NULL) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_LoadConfigs(ctx) == REDISMODULE_ERR) {
if (tls_cfg) {
RedisModule_FreeString(ctx, tls_cfg);
tls_cfg = NULL;
}
return REDISMODULE_ERR;
}
...
}
Co-authored-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
A module may be loaded only during initial stage, a typical case is
connection type shared library.
Introduce REDISMODULE_CTX_FLAGS_SERVER_STARTUP context flag
to tell the module the stage of Redis. Then the module gets the flag
by RedisModule_GetContextFlags(ctx), tests flags and returns error in
onload handler.
Suggested-by: Oran Agra <oran@redislabs.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
A module may be loaded only during initial stage, a typical case is
connection type shared library.
Introduce REDISMODULE_CTX_FLAGS_SERVER_STARTUP context flag
to tell the module the stage of Redis. Then the module gets the flag
by RedisModule_GetContextFlags(ctx), tests flags and returns error in
onload handler.
Also introduce 'REDISMODULE_CTX_FLAGS_SENTINEL' context flag to tell
the module the sentinel mode or not.
Suggested-by: Oran Agra <oran@redislabs.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Suggested by Oran, add necessary listeners information in 'INFO'
command. It would be helpful for debug.
Example of this:
127.0.0.1:6379> INFO SERVER
redis_version:255.255.255
...
listener0:name=tcp,bind=127.0.0.1,port=6380
listener1:name=unix,bind=/run/redis.sock
listener2:name=tls,bind=127.0.0.1,port=6379
...
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Introduce listen method into connection type, this allows no hard code
of listen logic. Originally, we initialize server during startup like
this:
if (server.port)
listenToPort(server.port,&server.ipfd);
if (server.tls_port)
listenToPort(server.port,&server.tlsfd);
if (server.unixsocket)
anetUnixServer(...server.unixsocket...);
...
if (createSocketAcceptHandler(&server.ipfd, acceptTcpHandler) != C_OK)
if (createSocketAcceptHandler(&server.tlsfd, acceptTcpHandler) != C_OK)
if (createSocketAcceptHandler(&server.sofd, acceptTcpHandler) != C_OK)
...
If a new connection type gets supported, we have to add more hard code
to setup listener.
Introduce .listen and refactor listener, and Unix socket supports this.
this allows to setup listener arguments and create listener in a loop.
What's more, '.listen' is defined in connection.h, so we should include
server.h to import 'struct socketFds', but server.h has already include
'connection.h'. To avoid including loop(also to make code reasonable),
define 'struct connListener' in connection.h instead of 'struct socketFds'
in server.h. This leads this commit to get more changes.
There are more fields in 'struct connListener', hence it's possible to
simplify changeBindAddr & applyTLSPort() & updatePort() into a single
logic: update the listener config from the server.xxx, and re-create
the listener.
Because of the new field 'priv' in struct connListener, we expect to pass
this to the accept handler(even it's not used currently), this may be used
in the future.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Suggested by Oran, use an array to store all the connection types
instead of a linked list, and use connection name of string. The index
of a connection is dynamically allocated.
Currently we support max 8 connection types, include:
- tcp
- unix socket
- tls
and RDMA is in the plan, then we have another 4 types to support, it
should be enough in a long time.
Introduce 3 functions to get connection type by a fast path:
- connectionTypeTcp()
- connectionTypeTls()
- connectionTypeUnix()
Note that connectionByType() is designed to use only in unlikely code path.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Unix socket uses different accept handler/create listener from TCP,
to hide these difference to avoid hard code, use a new unix socket
connection type. Also move 'acceptUnixHandler' into unix.c.
Currently, the connection framework becomes like following:
uplayer
|
connection layer
/ | \
TCP Unix TLS
It's possible to build Unix socket support as a shared library, and
load it dynamically. Because TCP and Unix socket don't require any
heavy dependencies or overheads, we build them into Redis statically.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Abstract accept handler for socket&TLS, and add helper function
'connAcceptHandler' to get accept handler by specified type.
Also move acceptTcpHandler into socket.c, and move
acceptTLSHandler into tls.c.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
socketFds is also suitable for Unix socket, then we can use
'createSocketAcceptHandler' to create accept handler.
And then, we can abstract accept handler in the future.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Abstract common interface of connection type, so Redis can hide the
implementation and uplayer only calls connection API without macro.
uplayer
|
connection layer
/ \
socket TLS
Currently, for both socket and TLS, all the methods of connection type
are declared as static functions.
It's possible to build TLS(even socket) as a shared library, and Redis
loads it dynamically in the next step.
Also add helper function connTypeOfCluster() and
connTypeOfReplication() to simplify the code:
link->conn = server.tls_cluster ? connCreateTLS() : connCreateSocket();
-> link->conn = connCreate(connTypeOfCluster());
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Introduce .get_peer_cert, .get_ctx and .get_client_ctx for TLS, also
hide redis_tls_ctx & redis_tls_client_ctx.
Then outside could access the variables by connection API only:
- redis_tls_ctx -> connTypeGetCtx(CONN_TYPE_TLS)
- redis_tls_client_ctx -> connTypeGetClientCtx(CONN_TYPE_TLS)
Also remove connTLSGetPeerCert(), use connGetPeerCert() instead.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Introduce .has_pending_data and .process_pending_data for connection
type, and hide tlsHasPendingData() and tlsProcessPendingData(). Also
set .has_pending_data and .process_pending_data as NULL explicitly in
socket.c.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Use connTypeRegister() to register a connection type into redis, and
query connection by connectionByType() via type.
With this change, we can hide TLS specified methods into connection
type:
- void tlsInit(void);
- void tlsCleanup(void);
- int tlsConfigure(redisTLSContextConfig *ctx_config);
- int isTlsConfigured(void);
Merge isTlsConfigured & tlsConfigure, use an argument *reconfigure*
to distinguish:
tlsConfigure(&server.tls_ctx_config)
-> onnTypeConfigure(CONN_TYPE_TLS, &server.tls_ctx_config, 1)
isTlsConfigured() && tlsConfigure(&server.tls_ctx_config)
-> connTypeConfigure(CONN_TYPE_TLS, &server.tls_ctx_config, 0)
Finally, we can remove USE_OPENSSL from config.c. If redis is built
without TLS, and still run redis with TLS, then redis reports:
# Missing implement of connection type 1
# Failed to configure TLS. Check logs for more info.
The log can be optimised, let's leave it in the future. Maybe we can
use connection type as a string.
Although uninitialized fields of a static struct are zero, we still
set them as NULL explicitly in socket.c, let them clear to read & maintain:
.init = NULL,
.cleanup = NULL,
.configure = NULL,
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Originally, connPeerToString is designed to get the address info from
socket only(for both TCP & TLS), and the API 'connPeerToString' is
oriented to operate a FD like:
int connPeerToString(connection *conn, char *ip, size_t ip_len, int *port) {
return anetFdToString(conn ? conn->fd : -1, ip, ip_len, port, FD_TO_PEER_NAME);
}
Introduce connAddr and implement .addr method for socket and TLS,
thus the API 'connAddr' and 'connFormatAddr' become oriented to a
connection like:
static inline int connAddr(connection *conn, char *ip, size_t ip_len, int *port, int remote) {
if (conn && conn->type->addr) {
return conn->type->addr(conn, ip, ip_len, port, remote);
}
return -1;
}
Also remove 'FD_TO_PEER_NAME' & 'FD_TO_SOCK_NAME', use a boolean type
'remote' to get local/remote address of a connection.
With these changes, it's possible to support the other connection
types which does not use socket(Ex, RDMA).
Thanks to Oran for suggestions!
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reorder methods for CT_Socket & CT_TLS, also add comments to make the
methods clear.
Also move the CT_TLS to the end of file, other methods can be static
in the next step.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
These functions are really short enough and they are the connection
functions, separate them from the socket source.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Till now Redis officially supported tuning it via environment variable see #1074.
But we had other requests to allow changing it at runtime, see #799, and #11041.
Note that `strcoll()` is used as Lua comparison function and also for comparison of
certain string objects in Redis, which leads to a problem that, in different regions,
for some characters, the result may be different. Below is an example.
```
127.0.0.1:6333> SORT test alpha
1) "<"
2) ">"
3) ","
4) "*"
127.0.0.1:6333> CONFIG GET locale-collate
1) "locale-collate"
2) ""
127.0.0.1:6333> CONFIG SET locale-collate 1
(error) ERR CONFIG SET failed (possibly related to argument 'locale')
127.0.0.1:6333> CONFIG SET locale-collate C
OK
127.0.0.1:6333> SORT test alpha
1) "*"
2) ","
3) "<"
4) ">"
```
That will cause accidental code compatibility issues for Lua scripts and some
Redis commands. This commit creates a new config parameter to control the
local environment which only affects `Collate` category. Above shows how it
affects `SORT` command, and below shows the influence on Lua scripts.
```
127.0.0.1:6333> CONFIG GET locale-collate
1) " locale-collate"
2) "C"
127.0.0.1:6333> EVAL "return ',' < '*'" 0
(nil)
127.0.0.1:6333> CONFIG SET locale-collate ""
OK
127.0.0.1:6333> EVAL "return ',' < '*'" 0
(integer) 1
```
Co-authored-by: calvincjli <calvincjli@tencent.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
This PR includes 2 missed test cases of XDEL and XGROUP CREATE command
1. one test case: XDEL delete multiply id once
2. 3 test cases: XGROUP CREATE has ENTRIESREAD parameter,
which equal 0 (special positive number), 3 and negative value.
Co-authored-by: Ubuntu <lucas.guang.yang1@huawei.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
This PR makes sure that "name" is unique for all arguments in the same
level (i.e. all args of a command and all args within a block/oneof).
This means several argument with identical meaning can be referred to together,
but also if someone needs to refer to a specific one, they can use its full path.
In addition, the "display_text" field has been added, to be used by redis.io
in order to render the syntax of the command (for the vast majority it is
identical to "name" but sometimes we want to use a different string
that is not "name")
The "display" field is exposed via COMMAND DOCS and will be present
for every argument, except "oneof" and "block" (which are container
arguments)
Other changes:
1. Make sure we do not have any container arguments ("oneof" or "block")
that contain less than two sub-args (otherwise it doesn't make sense)
2. migrate.json: both AUTH and AUTH2 should not be "optional"
3. arg names cannot contain underscores, and force the usage of hyphens
(most of these were a result of the script that generated the initial json files
from redis.io commands.json).
Currently, we call zfree(cmd->args), but the argument array
needs to be freed recursively (there might be sub-args).
Also fixed memory leaks on cmd->tips and cmd->history.
Fixes#11145
Fix replication inconsistency on modules that uses key space notifications.
### The Problem
In general, key space notifications are invoked after the command logic was
executed (this is not always the case, we will discuss later about specific
command that do not follow this rules). For example, the `set x 1` will trigger
a `set` notification that will be invoked after the `set` logic was performed, so
if the notification logic will try to fetch `x`, it will see the new data that was written.
Consider the scenario on which the notification logic performs some write
commands. for example, the notification logic increase some counter,
`incr x{counter}`, indicating how many times `x` was changed.
The logical order by which the logic was executed is has follow:
```
set x 1
incr x{counter}
```
The issue is that the `set x 1` command is added to the replication buffer
at the end of the command invocation (specifically after the key space
notification logic was invoked and performed the `incr` command).
The replication/aof sees the commands in the wrong order:
```
incr x{counter}
set x 1
```
In this specific example the order is less important.
But if, for example, the notification would have deleted `x` then we would
end up with primary-replica inconsistency.
### The Solution
Put the command that cause the notification in its rightful place. In the
above example, the `set x 1` command logic was executed before the
notification logic, so it should be added to the replication buffer before
the commands that is invoked by the notification logic. To achieve this,
without a major code refactoring, we save a placeholder in the replication
buffer, when finishing invoking the command logic we check if the command
need to be replicated, and if it does, we use the placeholder to add it to the
replication buffer instead of appending it to the end.
To be efficient and not allocating memory on each command to save the
placeholder, the replication buffer array was modified to reuse memory
(instead of allocating it each time we want to replicate commands).
Also, to avoid saving a placeholder when not needed, we do it only for
WRITE or MAY_REPLICATE commands.
#### Additional Fixes
* Expire and Eviction notifications:
* Expire/Eviction logical order was to first perform the Expire/Eviction
and then the notification logic. The replication buffer got this in the
other way around (first notification effect and then the `del` command).
The PR fixes this issue.
* The notification effect and the `del` command was not wrap with
`multi-exec` (if needed). The PR also fix this issue.
* SPOP command:
* On spop, the `spop` notification was fired before the command logic
was executed. The change in this PR would have cause the replication
order to be change (first `spop` command and then notification `logic`)
although the logical order is first the notification logic and then the
`spop` logic. The right fix would have been to move the notification to
be fired after the command was executed (like all the other commands),
but this can be considered a breaking change. To overcome this, the PR
keeps the current behavior and changes the `spop` code to keep the right
logical order when pushing commands to the replication buffer. Another PR
will follow to fix the SPOP properly and match it to the other command (we
split it to 2 separate PR's so it will be easy to cherry-pick this PR to 7.0 if
we chose to).
#### Unhanded Known Limitations
* key miss event:
* On key miss event, if a module performed some write command on the
event (using `RM_Call`), the `dirty` counter would increase and the read
command that cause the key miss event would be replicated to the replication
and aof. This problem can also happened on a write command that open
some keys but eventually decides not to perform any action. We decided
not to handle this problem on this PR because the solution is complex
and will cause additional risks in case we will want to cherry-pick this PR.
We should decide if we want to handle it in future PR's. For now, modules
writers is advice not to perform any write commands on key miss event.
#### Testing
* We already have tests to cover cases where a notification is invoking write
commands that are also added to the replication buffer, the tests was modified
to verify that the replica gets the command in the correct logical order.
* Test was added to verify that `spop` behavior was kept unchanged.
* Test was added to verify key miss event behave as expected.
* Test was added to verify the changes do not break lazy expiration.
#### Additional Changes
* `propagateNow` function can accept a special dbid, -1, indicating not
to replicate `select`. We use this to replicate `multi/exec` on `propagatePendingCommands`
function. The side effect of this change is that now the `select` command
will appear inside the `multi/exec` block on the replication stream (instead of
outside of the `multi/exec` block). Tests was modified to match this new behavior.
Make sure the script calls in the tests declare the keys they intend to use.
Do that with minimal changes to existing lines (so many scripts still have a hard coded key names)
Co-authored-by: Valentino Geron <valentino@redis.com>
Before this change in hashTypeSet() function, we first use dictFind()
to look for the field and if it does not exist, we use dictAdd() to add it.
In dictAdd() function the dictionary will look for the field again and I
think this is meaningless as we already know that the field does not exist.
An optimization is to use dictAddRaw() instead of dictFind() and dictAdd().
If we use dictAddRaw(), a new entry will be added when the field does not
exist, and what we should do then is just set the value of that entry, and set
its key to 'sdsdup(field)' in the case that 'HASH_SET_TAKE_FIELD' flag wasn't set.
Fix Lua compile warning on GCC 12.1
GCC 12.1 prints a warning on compile:
```
ldump.c: In function ‘DumpString’:
ldump.c:63:26: warning: the comparison will always evaluate as ‘false’ for the pointer operand in ‘s + 24’ must not be NULL [-Waddress]
63 | if (s==NULL || getstr(s)==NULL)
```
It seems correct, `getstr(s)` can't be `NULL`.
Also, I see Lua v5.2 does not have that check: https://github.com/lua/lua/blob/v5-2/ldump.c#L63
This pr mainly has the following four changes:
1. Add missing lua_pop in `luaGetFromRegistry`.
This bug affects `redis.register_function`, where `luaGetFromRegistry` in
`luaRegisterFunction` will return null when we call `redis.register_function` nested.
.e.g
```
FUNCTION LOAD "#!lua name=mylib \n local lib=redis \n lib.register_function('f2', function(keys, args) lib.register_function('f1', function () end) end)"
fcall f2 0
````
But since we exit when luaGetFromRegistry returns null, it does not cause the stack to grow indefinitely.
3. When getting `REGISTRY_RUN_CTX_NAME` from the registry, use `serverAssert`
instead of error return. Since none of these lua functions are registered at the time
of function load, scriptRunCtx will never be NULL.
4. Add `serverAssert` for `luaLdbLineHook`, `luaEngineLoadHook`.
5. Remove `luaGetFromRegistry` from `redis_math_random` and
`redis_math_randomseed`, it looks like they are redundant.
The divided by two and less <= 10 logics were changed in 06ca9d6839.
Now we just decrement the counter by num_periods.
The lfu-decay-time special value of 0 's meaning was actually changed in 06ca9d6839.
Now we won't do anything on counter if lfu-decay-time is 0.
Fix overflow in redis-benchmark affecting latency measurements on 32bit builds.
If `long` is 4 bytes (typical on 32 bit systems), multiplication overflows.
Using `long long` will fix the issue as it is guaranteed to be at least 8 bytes.
Also, I've added a change to reuse `ustime()` for `mstime()`.
some skip tags were missing on some tests
avoid using HELLO if denytags has resp3 (target server may not support it)
Co-authored-by: Valentino Geron <valentino@redis.com>
`bitfield` with `get` may not be readonly.
```
127.0.0.1:6384> acl setuser hello on nopass %R~* +@all
OK
127.0.0.1:6384> auth hello 1
OK
127.0.0.1:6384> bitfield hello set i8 0 1
(error) NOPERM this user has no permissions to access one of the keys used as arguments
127.0.0.1:6384> bitfield hello set i8 0 1 get i8 0
1) (integer) 0
2) (integer) 1
```
Co-authored-by: Oran Agra <oran@redislabs.com>
This is the history of aof-race related changes:
1. added in 3aa4b00970
2. disabled in dcdfd005a0
3. enabled in 5c63922691
4. disabled in 53a2af3941
This PR refreshes the aof-race test, re-enable it.
Closes#10971
This is an addition to #11039, which cleans up rdbLoad* related errno. Remove the
errno print from the outer message (may be invalid since errno may have been overwritten).
Our aim should be the code that detects the error and knows which system call
triggered it, is the one to print errno, and not the code way up above (in some cases
a result of a logical error and not a system one).
Remove the code to update errno in rdbLoadRioWithLoadingCtx, signature check
and the rdb version check, in these cases, we do print the error message.
The caller dose not have the specific logic for handling EINVAL.
Small fix around rdb-preamble AOF: A truncated RDB is considered a failure,
not handled the same as a truncated AOF file.
Fixing few macros that doesn't follows most basic safety conventions
which is wrapping any usage of passed variable
with parentheses and if written more than one command, then wrap
it with do-while(0) (or parentheses).
* some of the tests don't clean the key the use
* marked tests with `{singledb:skip}` if they use SELECT
Co-authored-by: Valentino Geron <valentino@redis.com>
There is a -Wimplicit-function-declaration warning in here:
```
keyspace_events.c: In function ‘KeySpace_NotificationGeneric’:
keyspace_events.c:67:9: warning: implicit declaration of function ‘usleep’; did you mean ‘sleep’? [-Wimplicit-function-declaration]
67 | usleep(1);
| ^~~~~~
| sleep
```
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.
