Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

6.2 #3

Open
wants to merge 213 commits into
base: unstable
Choose a base branch
from
Open

6.2 #3

wants to merge 213 commits into from

Conversation

caipengbo
Copy link
Owner

No description provided.

oranagra and others added 30 commits December 14, 2020 20:54
Redis 6.2.0 GA
Redis 6.2.1
Release 6.2.2
Interior rax pointers were not being freed

(cherry picked from commit c73b4dd)
This scene is hard to happen. When first attempt some keys expired,
only kv position is updated not ov. Then socket err happens, second
attempt is taken. This time kv items may be mismatching with ov items.

(cherry picked from commit 080d457)
When redis-check-aof finds an error, it prints the line number for faster troubleshooting. 

(cherry picked from commit 761d7d2)
Modules event subscribers may get wrong things in notifyKeyspaceEvent callback,
such as wrong number of keys, or be able to lookup this key.
This commit changes the order to be like the one in evict.c.

Cleanup:
Since we know the key exists (it expires now), db*Delete is sure to return 1,
so there's no need to check it's output (misleading).

(cherry picked from commit 63acfe4)
This prevents a case where NTP moves the system clock
forward resulting in a false detection of a busy script.

Signed-off-by: zyxwvu Shi <[email protected]>
(cherry picked from commit f61c37c)
…8868)

This solves an issue reported in #8712 in which a replica would bypass
the client write pause check and cause an assertion due to executing a
write command during failover.

The fact is that we don't expect replicas to execute any command other
than maybe REPLCONF and PING, etc. but matching against the ADMIN
command flag is insufficient, so instead i just block keyspace access
for now.

(cherry picked from commit 46f4ebb)
Specifically we had issues with NTP sync failure which was resolved here: vmactions/freebsd-vm@457af73

(cherry picked from commit 2e88b06)
Use an invalid IP address to trigger CONFIG SET bind failure, instead of DNS which is not guaranteed to always fail.

(cherry picked from commit 2b22fff)
- Immediately exit on errors that are not related to topology updates.
- Deprecates the `-e` option ( retro compatible ) and warns that we now
  exit immediately on errors that are not related to topology updates.
- Fixed wrongfully failing on config fetch error (warning only). This only affects RE.

Bottom line:
- MOVED and ASK errors will not show any warning (unlike the throttled error with `-e` before).
- CLUSTERDOWN still prints an error unconditionally and sleeps for 1 second.
- other errors are fatal.

(cherry picked from commit ef6f902)
…de (#8870)

When redis-cli was used with both -c (cluster) and -s (unix socket),
it would have kept trying to use that unix socket, even if it got
redirected by the cluster (resulting in an infinite loop).

(cherry picked from commit 416f277)
…(#8872)

missing zfree(data) in redis-benchmark.

And also correct the wrong size in lrange.
the text mentioned 500, but size was 450, changed to 500

(cherry picked from commit 1eff856)
oranagra and others added 30 commits December 12, 2022 17:02
This test sets the master ping interval to 1 hour, in order to avoid
pings in the replicatoin stream incrementing the replication offset,
however, it didn't increase the repl-timeout so on slow machines
where the test took more than 60 seconds, the replicas would drop
and reconnect.

```
*** [err]: PSYNC2: Partial resync after restart using RDB aux fields in tests/integration/psync2.tcl
Replica didn't partial sync
```

The test would detect 4 additional partial syncs where it expects
only one.

(cherry picked from commit b0250b4)
**Signed integer overflow.** Although, signed overflow issue can be problematic time to time
and change how compiler generates code, current findings mostly about signed shift or simple
addition overflow. For most platforms Redis can be compiled for, this wouldn't cause any issue
as far as I can tell (checked generated code on godbolt.org).

UB means nothing guaranteed and risky to reason about program behavior but I don't think any
of the fixes here worth backporting. As sanitizers are now part of the CI, preventing new issues
will be the real benefit.

partial cherry pick from commit b91d8b2
The bug in BITFIELD seems to affect 12.2.1 used on Alpine
* Fix test modules linking on macOS 11.x.
* Use macOS 10.x for FreeBSD VM as VirtualBox is not yet supported on
  11.

(cherry picked from commit 6d5a911)
This solves several problems in a more elegant way:

* No need to explicitly use `-lc` on x86_64 when building with `-m32`.
* Avoids issues with undefined floating point emulation funcs on ARM.

(cherry picked from commit f26e90b)
Before this commit, TLS tests on Ubuntu 22.04 would fail as dropped
connections result with an ECONNABORTED error thrown instead of an empty
read.

(cherry picked from commit 69d5576)
As Sentinel supports dynamic IP only when using hostnames, there
are few leftover addess comparison logic that doesn't take into
account that the IP might get change.

Co-authored-by: moticless <[email protected]>
(cherry picked from commit 4a27aa4)
Related to the hang reported in #11671
Currently, redis can disconnect a client due to reaching output buffer limit,
it'll also avoid feeding that output buffer with more data, but it will keep
running the loop in the command (despite the client already being marked for
disconnection)

This PR is an attempt to mitigate the problem, specifically for commands that
are easy to abuse, specifically: KEYS, HRANDFIELD, SRANDMEMBER, ZRANDMEMBER.
The RAND family of commands can take a negative COUNT argument (which is not
bound to the number of elements in the key), so it's enough to create a key
with one field, and then these commands can be used to hang redis.
For KEYS the caller can use the existing keyspace in redis (if big enough).
Authenticated users issuing specially crafted SETRANGE and SORT(_RO)
commands can trigger an integer overflow, resulting with Redis attempting
to allocate impossible amounts of memory and abort with an OOM panic.
missing range check in ZRANDMEMBER and HRANDIFLD leading to panic due
to protocol limitations
in Redis 7.0 this fix covers KEYS as well, but in 6.2 and 6.0 it doesn't,
this is because in 7.0 there's a mechanism to avoid sending partial replies
to the client, and in older releases there isn't, and without it there's a
risk that the client would be able to read what looks like a complete KEYS
command.
Turns out that a fork child calling getExpire while persisting keys (and
possibly also a result of some module fork tasks) could cause dictFind
to do incremental rehashing in the child process, which is both a waste
of time, and also causes COW harm.

(cherry picked from commit 2bec254)
(cherry picked from commit 3e82bdf)
In #7875 (Redis 6.2), we changed the sds alloc to be the usable allocation
size in order to:

> reduce the need for realloc calls by making the sds implicitly take over
the internal fragmentation

This change was done most sds functions, excluding `sdsRemoveFreeSpace` and
`sdsResize`, the reason is that in some places (e.g. clientsCronResizeQueryBuffer)
we call sdsRemoveFreeSpace when we see excessive free space and want to trim it.
so if we don't trim it exactly to size, the caller may still see excessive free space and
call it again and again.

However, this resulted in some excessive calls to realloc, even when there's no need
and it's gonna be a no-op (e.g. when reducing 15 bytes allocation to 13).

It turns out that a call for realloc with jemalloc can be expensive even if it ends up
doing nothing, so this PR adds a check using `je_nallocx`, which is cheap to avoid
the call for realloc.

in addition to that this PR unifies sdsResize and sdsRemoveFreeSpace into common
code. the difference between them was that sdsResize would avoid using SDS_TYPE_5,
since it want to keep the string ready to be resized again, while sdsRemoveFreeSpace
would permit using SDS_TYPE_5 and get an optimal memory consumption.
now both methods take a `would_regrow` argument that makes it more explicit.

the only actual impact of that is that in clientsCronResizeQueryBuffer we call both sdsResize
and sdsRemoveFreeSpace for in different cases, and we now prevent the use of SDS_TYPE_5 in both.

The new test that was added to cover this concern used to pass before this PR as well,
this PR is just a performance optimization and cleanup.

Benchmark:
`redis-benchmark -c 100 -t set  -d 512 -P 10  -n  100000000`
on i7-9850H with jemalloc, shows improvement from 1021k ops/sec to 1067k (average of 3 runs).
some 4.5% improvement.

Co-authored-by: Oran Agra <[email protected]>
(cherry picked from commit 46393f9)
(cherry picked from commit b12eeccddd9318a5d97a5aee2dad88999dfad53f)
(cherry picked from commit a35e083)
(cherry picked from commit 76473f50990e06872d5a08886549815077f5def5)
(cherry picked from commit f7150c45bc5d6f03c8ba86a9a9296d024c6848dc)
…al patterns (CVE-2022-36021)

Authenticated users can use string matching commands with a
specially crafted pattern to trigger a denial-of-service attack on Redis,
causing it to hang and consume 100% CPU time.

(cherry picked from commit e75f92047c22e659d49bba3a083cd0c9935f21e6)
Issue happens when passing a negative long value that greater than
the max positive value that the long can store.

(cherry picked from commit 41430af6a821c551abb862666ef896f2c196dea6)
…149)

Check the validity of the value before performing the create operation,
prevents new data from being generated even if the request fails to execute.

Co-authored-by: Oran Agra <[email protected]>
Co-authored-by: chendianqiang <[email protected]>
Co-authored-by: Binbin <[email protected]>
(cherry picked from commit bc7fe41)
(cherry picked from commit 606a385935363ea46c0df4f40f8a949d85f7a20a)
… (#11992)

The existing logic for killing pub-sub clients did not handle the `allchannels`
permission correctly. For example, if you:

    ACL SETUSER foo allchannels

Have a client authenticate as the user `foo` and subscribe to a channel, and then:

    ACL SETUSER foo resetchannels

The subscribed client would not be disconnected, though new clients under that user
would be blocked from subscribing to any channels.

This was caused by an incomplete optimization in `ACLKillPubsubClientsIfNeeded`
checking whether the new channel permissions were a strict superset of the old ones.

(cherry picked from commit f38aa6b)
(cherry picked from commit 9caeadb8661f5fabd8386e3f07d445fe6ca46e7f)
…1885)

This change attempts to alleviate a minor memory usage degradation for Redis 6.2 and onwards when using rather large objects (~2k) in streams. Introduced in #6281, we pre-allocate the head nodes of a stream to be 4kb, to limit the amount of unnecessary initial reallocations that are done. However, if we only ever allocate one object because 2 objects exceeds the max_stream_entry_size, we never actually shrink it to fit the single item. This can lead to a lot of excessive memory usage. For smaller item sizes this becomes less of an issue, as the overhead decreases as the items become smaller in size.

This commit also changes the MEMORY USAGE of streams, since it was reporting the lpBytes instead of the allocated size. This introduced an observability issue when diagnosing the memory issue, since Redis reported the same amount of used bytes pre and post change, even though the new implementation allocated more memory.

(cherry picked from commit 2bb29e4)
(cherry picked from commit 1718151)
…ons (#11875)

This bug seems to be there forever, CLIENT REPLY OFF|SKIP will
mark the client with CLIENT_REPLY_OFF or CLIENT_REPLY_SKIP flags.
With these flags, prepareClientToWrite called by addReply* will
return C_ERR directly. So the client can't receive the Pub/Sub
messages and any other push notifications, e.g client side tracking.

In this PR, we adding a CLIENT_PUSHING flag, disables the reply
silencing flags. When adding push replies, set the flag, after the reply,
clear the flag. Then add the flag check in prepareClientToWrite.

Fixes #11874

Note, the SUBSCRIBE command response is a bit awkward,
see redis/redis-doc#2327

Co-authored-by: Oran Agra <[email protected]>
(cherry picked from commit 416842e)
(cherry picked from commit f8ae7a4)
New test fails on valgrind because strtold("+inf") with valgrind returns a non-inf result
same thing is done in incr.tcl.

(cherry picked from commit c3b7bde)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.