Commit Graph

2089 Commits

Author SHA1 Message Date
Wen Hui
813924b4c3
Sanitizer reported memory leak for '--invalid' option or port number is missed cases to redis-server. (#12322)
Observed that the sanitizer reported memory leak as clean up is not done
before the process termination in negative/following cases:

**- when we passed '--invalid' as option to redis-server.**

```
 -vm:~/mem-leak-issue/redis$ ./src/redis-server --invalid

*** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
Reading the configuration file, at line 2
>>> 'invalid'
Bad directive or wrong number of arguments

=================================================================
==865778==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7f0985f65867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x558ec86686ec in ztrymalloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:117
    #2 0x558ec86686ec in ztrymalloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:135
    #3 0x558ec86686ec in ztryrealloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:276
    #4 0x558ec86686ec in zrealloc /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:327
    #5 0x558ec865dd7e in sdssplitargs /home/ubuntu/mem-leak-issue/redis/src/sds.c:1172
    #6 0x558ec87a1be7 in loadServerConfigFromString /home/ubuntu/mem-leak-issue/redis/src/config.c:472
    #7 0x558ec87a13b3 in loadServerConfig /home/ubuntu/mem-leak-issue/redis/src/config.c:718
    #8 0x558ec85e6f15 in main /home/ubuntu/mem-leak-issue/redis/src/server.c:7258
    #9 0x7f09856e5d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).

```

**- when we pass '--port' as option and missed to add port number to redis-server.**

```
vm:~/mem-leak-issue/redis$ ./src/redis-server --port

*** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
Reading the configuration file, at line 2
>>> 'port'
wrong number of arguments

=================================================================
==865846==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7fdcdbb1f867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x557e8b04f6ec in ztrymalloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:117
    #2 0x557e8b04f6ec in ztrymalloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:135
    #3 0x557e8b04f6ec in ztryrealloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:276
    #4 0x557e8b04f6ec in zrealloc /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:327
    #5 0x557e8b044d7e in sdssplitargs /home/ubuntu/mem-leak-issue/redis/src/sds.c:1172
    #6 0x557e8b188be7 in loadServerConfigFromString /home/ubuntu/mem-leak-issue/redis/src/config.c:472
    #7 0x557e8b1883b3 in loadServerConfig /home/ubuntu/mem-leak-issue/redis/src/config.c:718
    #8 0x557e8afcdf15 in main /home/ubuntu/mem-leak-issue/redis/src/server.c:7258
    #9 0x7fdcdb29fd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Indirect leak of 10 byte(s) in 1 object(s) allocated from:
    #0 0x7fdcdbb1fc18 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164
    #1 0x557e8b04f9aa in ztryrealloc_usable_internal /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:287
    #2 0x557e8b04f9aa in ztryrealloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:317
    #3 0x557e8b04f9aa in zrealloc_usable /home/ubuntu/mem-leak-issue/redis/src/zmalloc.c:342
    #4 0x557e8b033f90 in _sdsMakeRoomFor /home/ubuntu/mem-leak-issue/redis/src/sds.c:271
    #5 0x557e8b033f90 in sdsMakeRoomFor /home/ubuntu/mem-leak-issue/redis/src/sds.c:295
    #6 0x557e8b033f90 in sdscatlen /home/ubuntu/mem-leak-issue/redis/src/sds.c:486
    #7 0x557e8b044e1f in sdssplitargs /home/ubuntu/mem-leak-issue/redis/src/sds.c:1165
    #8 0x557e8b188be7 in loadServerConfigFromString /home/ubuntu/mem-leak-issue/redis/src/config.c:472
    #9 0x557e8b1883b3 in loadServerConfig /home/ubuntu/mem-leak-issue/redis/src/config.c:718
    #10 0x557e8afcdf15 in main /home/ubuntu/mem-leak-issue/redis/src/server.c:7258
    #11 0x7fdcdb29fd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: 18 byte(s) leaked in 2 allocation(s).

```

As part analysis found that the sdsfreesplitres is not called when this condition checks are being hit.

Output after the fix:


```
vm:~/mem-leak-issue/redis$ ./src/redis-server --invalid

*** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
Reading the configuration file, at line 2
>>> 'invalid'
Bad directive or wrong number of arguments
vm:~/mem-leak-issue/redis$

===========================================
vm:~/mem-leak-issue/redis$ ./src/redis-server --jdhg

*** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
Reading the configuration file, at line 2
>>> 'jdhg'
Bad directive or wrong number of arguments

---------------------------------------------------------------------------
vm:~/mem-leak-issue/redis$ ./src/redis-server --port

*** FATAL CONFIG FILE ERROR (Redis 255.255.255) ***
Reading the configuration file, at line 2
>>> 'port'
wrong number of arguments
```

Co-authored-by: Oran Agra <oran@redislabs.com>
2023-06-20 10:07:29 +03:00
Shaya Potter
07316f16f0
Add ability for modules to know which client is being cmd filtered (#12219)
Adds API
- RedisModule_CommandFilterGetClientId()

Includes addition to commandfilter test module to validate that it works
by performing the same command from 2 different clients
2023-06-20 09:03:52 +03:00
Wen Hui
070453eef3
Cluster human readable nodename feature (#9564)
This PR adds a human readable name to a node in clusters that are visible as part of error logs. This is useful so that admins and operators of Redis cluster have better visibility into failures without having to cross-reference the generated ID with some logical identifier (such as pod-ID or EC2 instance ID). This is mentioned in #8948. Specific nodenames can be set by using the variable cluster-announce-human-nodename. The nodename is gossiped using the clusterbus extension in #9530.

Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
2023-06-17 21:16:51 -07:00
sundb
b00a235186
Use Reservoir Sampling for random sampling of dict, and fix hang during fork (#12276)
## Issue:
When a dict has a long chain or the length of the chain is longer than
the number of samples, we will never be able to sample the elements
at the end of the chain using dictGetSomeKeys().
This could mean that SRANDMEMBER can be hang in and endless loop.
The most severe case, is the pathological case of when someone uses SCAN+DEL
or SSCAN+SREM creating an unevenly distributed dict.
This was amplified by the recent change in #11692 which prevented a
down-sizing rehashing while there is a fork.

## Solution
1. Before, we will stop sampling when we reach the maximum number
  of samples, even if there is more data after the current chain.
  Now when we reach the maximum we use the Reservoir Sampling
  algorithm to fairly sample the end of the chain that cannot be sampled
2. Fix the rehashing code, so that the same as it allows rehashing for up-sizing
  during fork when the ratio is extreme, it will allow it for down-sizing as well.

Issue was introduced (or became more severe) by #11692

Co-authored-by: Oran Agra <oran@redislabs.com>
2023-06-16 19:13:07 +03:00
Binbin
439b0315c8
Fix SPOP/RESTORE propagation when doing lazy free (#12320)
In SPOP, when COUNT is greater than or equal to set's size,
we will remove the set. In dbDelete, we will do DEL or UNLINK
according to the lazy flag. This is also required for propagate.

In RESTORE, we won't store expired keys into the db, see #7472.
When used together with REPLACE, it should emit a DEL or UNLINK
according to the lazy flag.

This PR also adds tests to cover the propagation. The RESTORE
test will also cover #7472.
2023-06-16 08:14:11 -07:00
YaacovHazan
5da9eecdb8
Removing duplicated tests (#12318)
In 4ba47d2d2 the following tests added in both tracking.tcl and introspection.tcl

- Coverage: Basic CLIENT CACHING
- Coverage: Basic CLIENT REPLY
- Coverage: Basic CLIENT TRACKINGINFO
- Coverage: Basic CLIENT GETREDIR
2023-06-16 15:55:24 +03:00
Binbin
9dc6f93e9d
Add command being unblocked cause another command to get unblocked execution order test (#12324)
* Add command being unblocked cause another command to get unblocked execution order test

In #12301, we observed that if the
`while(listLength(server.ready_keys) != 0)`
in handleClientsBlockedOnKeys is changed to
`if(listLength(server.ready_keys) != 0)`,
the order of command execution will change.

It is wrong to change that. It means that if a command
being unblocked causes another command to get unblocked
(like a BLMOVE would do), then the new unblocked command
will wait for later to get processed rather than right away.

It'll not have any real implication if we change that since
we do call handleClientsBlockedOnKeys in beforeSleep again,
and redis will still behave correctly, but we don't change that.

An example:
1. $rd1 blmove src{t} dst{t} left right 0
2. $rd2 blmove dst{t} src{t} right left 0
3. $rd3 set key1{t}, $rd3 lpush src{t}, $rd3 set key2{t} in a pipeline

The correct order would be:
1. set key1{t}
2. lpush src{t}
3. lmove src{t} dst{t} left right
4. lmove dst{t} src{t} right left
5. set key2{t}

The wrong order would be:
1. set key1{t}
2. lpush src{t}
3. lmove src{t} dst{t} left right
4. set key2{t}
5. lmove dst{t} src{t} right left

This PR adds corresponding test to cover it.

* Add comment near while(listLength(server.ready_keys) != 0)
2023-06-16 15:39:00 +03:00
Binbin
254475537a
Optimize SET PXAT to reduce calls of rewriteClientCommandVector (#12316)
In PXAT case, there is no need to do the rewriteClientCommandVector,
a simply benchmark show we gain a improvement of 10%.
2023-06-15 10:07:47 +03:00
Wen Hui
1946083410
Removing the duplicate test case (#12310)
Looks like the Zadd test case was copied to create Zincrby test case ,but missed to change the command.
2023-06-14 10:03:33 +03:00
Harkrishn Patro
a9e32767f7
Allow cluster slots/shards api to respond during loading (#12269)
It would be helpful for clients to get cluster slots/shards information during a node failover and is loading data.
2023-06-13 18:16:32 +03:00
Binbin
e7129e43e0
Fix XREADGROUP BLOCK stuck in endless loop (#12301)
For the XREADGROUP BLOCK > scenario, there is an endless loop.
Due to #11012, it keep going, reprocess command -> blockForKeys -> reprocess command

The right fix is to avoid an endless loop in handleClientsBlockedOnKey and handleClientsBlockedOnKeys,
looks like there was some attempt in handleClientsBlockedOnKeys but maybe not sufficiently good,
and it looks like using a similar trick in handleClientsBlockedOnKey is complicated.
i.e. stashing the list on the stack and iterating on it after creating a fresh one for future use,
is problematic since the code keeps accessing the global list.

Co-authored-by: Oran Agra <oran@redislabs.com>
2023-06-13 13:27:05 +03:00
Oran Agra
f228ec1ea5
flushSlavesOutputBuffers should not write to replicas scheduled to drop (#12242)
This will increase the size of an already large COB (one already passed
the threshold for disconnection)

This could also mean that we'll attempt to write that data to the socket
and the replica will manage to read it, which will result in an
undesired partial sync (undesired for the test)
2023-06-12 14:05:34 +03:00
YaacovHazan
0bfb6d5582
Reset command duration for rejected command. (#12247)
In 7.2, After 971b177fa we make sure (assert) that
the duration has been recorded when resetting the client.

This is not true for rejected commands.
The use case I found is a blocking command that an ACL rule changed before
it was unblocked, and while reprocessing it, the command rejected and triggered the assert.

The PR reset the command duration inside rejectCommand / rejectCommandSds.

Co-authored-by: Oran Agra <oran@redislabs.com>
2023-06-11 23:12:05 +03:00
Chen Tianjie
2d7d3911da
Allow bigger tolerance in eventloop duration test. (#12179)
In #11963, some new tests about eventloop duration were added, which includes time measurement in TCL scripts. This has caused some unexpected CI failures, such as #12169 and #12177, due to slow test servers or some performance jittering.
2023-06-11 09:02:41 +03:00
Wen Hui
d412269ff8
Adding missing test cases for Addslot Command (#12288)
Added missing test case coverage for below scenarios:

1. The command only works if all the specified slots are, from
  the point of view of the node receiving the command, currently
  not assigned. A node will refuse to take ownership for slots that
  already belong to some other node (including itself).
2. The command fails if the same slot is specified multiple times.
2023-06-11 08:36:26 +03:00
Binbin
da8f7428fa
Try to fix SENTINEL SIMULATE-FAILURE test by re-source init-tests before each test (#12194)
This test was introduced in #12079, it works well most of the time, but
occasionally fails:
```
00:34:45> SENTINEL SIMULATE-FAILURE crash-after-election works: OK
00:34:45> SENTINEL SIMULATE-FAILURE crash-after-promotion works: FAILED: Sentinel set crash-after-promotion but did not exit
```

Don't know the reason, it may be affected by the exit of the previous
crash-after-election test. Because it doesn't really make much sense to
go deeper into it now, we re-source init-tests to get a clean environment
before each test, to try to fix this.

After applying this change, we found a new error:
```
16:39:33> SENTINEL SIMULATE-FAILURE crash-after-election works: FAILED: caught an error in the test couldn't open socket: connection refused
couldn't open socket: connection refused
```

I am guessing the sentinel triggers failover and exits before SENTINEL FAILOVER,
added a new || condition in wait_for_condition to fix it.
2023-05-29 13:43:26 +03:00
Oran Agra
2764dc3768
Optimize MSETNX to avoid double lookup (#11944)
This is a redo of #11594 which got reverted in #11940
It improves performance by avoiding double lookup of the the key.
2023-05-28 10:58:29 +03:00
Oran Agra
6117f28822
Fix WAIT for clients being blocked in a module command (#12220)
So far clients being blocked and unblocked by a module command would
update the c->woff variable and so WAIT was ineffective and got released
without waiting for the command actions to propagate.

This seems to have existed since forever, but not for RM_BlockClientOnKeys.

It is problematic though to know if the module did or didn't propagate
anything in that command, so for now, instead of adding an API, we'll
just update the woff to the latest offset when unblocking, this will
cause the client to possibly wait excessively, but that's not that bad.
2023-05-28 10:10:52 +03:00
Wen Hui
1a188e4ed6
[BUG] Incorrect error msg for XREAD command (#12238)
XREAD only supports a special ID of $ and XREADGROUP only supports ^.
make sure not to suggest the wrong one when rerunning an error about unbalanced ID arguments

Co-authored-by: Oran Agra <oran@redislabs.com>
2023-05-28 08:37:32 +03:00
judeng
d71478a889
postpone the initialization of oject's lru&lfu until it is added to the db as a value object (#11626)
This pr can get two performance benefits:
1. Stop redundant initialization when most robj objects are created
2. LRU_CLOCK will no longer be called in io threads, so we can avoid the `atomicGet`

Another code optimization:
deleted the redundant judgment in dbSetValue, no matter in LFU or LRU, the lru field inold
robj is always the freshest (it is always updated in lookupkey), so we don't need to judge if in LFU
2023-05-24 09:40:11 +03:00
Wen Hui
d664889992
Adding test case for hvals, hkeys, hexists against wrong type (#12198)
HVALS, HKEYS and HEXISTS commands wrong type test cases were not covered so added the test cases.
2023-05-24 09:34:13 +03:00
Binbin
d0994c5bca
Sync the new loglevel nothing to sentinel (#12223)
We add a new loglevel 'nothing' to disable logging in #12133.
This PR syncs that config change to sentinel. Because in #11214
we support modifying loglevel in runtime.

Although I think sentinel doesn't need this nothing config,
it's better to be consistent.
2023-05-24 09:32:39 +03:00
Binbin
ec5721d6ca
Add dummy CLUSTER SLAVES call tests to fix reply ci (#12224)
In #12166, we removed a call to CLUSTER SLAVES, which
then caused reply-schemas ci to fail:
```
WARNING! The following commands were not hit at all:
  cluster|slaves
  ERROR! at least one command was not hit by the tests
```

Because we already have command output that cover CLUSTER REPLICAS
elsewhere, here we simply add some dummy tests to fix the ci.
2023-05-24 09:28:38 +03:00
Ping Xie
4c74dd986f
Exclude aux fields from "cluster nodes" and "cluster replicas" output (#12166)
This commit excludes aux fields from the output of the `cluster nodes` and `cluster replicas` command.
We may decide to re-introduce them in some form or another in the future, but not in v7.2.
2023-05-23 18:32:37 +03:00
binfeng-xin
38e284f106
optimize spopwithcount propagation (#12082)
A single SPOP with command with count argument resulted in many SPOP
commands being propagated to the replica.
This is inefficient because the key name is repeated many times, and is also
being looked-up many times.
also it results in high QPS metrics on the replica.
To solve that, we flush batches of 1024 fields per SPOP command.

Co-authored-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
2023-05-22 10:27:14 +03:00
Binbin
48757934ff
Performance improvement to ZADD and ZRANGESTORE, convert to skiplist and expand dict in advance (#12185)
For zsets that will eventually be stored as the skiplist encoding (has a dict),
we can convert it to skiplist ahead of time. This change checks the number
of arguments in the ZADD command, and converts the data-structure
if the number of new entries exceeds the listpack-max-entries configuration.
This can cause us to over-allocate memory if there are duplicate entries in the
input, which is unexpected.

For ZRANGESTORE, we know the size of the zset, so we can expand
the dict in advance, to avoid the temporary dict from being rehashed
while it grows.

Simple benchmarks shows it provides some 4% improvement in ZADD and 20% in ZRANGESTORE
2023-05-18 15:24:46 +03:00
Hanna Fadida
37cf1984b9
Add BITFIELD_RO basic tests for non-repl use cases (#12187)
Current tests for BITFIELD_RO command are skipped in the external mode,
and therefore reply-schemas-validator reports a coverage error.
This PR adds basic tests to increase coverage.
2023-05-18 12:16:46 +03:00
Wen Hui
df1890ef7f
Allow SENTINEL CONFIG SET and SENTINEL CONFIG GET to handle multiple parameters. (#10362)
Extend SENTINEL CONFIG SET and SENTINEL CONFIG GET to be
compatible with variadic CONFIG SET and CONFIG GET and allow multiple
parameters to be modified in a single call atomically.

Co-authored-by: Oran Agra <oran@redislabs.com>
2023-05-17 10:26:02 +03:00
Binbin
fd566f4050
Fix for set max entries edge case in setTypeCreate / setTypeMaybeConvert (#12183)
In the judgment in setTypeCreate, we should judge size_hint <= max_entries.

This results in the following inconsistencies:
```
127.0.0.1:6379> config set set-max-intset-entries 5 set-max-listpack-entries 5
OK

127.0.0.1:6379> sadd intset_set1 1 2 3 4 5
(integer) 5
127.0.0.1:6379> object encoding intset_set1
"hashtable"
127.0.0.1:6379> sadd intset_set2 1 2 3 4
(integer) 4
127.0.0.1:6379> sadd intset_set2 5
(integer) 1
127.0.0.1:6379> object encoding intset_set2
"intset"

127.0.0.1:6379> sadd listpack_set1 a 1 2 3 4
(integer) 5
127.0.0.1:6379> object encoding listpack_set1
"hashtable"
127.0.0.1:6379> sadd listpack_set2 a 1 2 3
(integer) 4
127.0.0.1:6379> sadd listpack_set2 4
(integer) 1
127.0.0.1:6379> object encoding listpack_set2
"listpack"
```

This was introduced in #12019, added corresponding tests.
2023-05-16 11:32:21 -07:00
Wen Hui
e45272884e
Adding missing test case for smembers scard commands (#12148)
Minor missing test case addition.

SMEMBERS SCARD against non set
SMEMBERS SCARD against non existing key
2023-05-16 09:26:49 -07:00
Oran Agra
2ffde15a1d
increase tollerance of new event loop test, fails on freebsd CI (#12169)
new test added in #11963, fails on freebsd CI which is slow.
2023-05-14 17:40:29 +03:00
JJ Lu
11cf5cbdcc
Fix bug: LPOS RANK LONG_ MIN causes overflow (#12167)
Limit the range of RANK to -LONG_ MAX ~ LONG_ MAX.
Without this limit, passing -9223372036854775808 would effectively
be the same as passing -1.
2023-05-14 09:04:33 +03:00
Chen Tianjie
29ca87955e
Add basic eventloop latency measurement. (#11963)
The measured latency(duration) includes the list below, which can be shown by `INFO STATS`.
```
eventloop_cycles  // ever increasing counter
eventloop_duration_sum // cumulative duration of eventloop in microseconds
eventloop_duration_cmd_sum  // cumulative duration of executing commands in microseconds
instantaneous_eventloop_cycles_per_sec  // average eventloop count per second in recent 1.6s
instantaneous_eventloop_duration_usec  // average single eventloop duration in recent 1.6s
```

Also added some experimental metrics, which are shown only when `INFO DEBUG` is called.
This section isn't included in the default INFO, or even in `INFO ALL` and the fields in this section
can change in the future without considering backwards compatibility.
```
eventloop_duration_aof_sum  // cumulative duration of writing AOF
eventloop_duration_cron_sum  // cumulative duration cron jobs (serverCron, beforeSleep excluding IO and AOF)
eventloop_cmd_per_cycle_max  // max number of commands executed in one eventloop
eventloop_duration_max  // max duration of one eventloop
```

All of these are being reset by CONFIG RESETSTAT
2023-05-12 20:13:15 +03:00
Leibale Eidelman
e04ebdb8d3
fix GEORADIUS[BYMEMBER] STORE & STOREDIST args spec (#12151)
in GEO commands, `STORE` and `STOREDIST` are mutually exclusive.
use `oneof` block to contain them
2023-05-09 14:24:37 +03:00
Binbin
6ab2174d37
EXPIRE precision test more attempts and more tolerant (#12150)
The test failed on MacOS:
```
*** [err]: EXPIRE precision is now the millisecond in tests/unit/expire.tcl
Expected 'somevalue {}' to equal or match '{} {}'
```

`set a [r get x]`, even though we tried 10 times, sometimes we
still get {}, this is a time-sensitive test.

In this PR, we add the following changes:
1. More attempts, change it from 10 to 30.
2. More tolerant, change the `after 900` to `after 800`.

In addition, we judging $a in advance and changing `after 1100`
to `after 300`, this will save us some times.
2023-05-09 14:14:22 +03:00
cui fliter
03d50e0c30
Remove several instances of duplicate "the" in comments (#12144)
Remove several instances of duplicate "the" in comments
2023-05-08 16:12:44 -07:00
Wen Hui
42dd98ec19
adding missing test cases GET and GETEX (#12125)
adding test case of expired key or not exist for GET and GETEX.
for better test coverage.
2023-05-07 11:46:11 +03:00
sundb
ce5f4ea3a9
Delete empty key if fails after moduleCreateEmptyKey() in module (#12129)
When `RM_ZsetAdd()`/`RM_ZsetIncrby()`/`RM_StreamAdd()` fails, if a new key happens to 
be created using `moduleCreateEmptyKey()`, we should clean up the empty key.

## Test
1) Add new module commands(`zset.add` and `zset.incrby`) to cover  `RM_ZsetAdd()`/`RM_ZsetIncrby()`.
2) Add a large-memory test to cover `RM_StreamAdd()`.
2023-05-07 10:13:19 +03:00
zhaozhao.zz
b0dd7b3245
Free backlog only if rsi is invalid when master reboot (#12088)
When master reboot from RDB, if rsi in RDB is valid we should not free replication backlog, even if master_repl_offset or repl-offset is 0.

Since if master doesn't send any data to replicas master_repl_offset is 0, it's a valid number.

A clear example:

1. start a master and apply some write commands, the master's master_repl_offset is 0 since it has no replicas.
2. stop write commands on master, and start another instance and replicaof the master, trigger an FULLRESYNC
3. the master's master_repl_offset is still 0 (set a large number for repl-ping-replica-period), do BGSAVE and restart the master
4. master load master_repl_offset from RDB's rsi and it's still 0, and we should make sure replica can partially resync with master.
2023-05-06 11:53:28 +08:00
Binbin
e49c2a5292
Pause cron to prevent premature shrinking in querybuf test (#12126)
Tests occasionally fail since #12000:
```
*** [err]: query buffer resized correctly when not idle in tests/unit/querybuf.tcl
Expected 0 > 32768 (context: type eval line 11 cmd {assert {$orig_test_client_qbuf > 32768}} proc ::test)

*** [err]: query buffer resized correctly with fat argv in tests/unit/querybuf.tcl
query buffer should not be resized when client idle time smaller than 2s
```

The reason may be because we set hz to 100, querybuf shrinks before we count
client_query_buffer. We avoid this problem by setting pause-cron to 1.
2023-05-04 13:02:08 +03:00
guybe7
857c09b04d
multi.tcl: reset readraw at the end of the test (#12123)
1. reset the readraw mode after a test that uses it. undetected since the
  only test after that on the same server didn't read any replies.
2. fix a cross slot issue that was undetected in cluster mode because
  readraw doesn't throw exceptions on errors.
2023-05-04 11:58:31 +03:00
Madelyn Olson
5e3be1be09
Remove prototypes with empty declarations (#12020)
Technically declaring a prototype with an empty declaration has been deprecated since the early days of C, but we never got a warning for it. C2x will apparently be introducing a breaking change if you are using this type of declarator, so Clang 15 has started issuing a warning with -pedantic. Although not apparently a problem for any of the compiler we build on, if feels like the right thing is to properly adhere to the C standard and use (void).
2023-05-02 17:31:32 -07:00
Wen Hui
f32d1817e3
Updating missing test cases for Hash commands (#12116)
Adding missing test case against wrong type for HRANDFIELD HGET HGETALL HDEL HINCRBY HINCRBYFLOAT HSTRLEN.
2023-05-01 21:00:07 +03:00
Binbin
d659c73456
Add missing reply schema and coverage tests (#12079)
The change in #12018 break the CI (fixed by #12083).
There are quite a few sentinel commands that are missing both test coverage and also schema.

PR added reply-schema to the following commands:
- sentinel debug
- sentinel info-cache
- sentinel pendding-scripts
- sentinel reset
- sentinel simulate-failure

Added some very basic tests for other sentinel commands, just so that they have some coverage.
- sentinel help
- sentinel masters
- sentinel myid
- sentinel sentinels
- sentinel slaves

These tests should be improved / replaced in a followup PR.
2023-04-27 09:32:14 +03:00
judeng
9b588f3820
minor optimization for slowlog get (#12103)
We can always know the array length of the response, so there is no need to
use addReplyDeferredLen which may introduce some additional overheads.
2023-04-25 10:17:21 +03:00
Wen Hui
091412cf62
add test cases for decr decrby on missing key (#12070)
Minor test case addition for DECR and DECRBY.

Currently DECR and DECRBY do not have test case coverage for the
scenarios where they run on a non-existing key.
2023-04-19 09:55:56 +03:00
Binbin
20533cc1d7
Tests: Do not save an RDB by default and add a SIGTERM default AOFRW test (#12064)
In order to speed up tests, avoid saving an RDB (mostly notable on shutdown),
except for tests that explicitly test the RDB mechanism

In addition, use `shutdown-on-sigterm force` to prevetn shutdown from failing
in case the server is in the middle of the initial AOFRW

Also a a test that checks that the `shutdown-on-sigterm default` is to refuse
shutdown if there's an initial AOFRW

Co-authored-by: Guy Benoish <guy.benoish@redislabs.com>
2023-04-18 16:14:26 +03:00
sundb
42c8c61813
Fix some compile warnings and errors when building with gcc-12 or clang (#12035)
This PR is to fix the compilation warnings and errors generated by the latest
complier toolchain, and to add a new runner of the latest toolchain for daily CI.

## Fix various compilation warnings and errors

1) jemalloc.c

COMPILER: clang-14 with FORTIFY_SOURCE

WARNING:
```
src/jemalloc.c:1028:7: warning: suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma? [-Wstring-concatenation]
                    "/etc/malloc.conf",
                    ^
src/jemalloc.c:1027:3: note: place parentheses around the string literal to silence warning
                "\"name\" of the file referenced by the symbolic link named "
                ^
```

REASON:  the compiler to alert developers to potential issues with string concatenation
that may miss a comma,
just like #9534 which misses a comma.

SOLUTION: use `()` to tell the compiler that these two line strings are continuous.

2) config.h

COMPILER: clang-14 with FORTIFY_SOURCE

WARNING:
```
In file included from quicklist.c:36:
./config.h:319:76: warning: attribute declaration must precede definition [-Wignored-attributes]
char *strcat(char *restrict dest, const char *restrict src) __attribute__((deprecated("please avoid use of unsafe C functions. prefer use of redis_strlcat instead")));
```

REASON: Enabling _FORTIFY_SOURCE will cause the compiler to use `strcpy()` with check,
it results in a deprecated attribute declaration after including <features.h>.

SOLUTION: move the deprecated attribute declaration from config.h to fmacro.h before "#include <features.h>".

3) networking.c

COMPILER: GCC-12

WARNING: 
```
networking.c: In function ‘addReplyDouble.part.0’:
networking.c:876:21: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
  876 |         dbuf[start] = '$';
      |                     ^
networking.c:868:14: note: at offset -5 into destination object ‘dbuf’ of size 5152
  868 |         char dbuf[MAX_LONG_DOUBLE_CHARS+32];
      |              ^
networking.c:876:21: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
  876 |         dbuf[start] = '$';
      |                     ^
networking.c:868:14: note: at offset -6 into destination object ‘dbuf’ of size 5152
  868 |         char dbuf[MAX_LONG_DOUBLE_CHARS+32];
```

REASON: GCC-12 predicts that digits10() may return 9 or 10 through `return 9 + (v >= 1000000000UL)`.

SOLUTION: add an assert to let the compiler know the possible length;

4) redis-cli.c & redis-benchmark.c

COMPILER: clang-14 with FORTIFY_SOURCE

WARNING:
```
redis-benchmark.c:1621:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive] #ifdef USE_OPENSSL
redis-cli.c:3015:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive] #ifdef USE_OPENSSL
```

REASON: when _FORTIFY_SOURCE is enabled, the compiler will use the print() with
check, which is a macro. this may result in the use of directives within the macro, which
is undefined behavior.

SOLUTION: move the directives-related code out of `print()`.

5) server.c

COMPILER: gcc-13 with FORTIFY_SOURCE

WARNING:
```
In function 'lookupCommandLogic',
    inlined from 'lookupCommandBySdsLogic' at server.c:3139:32:
server.c:3102:66: error: '*(robj **)argv' may be used uninitialized [-Werror=maybe-uninitialized]
 3102 |     struct redisCommand *base_cmd = dictFetchValue(commands, argv[0]->ptr);
      |                                                              ~~~~^~~
```

REASON: The compiler thinks that the `argc` returned by `sdssplitlen()` could be 0,
resulting in an empty array of size 0 being passed to lookupCommandLogic.
this should be a false positive, `argc` can't be 0 when strings are not NULL.

SOLUTION: add an assert to let the compiler know that `argc` is positive.

6) sha1.c

COMPILER: gcc-12

WARNING:
```
In function ‘SHA1Update’,
    inlined from ‘SHA1Final’ at sha1.c:195:5:
sha1.c:152:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
  152 |             SHA1Transform(context->state, &data[i]);
      |             ^
sha1.c:152:13: note: referencing argument 2 of type ‘const unsigned char[64]’
sha1.c: In function ‘SHA1Final’:
sha1.c:56:6: note: in a call to function ‘SHA1Transform’
   56 | void SHA1Transform(uint32_t state[5], const unsigned char buffer[64])
      |      ^
In function ‘SHA1Update’,
    inlined from ‘SHA1Final’ at sha1.c:198:9:
sha1.c:152:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
  152 |             SHA1Transform(context->state, &data[i]);
      |             ^
sha1.c:152:13: note: referencing argument 2 of type ‘const unsigned char[64]’
sha1.c: In function ‘SHA1Final’:
sha1.c:56:6: note: in a call to function ‘SHA1Transform’
   56 | void SHA1Transform(uint32_t state[5], const unsigned char buffer[64])
```

REASON: due to the bug[https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922], when
enable LTO, gcc-12 will not see `diagnostic ignored "-Wstringop-overread"`, resulting in a warning.

SOLUTION: temporarily set SHA1Update to noinline to avoid compiler warnings due
to LTO being enabled until the above gcc bug is fixed.

7) zmalloc.h

COMPILER: GCC-12

WARNING: 
```
In function ‘memset’,
    inlined from ‘moduleCreateContext’ at module.c:877:5,
    inlined from ‘RM_GetDetachedThreadSafeContext’ at module.c:8410:5:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:59:10: warning: ‘__builtin_memset’ writing 104 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
   59 |   return __builtin___memset_chk (__dest, __ch, __len,
```

REASON: due to the GCC-12 bug [https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503],
GCC-12 cannot see alloc_size, which causes GCC to think that the actual size of memory
is 0 when checking with __glibc_objsize0().

SOLUTION: temporarily set malloc-related interfaces to `noinline` to avoid compiler warnings
due to LTO being enabled until the above gcc bug is fixed.

## Other changes
1) Fixed `ps -p [pid]`  doesn't output `<defunct>` when using procps 4.x causing `replication
  child dies when parent is killed - diskless` test to fail.
2) Add a new fortify CI with GCC-13 and ubuntu-lunar docker image.
2023-04-18 09:53:51 +03:00
Wen Hui
d2db4aa753
Added getrange missing testcase (#12061)
Minor test case addition.
Currently GETRANGE command does not have the test case coverage for the scenarios:
An error is returned when key exists but of different type
Added missing test cases for getrange command.
2023-04-18 08:32:10 +03:00
judeng
e7f18432b8
avoid incorrect shrinking of querybuf when client is reading a big argv (#12000)
this pr fix two wrongs:
1. When client’s querybuf is pre-allocated for a fat argv, we need to update the
  querybuf_peak of the client immediately to completely avoid the unexpected
  shrinking of querybuf in the next clientCron (before data arrives to set the peak).
2. the protocol's bulklen does not include `\r\n`, but the allocation and the data we
  read does. so in `clientsCronResizeQueryBuffer`, the `resize` or `querybuf_peak`
  should add these 2 bytes.

the first bug is likely to hit us on large payloads over slow connections, in which case
transferring the payload can take longer and a cron event will be triggered (specifically
if there are not a lot of clients)
2023-04-16 15:49:26 +03:00