2024-03-21 09:30:47 -04:00
|
|
|
# SPDX-FileCopyrightText: 2024 Redict Contributors
|
|
|
|
# SPDX-FileCopyrightText: 2024 Salvatore Sanfilippo <antirez at gmail dot com>
|
|
|
|
#
|
|
|
|
# SPDX-License-Identifier: BSD-3-Clause
|
2024-03-21 15:11:44 -04:00
|
|
|
# SPDX-License-Identifier: LGPL-3.0-only
|
2024-03-21 09:30:47 -04:00
|
|
|
|
2020-09-09 09:01:16 -04:00
|
|
|
set testmodule [file normalize tests/modules/blockedclient.so]
|
|
|
|
|
|
|
|
start_server {tags {"modules"}} {
|
|
|
|
r module load $testmodule
|
|
|
|
|
|
|
|
test {Locked GIL acquisition} {
|
|
|
|
assert_match "OK" [r acquire_gil]
|
|
|
|
}
|
2020-10-11 10:21:58 -04:00
|
|
|
|
|
|
|
test {Locked GIL acquisition during multi} {
|
|
|
|
r multi
|
|
|
|
r acquire_gil
|
|
|
|
assert_equal {{Blocked client is not supported inside multi}} [r exec]
|
|
|
|
}
|
Unified MULTI, LUA, and RM_Call with respect to blocking commands (#8025)
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>
2020-11-17 11:58:55 -05:00
|
|
|
|
|
|
|
test {Locked GIL acquisition from RM_Call} {
|
|
|
|
assert_equal {Blocked client is not allowed} [r do_rm_call acquire_gil]
|
|
|
|
}
|
|
|
|
|
|
|
|
test {Blocking command are not block the client on RM_Call} {
|
|
|
|
r lpush l test
|
|
|
|
assert_equal [r do_rm_call blpop l 0] {l test}
|
|
|
|
|
|
|
|
r lpush l test
|
|
|
|
assert_equal [r do_rm_call brpop l 0] {l test}
|
|
|
|
|
|
|
|
r lpush l1 test
|
|
|
|
assert_equal [r do_rm_call brpoplpush l1 l2 0] {test}
|
|
|
|
assert_equal [r do_rm_call brpop l2 0] {l2 test}
|
|
|
|
|
|
|
|
r lpush l1 test
|
|
|
|
assert_equal [r do_rm_call blmove l1 l2 LEFT LEFT 0] {test}
|
|
|
|
assert_equal [r do_rm_call brpop l2 0] {l2 test}
|
|
|
|
|
|
|
|
r ZADD zset1 0 a 1 b 2 c
|
|
|
|
assert_equal [r do_rm_call bzpopmin zset1 0] {zset1 a 0}
|
|
|
|
assert_equal [r do_rm_call bzpopmax zset1 0] {zset1 c 2}
|
|
|
|
|
|
|
|
r xgroup create s g $ MKSTREAM
|
|
|
|
r xadd s * foo bar
|
|
|
|
assert {[r do_rm_call xread BLOCK 0 STREAMS s 0-0] ne {}}
|
|
|
|
assert {[r do_rm_call xreadgroup group g c BLOCK 0 STREAMS s >] ne {}}
|
|
|
|
|
|
|
|
assert {[r do_rm_call blpop empty_list 0] eq {}}
|
|
|
|
assert {[r do_rm_call brpop empty_list 0] eq {}}
|
|
|
|
assert {[r do_rm_call brpoplpush empty_list1 empty_list2 0] eq {}}
|
|
|
|
assert {[r do_rm_call blmove empty_list1 empty_list2 LEFT LEFT 0] eq {}}
|
|
|
|
|
|
|
|
assert {[r do_rm_call bzpopmin empty_zset 0] eq {}}
|
|
|
|
assert {[r do_rm_call bzpopmax empty_zset 0] eq {}}
|
|
|
|
|
|
|
|
r xgroup create empty_stream g $ MKSTREAM
|
|
|
|
assert {[r do_rm_call xread BLOCK 0 STREAMS empty_stream $] eq {}}
|
|
|
|
assert {[r do_rm_call xreadgroup group g c BLOCK 0 STREAMS empty_stream >] eq {}}
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
test {Monitor disallow inside RM_Call} {
|
|
|
|
set e {}
|
|
|
|
catch {
|
|
|
|
r do_rm_call monitor
|
|
|
|
} e
|
|
|
|
set e
|
Adds pub/sub channel patterns to ACL (#7993)
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.
2020-12-01 07:21:39 -05:00
|
|
|
} {*ERR*DENY BLOCKING*}
|
Unified MULTI, LUA, and RM_Call with respect to blocking commands (#8025)
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>
2020-11-17 11:58:55 -05:00
|
|
|
|
|
|
|
test {subscribe disallow inside RM_Call} {
|
|
|
|
set e {}
|
|
|
|
catch {
|
|
|
|
r do_rm_call subscribe x
|
|
|
|
} e
|
|
|
|
set e
|
Adds pub/sub channel patterns to ACL (#7993)
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.
2020-12-01 07:21:39 -05:00
|
|
|
} {*ERR*DENY BLOCKING*}
|
2020-12-08 09:41:20 -05:00
|
|
|
|
|
|
|
test {RM_Call from blocked client} {
|
|
|
|
r hset hash foo bar
|
|
|
|
r do_bg_rm_call hgetall hash
|
|
|
|
} {foo bar}
|
|
|
|
|
2022-06-21 03:01:13 -04:00
|
|
|
test {RM_Call from blocked client with script mode} {
|
|
|
|
r do_bg_rm_call_format S hset k foo bar
|
|
|
|
} {1}
|
|
|
|
|
|
|
|
test {RM_Call from blocked client with oom mode} {
|
|
|
|
r config set maxmemory 1
|
|
|
|
# will set server.pre_command_oom_state to 1
|
|
|
|
assert_error {OOM command not allowed*} {r hset hash foo bar}
|
|
|
|
r config set maxmemory 0
|
|
|
|
# now its should be OK to call OOM commands
|
|
|
|
r do_bg_rm_call_format M hset k1 foo bar
|
|
|
|
} {1} {needs:config-maxmemory}
|
|
|
|
|
2021-10-21 07:01:10 -04:00
|
|
|
test {RESP version carries through to blocked client} {
|
|
|
|
for {set client_proto 2} {$client_proto <= 3} {incr client_proto} {
|
Add reply_schema to command json files (internal for now) (#10273)
Work in progress towards implementing a reply schema as part of COMMAND DOCS, see #9845
Since ironing the details of the reply schema of each and every command can take a long time, we
would like to merge this PR when the infrastructure is ready, and let this mature in the unstable branch.
Meanwhile the changes of this PR are internal, they are part of the repo, but do not affect the produced build.
### Background
In #9656 we add a lot of information about Redis commands, but we are missing information about the replies
### Motivation
1. Documentation. This is the primary goal.
2. It should be possible, based on the output of COMMAND, to be able to generate client code in typed
languages. In order to do that, we need Redis to tell us, in detail, what each reply looks like.
3. We would like to build a fuzzer that verifies the reply structure (for now we use the existing
testsuite, see the "Testing" section)
### Schema
The idea is to supply some sort of schema for the various replies of each command.
The schema will describe the conceptual structure of the reply (for generated clients), as defined in RESP3.
Note that the reply structure itself may change, depending on the arguments (e.g. `XINFO STREAM`, with
and without the `FULL` modifier)
We decided to use the standard json-schema (see https://json-schema.org/) as the reply-schema.
Example for `BZPOPMIN`:
```
"reply_schema": {
"oneOf": [
{
"description": "Timeout reached and no elements were popped.",
"type": "null"
},
{
"description": "The keyname, popped member, and its score.",
"type": "array",
"minItems": 3,
"maxItems": 3,
"items": [
{
"description": "Keyname",
"type": "string"
},
{
"description": "Member",
"type": "string"
},
{
"description": "Score",
"type": "number"
}
]
}
]
}
```
#### Notes
1. It is ok that some commands' reply structure depends on the arguments and it's the caller's responsibility
to know which is the relevant one. this comes after looking at other request-reply systems like OpenAPI,
where the reply schema can also be oneOf and the caller is responsible to know which schema is the relevant one.
2. The reply schemas will describe RESP3 replies only. even though RESP3 is structured, we want to use reply
schema for documentation (and possibly to create a fuzzer that validates the replies)
3. For documentation, the description field will include an explanation of the scenario in which the reply is sent,
including any relation to arguments. for example, for `ZRANGE`'s two schemas we will need to state that one
is with `WITHSCORES` and the other is without.
4. For documentation, there will be another optional field "notes" in which we will add a short description of
the representation in RESP2, in case it's not trivial (RESP3's `ZRANGE`'s nested array vs. RESP2's flat
array, for example)
Given the above:
1. We can generate the "return" section of all commands in [redis-doc](https://redis.io/commands/)
(given that "description" and "notes" are comprehensive enough)
2. We can generate a client in a strongly typed language (but the return type could be a conceptual
`union` and the caller needs to know which schema is relevant). see the section below for RESP2 support.
3. We can create a fuzzer for RESP3.
### Limitations (because we are using the standard json-schema)
The problem is that Redis' replies are more diverse than what the json format allows. This means that,
when we convert the reply to a json (in order to validate the schema against it), we lose information (see
the "Testing" section below).
The other option would have been to extend the standard json-schema (and json format) to include stuff
like sets, bulk-strings, error-string, etc. but that would mean also extending the schema-validator - and that
seemed like too much work, so we decided to compromise.
Examples:
1. We cannot tell the difference between an "array" and a "set"
2. We cannot tell the difference between simple-string and bulk-string
3. we cannot verify true uniqueness of items in commands like ZRANGE: json-schema doesn't cover the
case of two identical members with different scores (e.g. `[["m1",6],["m1",7]]`) because `uniqueItems`
compares (member,score) tuples and not just the member name.
### Testing
This commit includes some changes inside Redis in order to verify the schemas (existing and future ones)
are indeed correct (i.e. describe the actual response of Redis).
To do that, we added a debugging feature to Redis that causes it to produce a log of all the commands
it executed and their replies.
For that, Redis needs to be compiled with `-DLOG_REQ_RES` and run with
`--reg-res-logfile <file> --client-default-resp 3` (the testsuite already does that if you run it with
`--log-req-res --force-resp3`)
You should run the testsuite with the above args (and `--dont-clean`) in order to make Redis generate
`.reqres` files (same dir as the `stdout` files) which contain request-response pairs.
These files are later on processed by `./utils/req-res-log-validator.py` which does:
1. Goes over req-res files, generated by redis-servers, spawned by the testsuite (see logreqres.c)
2. For each request-response pair, it validates the response against the request's reply_schema
(obtained from the extended COMMAND DOCS)
5. In order to get good coverage of the Redis commands, and all their different replies, we chose to use
the existing redis test suite, rather than attempt to write a fuzzer.
#### Notes about RESP2
1. We will not be able to use the testing tool to verify RESP2 replies (we are ok with that, it's time to
accept RESP3 as the future RESP)
2. Since the majority of the test suite is using RESP2, and we want the server to reply with RESP3
so that we can validate it, we will need to know how to convert the actual reply to the one expected.
- number and boolean are always strings in RESP2 so the conversion is easy
- objects (maps) are always a flat array in RESP2
- others (nested array in RESP3's `ZRANGE` and others) will need some special per-command
handling (so the client will not be totally auto-generated)
Example for ZRANGE:
```
"reply_schema": {
"anyOf": [
{
"description": "A list of member elements",
"type": "array",
"uniqueItems": true,
"items": {
"type": "string"
}
},
{
"description": "Members and their scores. Returned in case `WITHSCORES` was used.",
"notes": "In RESP2 this is returned as a flat array",
"type": "array",
"uniqueItems": true,
"items": {
"type": "array",
"minItems": 2,
"maxItems": 2,
"items": [
{
"description": "Member",
"type": "string"
},
{
"description": "Score",
"type": "number"
}
]
}
}
]
}
```
### Other changes
1. Some tests that behave differently depending on the RESP are now being tested for both RESP,
regardless of the special log-req-res mode ("Pub/Sub PING" for example)
2. Update the history field of CLIENT LIST
3. Added basic tests for commands that were not covered at all by the testsuite
### TODO
- [x] (maybe a different PR) add a "condition" field to anyOf/oneOf schemas that refers to args. e.g.
when `SET` return NULL, the condition is `arguments.get||arguments.condition`, for `OK` the condition
is `!arguments.get`, and for `string` the condition is `arguments.get` - https://github.com/redis/redis/issues/11896
- [x] (maybe a different PR) also run `runtest-cluster` in the req-res logging mode
- [x] add the new tests to GH actions (i.e. compile with `-DLOG_REQ_RES`, run the tests, and run the validator)
- [x] (maybe a different PR) figure out a way to warn about (sub)schemas that are uncovered by the output
of the tests - https://github.com/redis/redis/issues/11897
- [x] (probably a separate PR) add all missing schemas
- [x] check why "SDOWN is triggered by misconfigured instance replying with errors" fails with --log-req-res
- [x] move the response transformers to their own file (run both regular, cluster, and sentinel tests - need to
fight with the tcl including mechanism a bit)
- [x] issue: module API - https://github.com/redis/redis/issues/11898
- [x] (probably a separate PR): improve schemas: add `required` to `object`s - https://github.com/redis/redis/issues/11899
Co-authored-by: Ozan Tezcan <ozantezcan@gmail.com>
Co-authored-by: Hanna Fadida <hanna.fadida@redislabs.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Shaya Potter <shaya@redislabs.com>
2023-03-11 03:14:16 -05:00
|
|
|
if {[lsearch $::denytags "resp3"] >= 0} {
|
|
|
|
if {$client_proto == 3} {continue}
|
|
|
|
} elseif {$::force_resp3} {
|
|
|
|
if {$client_proto == 2} {continue}
|
|
|
|
}
|
2021-10-21 07:01:10 -04:00
|
|
|
r hello $client_proto
|
|
|
|
r readraw 1
|
|
|
|
set ret [r do_fake_bg_true]
|
|
|
|
if {$client_proto == 2} {
|
|
|
|
assert_equal $ret {:1}
|
|
|
|
} else {
|
|
|
|
assert_equal $ret "#t"
|
|
|
|
}
|
|
|
|
r readraw 0
|
Add reply_schema to command json files (internal for now) (#10273)
Work in progress towards implementing a reply schema as part of COMMAND DOCS, see #9845
Since ironing the details of the reply schema of each and every command can take a long time, we
would like to merge this PR when the infrastructure is ready, and let this mature in the unstable branch.
Meanwhile the changes of this PR are internal, they are part of the repo, but do not affect the produced build.
### Background
In #9656 we add a lot of information about Redis commands, but we are missing information about the replies
### Motivation
1. Documentation. This is the primary goal.
2. It should be possible, based on the output of COMMAND, to be able to generate client code in typed
languages. In order to do that, we need Redis to tell us, in detail, what each reply looks like.
3. We would like to build a fuzzer that verifies the reply structure (for now we use the existing
testsuite, see the "Testing" section)
### Schema
The idea is to supply some sort of schema for the various replies of each command.
The schema will describe the conceptual structure of the reply (for generated clients), as defined in RESP3.
Note that the reply structure itself may change, depending on the arguments (e.g. `XINFO STREAM`, with
and without the `FULL` modifier)
We decided to use the standard json-schema (see https://json-schema.org/) as the reply-schema.
Example for `BZPOPMIN`:
```
"reply_schema": {
"oneOf": [
{
"description": "Timeout reached and no elements were popped.",
"type": "null"
},
{
"description": "The keyname, popped member, and its score.",
"type": "array",
"minItems": 3,
"maxItems": 3,
"items": [
{
"description": "Keyname",
"type": "string"
},
{
"description": "Member",
"type": "string"
},
{
"description": "Score",
"type": "number"
}
]
}
]
}
```
#### Notes
1. It is ok that some commands' reply structure depends on the arguments and it's the caller's responsibility
to know which is the relevant one. this comes after looking at other request-reply systems like OpenAPI,
where the reply schema can also be oneOf and the caller is responsible to know which schema is the relevant one.
2. The reply schemas will describe RESP3 replies only. even though RESP3 is structured, we want to use reply
schema for documentation (and possibly to create a fuzzer that validates the replies)
3. For documentation, the description field will include an explanation of the scenario in which the reply is sent,
including any relation to arguments. for example, for `ZRANGE`'s two schemas we will need to state that one
is with `WITHSCORES` and the other is without.
4. For documentation, there will be another optional field "notes" in which we will add a short description of
the representation in RESP2, in case it's not trivial (RESP3's `ZRANGE`'s nested array vs. RESP2's flat
array, for example)
Given the above:
1. We can generate the "return" section of all commands in [redis-doc](https://redis.io/commands/)
(given that "description" and "notes" are comprehensive enough)
2. We can generate a client in a strongly typed language (but the return type could be a conceptual
`union` and the caller needs to know which schema is relevant). see the section below for RESP2 support.
3. We can create a fuzzer for RESP3.
### Limitations (because we are using the standard json-schema)
The problem is that Redis' replies are more diverse than what the json format allows. This means that,
when we convert the reply to a json (in order to validate the schema against it), we lose information (see
the "Testing" section below).
The other option would have been to extend the standard json-schema (and json format) to include stuff
like sets, bulk-strings, error-string, etc. but that would mean also extending the schema-validator - and that
seemed like too much work, so we decided to compromise.
Examples:
1. We cannot tell the difference between an "array" and a "set"
2. We cannot tell the difference between simple-string and bulk-string
3. we cannot verify true uniqueness of items in commands like ZRANGE: json-schema doesn't cover the
case of two identical members with different scores (e.g. `[["m1",6],["m1",7]]`) because `uniqueItems`
compares (member,score) tuples and not just the member name.
### Testing
This commit includes some changes inside Redis in order to verify the schemas (existing and future ones)
are indeed correct (i.e. describe the actual response of Redis).
To do that, we added a debugging feature to Redis that causes it to produce a log of all the commands
it executed and their replies.
For that, Redis needs to be compiled with `-DLOG_REQ_RES` and run with
`--reg-res-logfile <file> --client-default-resp 3` (the testsuite already does that if you run it with
`--log-req-res --force-resp3`)
You should run the testsuite with the above args (and `--dont-clean`) in order to make Redis generate
`.reqres` files (same dir as the `stdout` files) which contain request-response pairs.
These files are later on processed by `./utils/req-res-log-validator.py` which does:
1. Goes over req-res files, generated by redis-servers, spawned by the testsuite (see logreqres.c)
2. For each request-response pair, it validates the response against the request's reply_schema
(obtained from the extended COMMAND DOCS)
5. In order to get good coverage of the Redis commands, and all their different replies, we chose to use
the existing redis test suite, rather than attempt to write a fuzzer.
#### Notes about RESP2
1. We will not be able to use the testing tool to verify RESP2 replies (we are ok with that, it's time to
accept RESP3 as the future RESP)
2. Since the majority of the test suite is using RESP2, and we want the server to reply with RESP3
so that we can validate it, we will need to know how to convert the actual reply to the one expected.
- number and boolean are always strings in RESP2 so the conversion is easy
- objects (maps) are always a flat array in RESP2
- others (nested array in RESP3's `ZRANGE` and others) will need some special per-command
handling (so the client will not be totally auto-generated)
Example for ZRANGE:
```
"reply_schema": {
"anyOf": [
{
"description": "A list of member elements",
"type": "array",
"uniqueItems": true,
"items": {
"type": "string"
}
},
{
"description": "Members and their scores. Returned in case `WITHSCORES` was used.",
"notes": "In RESP2 this is returned as a flat array",
"type": "array",
"uniqueItems": true,
"items": {
"type": "array",
"minItems": 2,
"maxItems": 2,
"items": [
{
"description": "Member",
"type": "string"
},
{
"description": "Score",
"type": "number"
}
]
}
}
]
}
```
### Other changes
1. Some tests that behave differently depending on the RESP are now being tested for both RESP,
regardless of the special log-req-res mode ("Pub/Sub PING" for example)
2. Update the history field of CLIENT LIST
3. Added basic tests for commands that were not covered at all by the testsuite
### TODO
- [x] (maybe a different PR) add a "condition" field to anyOf/oneOf schemas that refers to args. e.g.
when `SET` return NULL, the condition is `arguments.get||arguments.condition`, for `OK` the condition
is `!arguments.get`, and for `string` the condition is `arguments.get` - https://github.com/redis/redis/issues/11896
- [x] (maybe a different PR) also run `runtest-cluster` in the req-res logging mode
- [x] add the new tests to GH actions (i.e. compile with `-DLOG_REQ_RES`, run the tests, and run the validator)
- [x] (maybe a different PR) figure out a way to warn about (sub)schemas that are uncovered by the output
of the tests - https://github.com/redis/redis/issues/11897
- [x] (probably a separate PR) add all missing schemas
- [x] check why "SDOWN is triggered by misconfigured instance replying with errors" fails with --log-req-res
- [x] move the response transformers to their own file (run both regular, cluster, and sentinel tests - need to
fight with the tcl including mechanism a bit)
- [x] issue: module API - https://github.com/redis/redis/issues/11898
- [x] (probably a separate PR): improve schemas: add `required` to `object`s - https://github.com/redis/redis/issues/11899
Co-authored-by: Ozan Tezcan <ozantezcan@gmail.com>
Co-authored-by: Hanna Fadida <hanna.fadida@redislabs.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Shaya Potter <shaya@redislabs.com>
2023-03-11 03:14:16 -05:00
|
|
|
r hello 2
|
2021-10-21 07:01:10 -04:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2022-04-07 04:52:28 -04:00
|
|
|
foreach call_type {nested normal} {
|
|
|
|
test "Busy module command - $call_type" {
|
2022-01-20 02:05:53 -05:00
|
|
|
set busy_time_limit 50
|
|
|
|
set old_time_limit [lindex [r config get busy-reply-threshold] 1]
|
|
|
|
r config set busy-reply-threshold $busy_time_limit
|
2024-03-21 05:56:59 -04:00
|
|
|
set rd [redict_deferring_client]
|
2022-01-20 02:05:53 -05:00
|
|
|
|
|
|
|
# run command that blocks until released
|
|
|
|
set start [clock clicks -milliseconds]
|
2022-04-07 04:52:28 -04:00
|
|
|
if {$call_type == "nested"} {
|
|
|
|
$rd do_rm_call slow_fg_command 0
|
|
|
|
} else {
|
|
|
|
$rd slow_fg_command 0
|
|
|
|
}
|
2022-01-20 02:05:53 -05:00
|
|
|
$rd flush
|
|
|
|
|
2022-04-18 07:56:00 -04:00
|
|
|
# send another command after the blocked one, to make sure we don't attempt to process it
|
|
|
|
$rd ping
|
|
|
|
$rd flush
|
|
|
|
|
2022-01-20 02:05:53 -05:00
|
|
|
# make sure we get BUSY error, and that we didn't get it too early
|
|
|
|
assert_error {*BUSY Slow module operation*} {r ping}
|
|
|
|
assert_morethan_equal [expr [clock clicks -milliseconds]-$start] $busy_time_limit
|
|
|
|
|
|
|
|
# abort the blocking operation
|
|
|
|
r stop_slow_fg_command
|
|
|
|
wait_for_condition 50 100 {
|
|
|
|
[catch {r ping} e] == 0
|
|
|
|
} else {
|
|
|
|
fail "Failed waiting for busy command to end"
|
|
|
|
}
|
2022-04-18 07:56:00 -04:00
|
|
|
assert_equal [$rd read] "1"
|
|
|
|
assert_equal [$rd read] "PONG"
|
2022-01-20 02:05:53 -05:00
|
|
|
|
2022-04-07 04:52:28 -04:00
|
|
|
# run command that blocks for 200ms
|
2022-01-20 02:05:53 -05:00
|
|
|
set start [clock clicks -milliseconds]
|
2022-04-07 04:52:28 -04:00
|
|
|
if {$call_type == "nested"} {
|
|
|
|
$rd do_rm_call slow_fg_command 200000
|
|
|
|
} else {
|
|
|
|
$rd slow_fg_command 200000
|
|
|
|
}
|
2022-01-20 02:05:53 -05:00
|
|
|
$rd flush
|
2024-03-21 05:56:59 -04:00
|
|
|
after 10 ;# try to make sure redict started running the command before we proceed
|
2022-01-20 02:05:53 -05:00
|
|
|
|
|
|
|
# make sure we didn't get BUSY error, it simply blocked till the command was done
|
|
|
|
r ping
|
|
|
|
assert_morethan_equal [expr [clock clicks -milliseconds]-$start] 200
|
|
|
|
$rd read
|
|
|
|
|
|
|
|
$rd close
|
|
|
|
r config set busy-reply-threshold $old_time_limit
|
|
|
|
}
|
2022-04-07 04:52:28 -04:00
|
|
|
}
|
2022-01-20 02:05:53 -05:00
|
|
|
|
|
|
|
test {RM_Call from blocked client} {
|
|
|
|
set busy_time_limit 50
|
|
|
|
set old_time_limit [lindex [r config get busy-reply-threshold] 1]
|
|
|
|
r config set busy-reply-threshold $busy_time_limit
|
|
|
|
|
|
|
|
# trigger slow operation
|
|
|
|
r set_slow_bg_operation 1
|
|
|
|
r hset hash foo bar
|
2024-03-21 05:56:59 -04:00
|
|
|
set rd [redict_deferring_client]
|
2022-01-20 02:05:53 -05:00
|
|
|
set start [clock clicks -milliseconds]
|
|
|
|
$rd do_bg_rm_call hgetall hash
|
|
|
|
|
2022-04-18 07:56:00 -04:00
|
|
|
# send another command after the blocked one, to make sure we don't attempt to process it
|
|
|
|
$rd ping
|
|
|
|
$rd flush
|
|
|
|
|
2022-01-20 02:05:53 -05:00
|
|
|
# wait till we know we're blocked inside the module
|
|
|
|
wait_for_condition 50 100 {
|
|
|
|
[r is_in_slow_bg_operation] eq 1
|
|
|
|
} else {
|
|
|
|
fail "Failed waiting for slow operation to start"
|
|
|
|
}
|
|
|
|
|
|
|
|
# make sure we get BUSY error, and that we didn't get here too early
|
|
|
|
assert_error {*BUSY Slow module operation*} {r ping}
|
2024-02-20 01:43:13 -05:00
|
|
|
assert_morethan_equal [expr [clock clicks -milliseconds]-$start] $busy_time_limit
|
2022-01-20 02:05:53 -05:00
|
|
|
# abort the blocking operation
|
|
|
|
r set_slow_bg_operation 0
|
|
|
|
|
|
|
|
wait_for_condition 50 100 {
|
|
|
|
[r is_in_slow_bg_operation] eq 0
|
|
|
|
} else {
|
|
|
|
fail "Failed waiting for slow operation to stop"
|
|
|
|
}
|
|
|
|
assert_equal [r ping] {PONG}
|
|
|
|
|
|
|
|
r config set busy-reply-threshold $old_time_limit
|
2022-04-18 07:56:00 -04:00
|
|
|
assert_equal [$rd read] {foo bar}
|
|
|
|
assert_equal [$rd read] {PONG}
|
2022-01-20 02:05:53 -05:00
|
|
|
$rd close
|
2022-04-18 07:56:00 -04:00
|
|
|
}
|
2022-01-20 02:05:53 -05:00
|
|
|
|
2020-12-08 09:41:20 -05:00
|
|
|
test {blocked client reaches client output buffer limit} {
|
|
|
|
r hset hash big [string repeat x 50000]
|
|
|
|
r hset hash bada [string repeat x 50000]
|
|
|
|
r hset hash boom [string repeat x 50000]
|
|
|
|
r config set client-output-buffer-limit {normal 100000 0 0}
|
|
|
|
r client setname myclient
|
|
|
|
catch {r do_bg_rm_call hgetall hash} e
|
|
|
|
assert_match "*I/O error*" $e
|
|
|
|
reconnect
|
|
|
|
set clients [r client list]
|
|
|
|
assert_no_match "*name=myclient*" $clients
|
|
|
|
}
|
2022-01-23 03:05:06 -05:00
|
|
|
|
2022-02-13 11:37:32 -05:00
|
|
|
test {module client error stats} {
|
|
|
|
r config resetstat
|
|
|
|
|
2022-02-21 04:20:41 -05:00
|
|
|
# simple module command that replies with string error
|
2022-04-25 06:08:13 -04:00
|
|
|
assert_error "ERR unknown command 'hgetalllll', with args beginning with:" {r do_rm_call hgetalllll}
|
Add new RM_Call flags for script mode, no writes, and error replies. (#10372)
The PR extends RM_Call with 3 new capabilities using new flags that
are given to RM_Call as part of the `fmt` argument.
It aims to assist modules that are getting a list of commands to be
executed from the user (not hard coded as part of the module logic),
think of a module that implements a new scripting language...
* `S` - Run the command in a script mode, this means that it will raise an
error if a command which are not allowed inside a script (flaged with the
`deny-script` flag) is invoked (like SHUTDOWN). In addition, on script mode,
write commands are not allowed if there is not enough good replicas (as
configured with `min-replicas-to-write`) and/or a disk error happened.
* `W` - no writes mode, Redis will reject any command that is marked with `write`
flag. Again can be useful to modules that implement a new scripting language
and wants to prevent any write commands.
* `E` - Return errors as RedisModuleCallReply. Today the errors that happened
before the command was invoked (like unknown commands or acl error) return
a NULL reply and set errno. This might be missing important information about
the failure and it is also impossible to just pass the error to the user using
RM_ReplyWithCallReply. This new flag allows you to get a RedisModuleCallReply
object with the relevant error message and treat it as if it was an error that was
raised by the command invocation.
Tests were added to verify the new code paths.
In addition small refactoring was done to share some code between modules,
scripts, and `processCommand` function:
1. `getAclErrorMessage` was added to `acl.c` to unified to log message extraction
from the acl result
2. `checkGoodReplicasStatus` was added to `replication.c` to check the status of
good replicas. It is used on `scriptVerifyWriteCommandAllow`, `RM_Call`, and
`processCommand`.
3. `writeCommandsGetDiskErrorMessage` was added to `server.c` to get the error
message on persistence failure. Again it is used on `scriptVerifyWriteCommandAllow`,
`RM_Call`, and `processCommand`.
2022-03-22 08:13:28 -04:00
|
|
|
assert_equal [errorrstat ERR r] {count=1}
|
2022-02-13 11:37:32 -05:00
|
|
|
|
2022-04-25 06:08:13 -04:00
|
|
|
# simple module command that replies with string error
|
|
|
|
assert_error "ERR unknown subcommand 'bla'. Try CONFIG HELP." {r do_rm_call config bla}
|
|
|
|
assert_equal [errorrstat ERR r] {count=2}
|
|
|
|
|
2022-02-21 04:20:41 -05:00
|
|
|
# module command that replies with string error from bg thread
|
2022-02-13 11:37:32 -05:00
|
|
|
assert_error "NULL reply returned" {r do_bg_rm_call hgetalllll}
|
Add new RM_Call flags for script mode, no writes, and error replies. (#10372)
The PR extends RM_Call with 3 new capabilities using new flags that
are given to RM_Call as part of the `fmt` argument.
It aims to assist modules that are getting a list of commands to be
executed from the user (not hard coded as part of the module logic),
think of a module that implements a new scripting language...
* `S` - Run the command in a script mode, this means that it will raise an
error if a command which are not allowed inside a script (flaged with the
`deny-script` flag) is invoked (like SHUTDOWN). In addition, on script mode,
write commands are not allowed if there is not enough good replicas (as
configured with `min-replicas-to-write`) and/or a disk error happened.
* `W` - no writes mode, Redis will reject any command that is marked with `write`
flag. Again can be useful to modules that implement a new scripting language
and wants to prevent any write commands.
* `E` - Return errors as RedisModuleCallReply. Today the errors that happened
before the command was invoked (like unknown commands or acl error) return
a NULL reply and set errno. This might be missing important information about
the failure and it is also impossible to just pass the error to the user using
RM_ReplyWithCallReply. This new flag allows you to get a RedisModuleCallReply
object with the relevant error message and treat it as if it was an error that was
raised by the command invocation.
Tests were added to verify the new code paths.
In addition small refactoring was done to share some code between modules,
scripts, and `processCommand` function:
1. `getAclErrorMessage` was added to `acl.c` to unified to log message extraction
from the acl result
2. `checkGoodReplicasStatus` was added to `replication.c` to check the status of
good replicas. It is used on `scriptVerifyWriteCommandAllow`, `RM_Call`, and
`processCommand`.
3. `writeCommandsGetDiskErrorMessage` was added to `server.c` to get the error
message on persistence failure. Again it is used on `scriptVerifyWriteCommandAllow`,
`RM_Call`, and `processCommand`.
2022-03-22 08:13:28 -04:00
|
|
|
assert_equal [errorrstat NULL r] {count=1}
|
2022-02-13 11:37:32 -05:00
|
|
|
|
2022-02-21 04:20:41 -05:00
|
|
|
# module command that returns an arity error
|
2022-02-13 11:37:32 -05:00
|
|
|
r do_rm_call set x x
|
|
|
|
assert_error "ERR wrong number of arguments for 'do_rm_call' command" {r do_rm_call}
|
2022-04-25 06:08:13 -04:00
|
|
|
assert_equal [errorrstat ERR r] {count=3}
|
2022-02-13 11:37:32 -05:00
|
|
|
|
2022-02-21 04:20:41 -05:00
|
|
|
# RM_Call that propagates an error
|
2022-02-13 11:37:32 -05:00
|
|
|
assert_error "WRONGTYPE*" {r do_rm_call hgetall x}
|
|
|
|
assert_equal [errorrstat WRONGTYPE r] {count=1}
|
2022-02-21 04:20:41 -05:00
|
|
|
assert_match {*calls=1,*,rejected_calls=0,failed_calls=1} [cmdrstat hgetall r]
|
2022-02-13 11:37:32 -05:00
|
|
|
|
2022-02-21 04:20:41 -05:00
|
|
|
# RM_Call from bg thread that propagates an error
|
2022-02-13 11:37:32 -05:00
|
|
|
assert_error "WRONGTYPE*" {r do_bg_rm_call hgetall x}
|
|
|
|
assert_equal [errorrstat WRONGTYPE r] {count=2}
|
2022-02-21 04:20:41 -05:00
|
|
|
assert_match {*calls=2,*,rejected_calls=0,failed_calls=2} [cmdrstat hgetall r]
|
|
|
|
|
2022-04-25 06:08:13 -04:00
|
|
|
assert_equal [s total_error_replies] 6
|
|
|
|
assert_match {*calls=5,*,rejected_calls=0,failed_calls=4} [cmdrstat do_rm_call r]
|
2022-02-21 04:20:41 -05:00
|
|
|
assert_match {*calls=2,*,rejected_calls=0,failed_calls=2} [cmdrstat do_bg_rm_call r]
|
2022-02-13 11:37:32 -05:00
|
|
|
}
|
|
|
|
|
2023-05-28 03:10:52 -04:00
|
|
|
set master [srv 0 client]
|
|
|
|
set master_host [srv 0 host]
|
|
|
|
set master_port [srv 0 port]
|
|
|
|
start_server [list overrides [list loadmodule "$testmodule"]] {
|
|
|
|
set replica [srv 0 client]
|
|
|
|
set replica_host [srv 0 host]
|
|
|
|
set replica_port [srv 0 port]
|
|
|
|
|
|
|
|
# Start the replication process...
|
|
|
|
$replica replicaof $master_host $master_port
|
|
|
|
wait_for_sync $replica
|
|
|
|
|
|
|
|
test {WAIT command on module blocked client} {
|
|
|
|
pause_process [srv 0 pid]
|
|
|
|
|
|
|
|
$master do_bg_rm_call_format ! hset bk1 foo bar
|
|
|
|
|
|
|
|
assert_equal [$master wait 1 1000] 0
|
|
|
|
resume_process [srv 0 pid]
|
|
|
|
assert_equal [$master wait 1 1000] 1
|
|
|
|
assert_equal [$replica hget bk1 foo] bar
|
|
|
|
}
|
|
|
|
}
|
Modules: Unblock from within a timer coverage (#12337)
Apart from adding the missing coverage, this PR also adds `blockedBeforeSleep`
that gathers all block-related functions from `beforeSleep`
The order inside `blockedBeforeSleep` is different: now `handleClientsBlockedOnKeys`
(which may unblock clients) is called before `processUnblockedClients` (which handles
unblocked clients).
It makes sense to have this order.
There are no visible effects of the wrong ordering, except some cleanups of the now-unblocked
client would have happen in the next `beforeSleep` (will now happen in the current one)
The reason we even got into it is because i triggers an assertion in logresreq.c (breaking
the assumption that `unblockClient` is called **before** actually flushing the reply to the socket):
`handleClientsBlockedOnKeys` is called, then it calls `moduleUnblockClientOnKey`, which calls
`moduleUnblockClient`, which adds the client to `moduleUnblockedClients` back to `beforeSleep`,
we call `handleClientsWithPendingWritesUsingThreads`, it writes the data of buf to the client, so
`client->bufpos` became 0
On the next `beforeSleep`, we call `moduleHandleBlockedClients`, which calls `unblockClient`,
which calls `reqresAppendResponse`, triggering the assert. (because the `bufpos` is 0) - see https://github.com/redis/redis/pull/12301#discussion_r1226386716
2023-06-22 16:15:16 -04:00
|
|
|
|
|
|
|
test {Unblock by timer} {
|
2024-01-31 02:28:50 -05:00
|
|
|
# When the client is unlock, we will get the OK reply.
|
|
|
|
assert_match "OK" [r unblock_by_timer 100 0]
|
|
|
|
}
|
|
|
|
|
|
|
|
test {block time is shorter than timer period} {
|
|
|
|
# This command does not have the reply.
|
2024-03-21 05:56:59 -04:00
|
|
|
set rd [redict_deferring_client]
|
2024-01-31 02:28:50 -05:00
|
|
|
$rd unblock_by_timer 100 10
|
|
|
|
# Wait for the client to unlock.
|
|
|
|
after 120
|
|
|
|
$rd close
|
Modules: Unblock from within a timer coverage (#12337)
Apart from adding the missing coverage, this PR also adds `blockedBeforeSleep`
that gathers all block-related functions from `beforeSleep`
The order inside `blockedBeforeSleep` is different: now `handleClientsBlockedOnKeys`
(which may unblock clients) is called before `processUnblockedClients` (which handles
unblocked clients).
It makes sense to have this order.
There are no visible effects of the wrong ordering, except some cleanups of the now-unblocked
client would have happen in the next `beforeSleep` (will now happen in the current one)
The reason we even got into it is because i triggers an assertion in logresreq.c (breaking
the assumption that `unblockClient` is called **before** actually flushing the reply to the socket):
`handleClientsBlockedOnKeys` is called, then it calls `moduleUnblockClientOnKey`, which calls
`moduleUnblockClient`, which adds the client to `moduleUnblockedClients` back to `beforeSleep`,
we call `handleClientsWithPendingWritesUsingThreads`, it writes the data of buf to the client, so
`client->bufpos` became 0
On the next `beforeSleep`, we call `moduleHandleBlockedClients`, which calls `unblockClient`,
which calls `reqresAppendResponse`, triggering the assert. (because the `bufpos` is 0) - see https://github.com/redis/redis/pull/12301#discussion_r1226386716
2023-06-22 16:15:16 -04:00
|
|
|
}
|
2024-01-31 06:10:19 -05:00
|
|
|
|
|
|
|
test {block time is equal to timer period} {
|
|
|
|
# These time is equal, they will be unlocked in the same event loop,
|
|
|
|
# when the client is unlock, we will get the OK reply from timer.
|
|
|
|
assert_match "OK" [r unblock_by_timer 100 100]
|
|
|
|
}
|
2023-05-28 03:10:52 -04:00
|
|
|
|
2022-01-23 03:05:06 -05:00
|
|
|
test "Unload the module - blockedclient" {
|
|
|
|
assert_equal {OK} [r module unload blockedclient]
|
|
|
|
}
|
2020-09-09 09:01:16 -04:00
|
|
|
}
|