This will allow to use: RedisModule_CreateStringPrintf(ctx, "%s %c %s", "string1", 0, "string2");
On large string, the previous code would incrementally retry to double the output buffer.
now it uses the the return value of snprintf and grows to the right size in one step.
and also avoids an excessive strlen in sdscat at the end.
(cherry picked from commit f4ca3d8757d6abb3536610ddb7b9ab3ad39e81df)
The bug occurs when 'callback' re-registers itself to a point
in the future and the execution time in non-negligible:
'now' refers to time BEFORE callback was executed and is used
to calculate 'next_period'.
We must get the actual current time when calculating 'next_period'
(cherry picked from commit 9cbdc8dcdbaf96869251dd9728c0876adf1b2492)
The bug occurs when 'callback' re-registers itself to a point
in the future and the execution time in non-negligible:
'now' refers to time BEFORE callback was executed and is used
to calculate 'next_period'.
We must get the actual current time when calculating 'next_period'
(cherry picked from commit 1a91a2700b24211e90c695d3fdbbfe7e8d75dbe4)
The RMAPI_FUNC_SUPPORTED was defined in the wrong place on redismodule.h
and was not visible to modules.
(cherry picked from commit 560d2dc0081bc35b81b6a64d25c4077fa2e69ad9)
The RMAPI_FUNC_SUPPORTED was defined in the wrong place on redismodule.h
and was not visible to modules.
(cherry picked from commit 97d647a13920c230a27685992a23679b44b73550)
Turns out this was broken since version 4.0 when we added sds size
classes.
The cluster code uses sds for the receive buffer, and then casts it to a
struct and accesses a 64 bit variable.
This commit replaces the use of sds with a simple reallocated buffer.
(cherry picked from commit b71d06c269878887eed85b63a731c3d4ad7a8b12)
Turns out this was broken since version 4.0 when we added sds size
classes.
The cluster code uses sds for the receive buffer, and then casts it to a
struct and accesses a 64 bit variable.
This commit replaces the use of sds with a simple reallocated buffer.
(cherry picked from commit 380f6048e0bbc762f12fa50b57b73cf29049f967)
When client tracking is enabled signalModifiedKey can increase memory usage,
this can cause the loop in performEvictions to keep running since it was measuring
the memory usage impact of signalModifiedKey.
The section that measures the memory impact of the eviction should be just on dbDelete,
excluding keyspace notification, client tracking, and propagation to AOF and replicas.
This resolves part of the problem described in #8069
p.s. fix took 1 minute, test took about 3 hours to write.
(cherry picked from commit b640e2944e42759412ac67228cf64c43dfbed9c3)
When client tracking is enabled signalModifiedKey can increase memory usage,
this can cause the loop in performEvictions to keep running since it was measuring
the memory usage impact of signalModifiedKey.
The section that measures the memory impact of the eviction should be just on dbDelete,
excluding keyspace notification, client tracking, and propagation to AOF and replicas.
This resolves part of the problem described in #8069
p.s. fix took 1 minute, test took about 3 hours to write.
(cherry picked from commit c4fdf09c0584a3cee32b92f01b7958c72776aedc)
This PR not only fixes the problem that swapdb does not make the
transaction fail, but also optimizes the FLUSHALL and FLUSHDB command to
set the CLIENT_DIRTY_CAS flag to avoid unnecessary traversal of clients.
FLUSHDB was changed to first iterate on all watched keys, and then on the
clients watching each key.
Instead of iterating though all clients, and for each iterate on watched keys.
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit f571f8467cecb69a1c7c6810a445addfb802d85a)
This PR not only fixes the problem that swapdb does not make the
transaction fail, but also optimizes the FLUSHALL and FLUSHDB command to
set the CLIENT_DIRTY_CAS flag to avoid unnecessary traversal of clients.
FLUSHDB was changed to first iterate on all watched keys, and then on the
clients watching each key.
Instead of iterating though all clients, and for each iterate on watched keys.
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 10f94b0ab12f9315939dcccf39d64b9388c0c7fa)
This isn't a leak, just an warning due to unreachable
allocation on the fork child.
Problem created by 4192faa
(cherry picked from commit 997c2dc7ec652ac49a6f1a3a5bb79627aff1a545)
This isn't a leak, just an warning due to unreachable
allocation on the fork child.
Problem created by 92a483b
(cherry picked from commit 2426aaa099e5dfee29cce17af39298d0ce14cc2a)
Turns out that when the fork child crashes, the crash log was deleting
the pidfile from the disk (although the parent is still running.
Now we set the pidfile of the fork process to NULL so the fork process
will never deletes it.
(cherry picked from commit 4192faa9821aa4c19be3bd245d8366a1bc1b0332)
Turns out that when the fork child crashes, the crash log was deleting
the pidfile from the disk (although the parent is still running.
Now we set the pidfile of the fork process to NULL so the fork process
will never deletes it.
(cherry picked from commit 92a483bca2df734aff5caada6c23409ed6256773)
instead of asking for the extra new space it wanted, it asked to grow the
string by the size it already has too.
i.e. a string of 1000 bytes, needing to grow by 10 bytes, would have been
asking for an **additional** 1010 bytes.
(cherry picked from commit f2bde2268a5420ed2d481fc0141cb73de0b1f200)
instead of asking for the extra new space it wanted, it asked to grow the
string by the size it already has too.
i.e. a string of 1000 bytes, needing to grow by 10 bytes, would have been
asking for an **additional** 1010 bytes.
(cherry picked from commit 595ecd5f4be39eeec71fb07f687b2d6b7cf5c20c)
This is a recent problem, introduced by 9b2a426 (redis 6.0)
The implications are:
The sole difference between LookupKeyRead and LookupKeyWrite is for command
executed on a replica, which are not received from its master client. (for the master,
and for the master client on the replica, these two functions behave the same)!
Since SORT is a write command, this bug only implicates a writable-replica.
And these are its implications:
- SORT STORE will behave as it did before the above mentioned commit (like before
redis 6.0). on a writable-replica an already logically expired the key would have
appeared missing. (store dest key would be deleted, instead of being populated
with the data from the already logically expired key)
- SORT (the non store variant, which in theory could have been executed on
read-only-replica if it weren't for the write flag), will (in redis 6.0) have a new bug
and return the data from the already logically expired key instead of empty response.
(cherry picked from commit 6230ba081109beaf367d8fe3552bc848dc3896f4)
This is a recent problem, introduced by 7471743 (redis 6.0)
The implications are:
The sole difference between LookupKeyRead and LookupKeyWrite is for command
executed on a replica, which are not received from its master client. (for the master,
and for the master client on the replica, these two functions behave the same)!
Since SORT is a write command, this bug only implicates a writable-replica.
And these are its implications:
- SORT STORE will behave as it did before the above mentioned commit (like before
redis 6.0). on a writable-replica an already logically expired the key would have
appeared missing. (store dest key would be deleted, instead of being populated
with the data from the already logically expired key)
- SORT (the non store variant, which in theory could have been executed on
read-only-replica if it weren't for the write flag), will (in redis 6.0) have a new bug
and return the data from the already logically expired key instead of empty response.
(cherry picked from commit 08ad6abd04c5aafe5471fa754000e512ae6b0f05)
Turns out the RDB checksum in Redis 6.0 on bigendian is broken.
It always returned 0, so the RDB files are generated as if checksum is
disabled, and will be loaded ok on littleendian, and on bigendian.
But it'll not be able to load RDB files generated on littleendian or older versions.
Similarly DUMP and RESTORE will work on the same version (0==0),
but will be unable to exchange dump payloads with littleendian or old versions.
(cherry picked from commit a67621f495762dfe09f9a7e28d12b0729a7b0d12)
Turns out the RDB checksum in Redis 6.0 on bigendian is broken.
It always returned 0, so the RDB files are generated as if checksum is
disabled, and will be loaded ok on littleendian, and on bigendian.
But it'll not be able to load RDB files generated on littleendian or older versions.
Similarly DUMP and RESTORE will work on the same version (0==0),
but will be unable to exchange dump payloads with littleendian or old versions.
(cherry picked from commit 324070c8f6f63240629893c2ce9bcbeecf6e77fb)
getRDB is "designed" to work in two modes: one for redis-cli --rdb and
one for redis-cli --cluster backup.
in the later case it uses the hiredis connection from the cluster nodes
and it used to free it without nullifying the context, so a later
attempt to free the context would crash.
I suppose the reason it seems to want to free the hiredis context ASAP
is that it wants to disconnect the replica link, so that replication
buffers will not be accumulated.
(cherry picked from commit 93b8b139305a99678528fabc9bf9cc5e9133b5a4)
getRDB is "designed" to work in two modes: one for redis-cli --rdb and
one for redis-cli --cluster backup.
in the later case it uses the hiredis connection from the cluster nodes
and it used to free it without nullifying the context, so a later
attempt to free the context would crash.
I suppose the reason it seems to want to free the hiredis context ASAP
is that it wants to disconnect the replica link, so that replication
buffers will not be accumulated.
(cherry picked from commit 41b2ed2bbc0671e43101feecc48cac26a5e312cb)
When a Lua script returns a map to redis (a feature which was added in
redis 6 together with RESP3), it would have returned the value first and
the key second.
If the client was using RESP2, it was getting them out of order, and if
the client was in RESP3, it was getting a map of value => key.
This was happening regardless of the Lua script using redis.setresp(3)
or not.
This also affects a case where the script was returning a map which it got
from from redis by doing something like: redis.setresp(3); return redis.call()
This fix is a breaking change for redis 6.0 users who happened to rely
on the wrong order (either ones that used redis.setresp(3), or ones that
returned a map explicitly).
This commit also includes other two changes in the tests:
1. The test suite now handles RESP3 maps as dicts rather than nested
lists
2. Remove some redundant (duplicate) tests from tracking.tcl
(cherry picked from commit bcde1d978d74f84cb4a66dc8bbd746842217f8b2)
When a Lua script returns a map to redis (a feature which was added in
redis 6 together with RESP3), it would have returned the value first and
the key second.
If the client was using RESP2, it was getting them out of order, and if
the client was in RESP3, it was getting a map of value => key.
This was happening regardless of the Lua script using redis.setresp(3)
or not.
This also affects a case where the script was returning a map which it got
from from redis by doing something like: redis.setresp(3); return redis.call()
This fix is a breaking change for redis 6.0 users who happened to rely
on the wrong order (either ones that used redis.setresp(3), or ones that
returned a map explicitly).
This commit also includes other two changes in the tests:
1. The test suite now handles RESP3 maps as dicts rather than nested
lists
2. Remove some redundant (duplicate) tests from tracking.tcl
(cherry picked from commit 2017407b4d1d19a91af1e7c0b199f2c1775dbaf9)
The crash log attempts to print the current client info, and when it
does that it attempts to check if the first argument happens to be a key
but it did so for commands with no arguments too, which caused the crash
log to crash half way and not reach its end.
(cherry picked from commit 318d58192289a471987aa12ca913a3569d4f54d0)
The crash log attempts to print the current client info, and when it
does that it attempts to check if the first argument happens to be a key
but it did so for commands with no arguments too, which caused the crash
log to crash half way and not reach its end.
(cherry picked from commit 152b5d46c4a76f2d16031ef794092bfc8d322f8a)
Normally IO threads should simply read data from the socket into the
buffer and attempt to parse it.
If a protocol error is detected, a reply is generated which may result
with installing a write handler which is not thread safe. This fix
delays that until the client is processed back in the main thread.
Fixes#8220
(cherry picked from commit 53f926dd9068a62795abc2e01dabac5be0ccbada)
Normally IO threads should simply read data from the socket into the
buffer and attempt to parse it.
If a protocol error is detected, a reply is generated which may result
with installing a write handler which is not thread safe. This fix
delays that until the client is processed back in the main thread.
Fixes#8220
(cherry picked from commit e7047ec2fcc20e150c0c8cf29addf582638d7e80)
Older arm64 Linux kernels have a bug that could lead to data corruption during
background save under the following scenario:
1) jemalloc uses MADV_FREE on a page,
2) jemalloc reuses and writes the page,
3) Redis forks the background save process, and
4) Linux performs page reclamation.
Under these conditions, Linux will reclaim the page wrongfully and the
background save process will read zeros when it tries to read the page.
The bug has been fixed in Linux with commit:
ff1712f953e27f0b0718762ec17d0adb15c9fd0b ("arm64: pgtable: Ensure dirty bit is
preserved across pte_wrprotect()")
This Commit adds an ignore-warnings config, when not found, redis will
print a warning and exit on startup (default behavior).
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 75bbf56d37d1134cc9c4f36900a4a623604776a5)
in 6.0 this warning is ignored by default in order to avoid adding
regression, specifically for deployments that don't need persistence or
replication
Older arm64 Linux kernels have a bug that could lead to data corruption during
background save under the following scenario:
1) jemalloc uses MADV_FREE on a page,
2) jemalloc reuses and writes the page,
3) Redis forks the background save process, and
4) Linux performs page reclamation.
Under these conditions, Linux will reclaim the page wrongfully and the
background save process will read zeros when it tries to read the page.
The bug has been fixed in Linux with commit:
ff1712f953e27f0b0718762ec17d0adb15c9fd0b ("arm64: pgtable: Ensure dirty bit is
preserved across pte_wrprotect()")
This Commit adds an ignore-warnings config, when not found, redis will
print a warning and exit on startup (default behavior).
Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit b02780c41dbc5b28d265b5cf141c03c1a7383ef9)
in 6.0 this warning is ignored by default in order to avoid adding
regression, specifically for deployments that don't need persistence or
replication
In response to large client query buffer optimization introduced in 1898e6c. The calculation of the amount of
remaining bytes we need to write to the query buffer was calculated wrong, as a result we are unnecessarily
growing the client query buffer by sdslen(c->querybuf) always. This fix corrects that behavior.
Please note the previous behavior prior to the before-mentioned change was correctly calculating the remaining
additional bytes, and this change makes that calculate to be consistent.
Useful context, the argument of size `ll` starts at qb_pos (which is now the beginning of the sds), but much of it
may have already been read from the socket, so we only need to grow the sds for the remainder of it.
(cherry picked from commit b8c98337cd8f90064085a7af4b416f6fb3e80e1c)
In response to large client query buffer optimization introduced in 1898e6c. The calculation of the amount of
remaining bytes we need to write to the query buffer was calculated wrong, as a result we are unnecessarily
growing the client query buffer by sdslen(c->querybuf) always. This fix corrects that behavior.
Please note the previous behavior prior to the before-mentioned change was correctly calculating the remaining
additional bytes, and this change makes that calculate to be consistent.
Useful context, the argument of size `ll` starts at qb_pos (which is now the beginning of the sds), but much of it
may have already been read from the socket, so we only need to grow the sds for the remainder of it.
(cherry picked from commit 11b3325e9999721d35ec64afac7b917664f6291b)
Module blocked clients cache the response in a temporary client,
the reply list in this client would be affected by the recent fix
in #7202, but when the reply is later copied into the real client,
it would have bypassed all the checks for output buffer limit, which
would have resulted in both: responding with a partial response to
the client, and also not disconnecting it at all.
(cherry picked from commit 11e0c4739b40c23a70aff5287e67e8ef6418bb2a)
Module blocked clients cache the response in a temporary client,
the reply list in this client would be affected by the recent fix
in #7202, but when the reply is later copied into the real client,
it would have bypassed all the checks for output buffer limit, which
would have resulted in both: responding with a partial response to
the client, and also not disconnecting it at all.
(cherry picked from commit 48efc25f749c3620f9245786582ac76cb40e9bf4)
Makes spt_init more careful with assumptions about what memory regions
may be overwritten. It will now only consider a contiguous block of argv
and envp elements and mind any gaps.
(cherry picked from commit 9f272197ea0d8e59f565782a6a1b420b9b94266d)
