fixes test issue introduced in #9167
1. invalid reads due to accessing non-retained string (passed as unblock context).
2. leaking module blocked client context, see #6922 for info.
(cherry picked from commit a8518cce951629eaccde40fd0e51b36a5dc6321c)
fixes test issue introduced in #9167
1. invalid reads due to accessing non-retained string (passed as unblock context).
2. leaking module blocked client context, see #6922 for info.
(cherry picked from commit a8518cce951629eaccde40fd0e51b36a5dc6321c)
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.
(cherry picked from commit aa139e2f02292d668370afde8c91575363c2d611)
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.
(cherry picked from commit aa139e2f02292d668370afde8c91575363c2d611)
In clusterManagerCommandImport strcat was used to concat COPY and
REPLACE, the space maybe not enough.
If we use --cluster-replace but not --cluster-copy, the MIGRATE
command contained COPY instead of REPLACE.
(cherry picked from commit a049f6295a28a20b11eff89083e91dab0738413b)
In clusterManagerCommandImport strcat was used to concat COPY and
REPLACE, the space maybe not enough.
If we use --cluster-replace but not --cluster-copy, the MIGRATE
command contained COPY instead of REPLACE.
(cherry picked from commit a049f6295a28a20b11eff89083e91dab0738413b)
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 ...
(cherry picked from commit b8a5da80c49501773f8778aaf5cbf595cef615e4)
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 ...
(cherry picked from commit b8a5da80c49501773f8778aaf5cbf595cef615e4)
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!
(cherry picked from commit 7f342020dcbdf9abe754d6b666efdeded7063870)
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!
(cherry picked from commit 7f342020dcbdf9abe754d6b666efdeded7063870)
1. redis-cli can output --rdb data to stdout
but redis-cli also write some messages to stdout which will mess up the rdb.
2. Make redis-cli flush stdout when printing a reply
This was needed in order to fix a hung in redis-cli test that uses
--replica.
Note that printf does flush when there's a newline, but fwrite does not.
3. fix the redis-cli --replica test which used to pass previously
because it didn't really care what it read, and because redis-cli
used printf to print these other things to stdout.
4. improve redis-cli --replica test to run with both diskless and disk-based.
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Viktor Söderqvist <viktor@zuiderkwast.se>
(cherry picked from commit 1eb4baa5b8e76adc337ae9fab49acc2585a0cdd0)
1. redis-cli can output --rdb data to stdout
but redis-cli also write some messages to stdout which will mess up the rdb.
2. Make redis-cli flush stdout when printing a reply
This was needed in order to fix a hung in redis-cli test that uses
--replica.
Note that printf does flush when there's a newline, but fwrite does not.
3. fix the redis-cli --replica test which used to pass previously
because it didn't really care what it read, and because redis-cli
used printf to print these other things to stdout.
4. improve redis-cli --replica test to run with both diskless and disk-based.
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Viktor Söderqvist <viktor@zuiderkwast.se>
(cherry picked from commit 1eb4baa5b8e76adc337ae9fab49acc2585a0cdd0)
- promote the code in DEBUG PROTOCOL to addReplyBigNum
- DEBUG PROTOCOL ATTRIB skips the attribute when client is RESP2
- networking.c addReply for push and attributes generate assertion when
called on a RESP2 client, anything else would produce a broken
protocol that clients can't handle.
(cherry picked from commit 6a5bac309e868deef749c36949723b415de2496f)
- promote the code in DEBUG PROTOCOL to addReplyBigNum
- DEBUG PROTOCOL ATTRIB skips the attribute when client is RESP2
- networking.c addReply for push and attributes generate assertion when
called on a RESP2 client, anything else would produce a broken
protocol that clients can't handle.
(cherry picked from commit 6a5bac309e868deef749c36949723b415de2496f)
due to a copy-paste bug, it used to reply with null response rather than empty array.
this commit includes new tests that are looking at the RESP response directly in
order to be able to tell the difference between them.
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit a418a2d3fc0250c094802d7e8ea64d96eedfda07)
due to a copy-paste bug, it used to reply with null response rather than empty array.
this commit includes new tests that are looking at the RESP response directly in
order to be able to tell the difference between them.
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit a418a2d3fc0250c094802d7e8ea64d96eedfda07)
This makes it possible to distinguish between null response and an empty
array (currently the tests infra translates both to an empty string/list)
(cherry picked from commit 7103367ad44b4241e59a709771cb464aa2a86b20)
This makes it possible to distinguish between null response and an empty
array (currently the tests infra translates both to an empty string/list)
(cherry picked from commit 7103367ad44b4241e59a709771cb464aa2a86b20)
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>
(cherry picked from commit 74fe15b3602ed7c003b5c53e45e31f7aa6d4a86f)
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>
(cherry picked from commit 74fe15b3602ed7c003b5c53e45e31f7aa6d4a86f)
mistakenly it used to return an empty array rather than 0.
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 95274f1f8a3ef4cb4033beecfaa99ea1439ed170)
mistakenly it used to return an empty array rather than 0.
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 95274f1f8a3ef4cb4033beecfaa99ea1439ed170)
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.
(cherry picked from commit 4bc5a8324d3cb23ed2cc8a9cd19444a893a6d52c)
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.
(cherry picked from commit 4bc5a8324d3cb23ed2cc8a9cd19444a893a6d52c)
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
(cherry picked from commit 1ccf2ca2f475a161b8ca0c574d4c0e6ef9ecf754)
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
(cherry picked from commit 1ccf2ca2f475a161b8ca0c574d4c0e6ef9ecf754)
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.
(cherry picked from commit 096c5fd5d22caa2c21a4863cbaaf64fd8e7107d2)
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.
(cherry picked from commit 096c5fd5d22caa2c21a4863cbaaf64fd8e7107d2)
sometimes you can be very deep in the call stack, without access to argv.
once you're there you may want your reply/log to contain the command name.
(cherry picked from commit e16d3eb998f7017c95ef17179de77aa5b6f2e272)
sometimes you can be very deep in the call stack, without access to argv.
once you're there you may want your reply/log to contain the command name.
(cherry picked from commit e16d3eb998f7017c95ef17179de77aa5b6f2e272)
The decision to stop trimming due to LIMIT in XADD and XTRIM was after the limit was reached.
i.e. the code was deleting **at least** that count of records (from the LIMIT argument's perspective, not the MAXLEN),
instead of **up to** that count of records.
see #9046
(cherry picked from commit eaa7a7bb93c1ac6dbf347e1c29ac719a12a75158)
The decision to stop trimming due to LIMIT in XADD and XTRIM was after the limit was reached.
i.e. the code was deleting **at least** that count of records (from the LIMIT argument's perspective, not the MAXLEN),
instead of **up to** that count of records.
see #9046
(cherry picked from commit eaa7a7bb93c1ac6dbf347e1c29ac719a12a75158)
xtrimCommand call streamParseAddOrTrimArgsOrReply should use xadd==0.
When the syntax is valid, it does not cause any bugs because the params of XADD is superset of XTRIM.
Just XTRIM will not respond with error on invalid syntax. The syntax of XADD will also be accpeted by XTRIM.
(cherry picked from commit 91f3689bf5dc4ce3cf00d9d957b9677b362a205e)
xtrimCommand call streamParseAddOrTrimArgsOrReply should use xadd==0.
When the syntax is valid, it does not cause any bugs because the params of XADD is superset of XTRIM.
Just XTRIM will not respond with error on invalid syntax. The syntax of XADD will also be accpeted by XTRIM.
(cherry picked from commit 91f3689bf5dc4ce3cf00d9d957b9677b362a205e)
When test stop 'load handler' by killing the process that generating the load,
some commands that already in the input buffer, still might be processed by the server.
This may cause some instability in tests, that count on that no more commands
processed after we stop the `load handler'
In this commit, new proc 'wait_load_handlers_disconnected' added, to verify that no more
cammands from any 'load handler' prossesed, by checking that the clients who
genreate the load is disconnceted.
Also, replacing check of dbsize with wait_for_ofs_sync before comparing debug digest, as
it would fail in case the last key the workload wrote was an overridden key (not a new one).
Affected tests
Race fix:
- failover command to specific replica works
- Connect multiple replicas at the same time (issue #141), master diskless=$mdl, replica diskless=$sdl
- AOF rewrite during write load: RDB preamble=$rdbpre
Cleanup and speedup:
- Test replication with blocking lists and sorted sets operations
- Test replication with parallel clients writing in different DBs
- Test replication partial resync: $descr (diskless: $mdl, $sdl, reconnect: $reconnect
(cherry picked from commit 32a2584e079a1b3c2d1e6649e38239381a73a459)
When test stop 'load handler' by killing the process that generating the load,
some commands that already in the input buffer, still might be processed by the server.
This may cause some instability in tests, that count on that no more commands
processed after we stop the `load handler'
In this commit, new proc 'wait_load_handlers_disconnected' added, to verify that no more
cammands from any 'load handler' prossesed, by checking that the clients who
genreate the load is disconnceted.
Also, replacing check of dbsize with wait_for_ofs_sync before comparing debug digest, as
it would fail in case the last key the workload wrote was an overridden key (not a new one).
Affected tests
Race fix:
- failover command to specific replica works
- Connect multiple replicas at the same time (issue #141), master diskless=$mdl, replica diskless=$sdl
- AOF rewrite during write load: RDB preamble=$rdbpre
Cleanup and speedup:
- Test replication with blocking lists and sorted sets operations
- Test replication with parallel clients writing in different DBs
- Test replication partial resync: $descr (diskless: $mdl, $sdl, reconnect: $reconnect
(cherry picked from commit 32a2584e079a1b3c2d1e6649e38239381a73a459)
There are two issues fixed in this commit:
1. we want to fail the EXEC command in case there is a watched key that's logically
expired but not yet deleted by active expire or lazy expire.
2. we saw that currently cache time is update in every `call()` (including nested calls),
this time is being also being use for the isKeyExpired comparison, we want to update
the cache time only in the first call (execCommand)
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit ac8b1df8850cc80fbf9ce8c2fbde0c1d3a1b4e91)
There are two issues fixed in this commit:
1. we want to fail the EXEC command in case there is a watched key that's logically
expired but not yet deleted by active expire or lazy expire.
2. we saw that currently cache time is update in every `call()` (including nested calls),
this time is being also being use for the isKeyExpired comparison, we want to update
the cache time only in the first call (execCommand)
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit ac8b1df8850cc80fbf9ce8c2fbde0c1d3a1b4e91)
In aof rewrite, when parent stop sending data to child, if there is
new rewrite data, aofChildWriteDiffData write event will be installed.
Then this event is issued and deletes the file event without do anyting.
This will happen over and over again until aof rewrite finish.
This bug used to waste a few system calls per excessive wake-up
(epoll_ctl and epoll_wait) per cycle, each cycle triggered by receiving
a write command from a client.
(cherry picked from commit cb961d8c8e10ff3b619f1579a03336a15e9e6f45)
In aof rewrite, when parent stop sending data to child, if there is
new rewrite data, aofChildWriteDiffData write event will be installed.
Then this event is issued and deletes the file event without do anyting.
This will happen over and over again until aof rewrite finish.
This bug used to waste a few system calls per excessive wake-up
(epoll_ctl and epoll_wait) per cycle, each cycle triggered by receiving
a write command from a client.
(cherry picked from commit cb961d8c8e10ff3b619f1579a03336a15e9e6f45)
Currently a replica is able to recover from a short read (when diskless loading
is enabled) and avoid crashing/exiting, replying to the master and then the rdb
could be sent again by the master for another load attempt by the replica.
There were a few scenarios that were not behaving similarly, such as when
there is no end-of-file marker, or when module aux data failed to load, which
should be allowed to occur due to a short read.
(cherry picked from commit f06d782f5abcb30efb0117841232828ed3e129bf)
Currently a replica is able to recover from a short read (when diskless loading
is enabled) and avoid crashing/exiting, replying to the master and then the rdb
could be sent again by the master for another load attempt by the replica.
There were a few scenarios that were not behaving similarly, such as when
there is no end-of-file marker, or when module aux data failed to load, which
should be allowed to occur due to a short read.
(cherry picked from commit f06d782f5abcb30efb0117841232828ed3e129bf)
