Skip to content

Redis 8.2.1 #28

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.

Sign up for GitHub

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

Open
tomerqodo wants to merge 4 commits into
base: qodo_action_req_1_base_redis_821_pr6
Choose a base branch
from
Open

Redis 8.2.1 #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d7e343
Prevent active defrag from triggering during replicaof db flush (#14274)
sundb Aug 15, 2025
11a8d37
Prevent crash when cgroups_ref is null in streamEntryIsReferenced() a…
sundb Aug 15, 2025
58de836
Redis 8.2.1
YaacovHazan Aug 18, 2025
3d3ed35
update pr
tomerqodo Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions 00-RELEASENOTES
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@ The release notes contain PRs from multiple repositories:
#Tn = Time Series (https://github.com/RedisTimeSeries/RedisTimeSeries)
#Pn = Probabilistic (https://github.com/RedisBloom/RedisBloom)


=============================================================
8.2.1 (v8.2.1) Committed Mon 18 Aug 2025 12:00:00 IST
=============================================================

Update urgency: `MODERATE`: Program an upgrade of the server, but it's not urgent.

### Bug fixes

- #14240 `INFO KEYSIZES` - potential incorrect histogram updates on cluster mode with modules
- #14274 Disable Active Defrag during flushing replica
- #14276 `XADD` or `XTRIM` can crash the server after loading RDB
- #Q6601 Potential crash when running `FLUSHDB` (MOD-10681)

### Performance and resource utilization

- Query Engine - LeanVec and LVQ proprietary Intel optimizations were removed from Redis Open Source
- #Q6621 Fix regression in `INFO` (MOD-10779)

===========================================================
8.2 GA (v8.2.0) Released Mon 4 Aug 2025 15:00:00 IST
===========================================================
Expand Down
10 changes: 10 additions & 0 deletions src/replication.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1949,7 +1949,17 @@ static void rdbLoadEmptyDbFunc(void) {
serverLog(LL_NOTICE, "MASTER <-> REPLICA sync: Flushing old data");
int empty_db_flags = server.repl_slave_lazy_flush ? EMPTYDB_ASYNC :
EMPTYDB_NO_FLAGS;

/* Temporarily disable active defragmentation during database flush.
* This prevents defrag from being triggered in replicationEmptyDbCallback()
* which could modify the database while it's being emptied. */
int orig_active_defrag = server.active_defrag_enabled;
server.active_defrag_enabled = 0;

emptyData(-1, empty_db_flags, replicationEmptyDbCallback);

/* Restore the original active defragmentation setting. */
server.active_defrag_enabled = 1;
Comment on lines +1956 to +1962

@qodo-code-review qodo-code-review Bot Jan 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. orig_active_defrag unused variable 📘 Rule violation ⛯ Reliability

• orig_active_defrag is introduced but never referenced, which will typically trigger an
  -Wunused-variable warning.
• With -Werror enabled, this warning becomes a build failure, violating the requirement to compile
  cleanly with warnings-as-errors.
• This also indicates the intended “restore original setting” behavior is not implemented as written
  in the comment.
Agent prompt 
## Issue description
A new local variable `orig_active_defrag` is assigned but never used, which will likely trigger `-Wunused-variable` and fail builds when compiled with `-Werror`.

## Issue Context
The comment says the original active defrag setting should be restored after `emptyData(...)`, but the code restores a hard-coded `1` instead. This both causes the unused variable warning and makes the restore behavior inconsistent with the comment.

## Fix Focus Areas
- src/replication.c[1953-1962]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

/* Once we have a link with the master and the synchronization was
Expand Down
1 change: 1 addition & 0 deletions src/t_stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2705,6 +2705,7 @@ int streamEntryIsReferenced(stream *s, streamID *id) {
return 1;

/* Check if the message is in any consumer group's PEL */
if (!s->cgroups_ref) return 1;
unsigned char buf[sizeof(streamID)];
streamEncodeID(buf, id);
return raxFind(s->cgroups_ref, buf, sizeof(streamID), NULL);
Comment on lines 2707 to 2711

@qodo-code-review qodo-code-review Bot Jan 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

3. Acked trim treated referenced 🐞 Bug ✓ Correctness

• streamEntryIsReferenced() returns 1 (referenced) when s->cgroups_ref is NULL, which is the
  opposite of what the PEL-reference check should imply.
• When consumer groups exist but the PEL is empty (common after ACK + reload), cgroups_ref is
  intentionally NULL, so this change makes fully-ACKed entries look referenced forever.
• This breaks ACKED delete/trim semantics (XADD/XTRIM/XDELEX/XACKDEL), causing trims to fail and
  streams to grow beyond MAXLEN/MINID expectations.
Agent prompt 
### Issue description
`streamEntryIsReferenced()` incorrectly returns `1` (referenced) when `s->cgroups_ref` is NULL. Since `cgroups_ref` is only built when PEL entries exist, NULL should mean “no PEL references exist”, so the function should not treat entries as referenced.

### Issue Context
This function is used by stream trimming/deletion paths under `DELETE_STRATEGY_ACKED` (e.g. XADD MAXLEN/MINID with ACKED), to decide which entries are eligible for removal.

### Fix Focus Areas
- src/t_stream.c[2707-2711]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand Down
5 changes: 3 additions & 2 deletions src/version.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
#define REDIS_VERSION "8.2.0"
#define REDIS_VERSION_NUM 0x00080200
/* Version information */
#define REDIS_VERSION "8.2.1"
#define REDIS_VERSION_NUM 0x00080201
Comment on lines +1 to +3

@qodo-code-review qodo-code-review Bot Jan 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. version.h missing include guards 📘 Rule violation ✓ Correctness

• src/version.h is a header file but contains no include guard at all, so multiple inclusion can
  lead to redefinition errors depending on how it’s used.
• The compliance requirement mandates double-underscore include guards with the __FILENAME_H
  pattern, which this file does not implement.
Agent prompt 
## Issue description
`src/version.h` lacks the required double-underscore include guards for header files.

## Issue Context
The compliance checklist requires all `.h` files to use include guards of the form `#ifndef __FILENAME_H` / `#define __FILENAME_H` / `#endif`.

## Fix Focus Areas
- src/version.h[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

109 changes: 78 additions & 31 deletions tests/unit/memefficiency.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ run_solo {defrag} {
}
}

proc discard_replies_every {rd count frequency discard_num} {
if {$count % $frequency != 0} {
for {set k 0} {$k < $discard_num} {incr k} {
$rd read ; # Discard replies
}
}
Comment on lines +70 to +75

@qodo-code-review qodo-code-review Bot Jan 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

4. Reply discard logic inverted 🐞 Bug ✓ Correctness

• The new discard_replies_every helper discards replies when $count % $frequency != 0, which is
  inverted from the intended “every N commands” behavior.
• In loops that call it every iteration, it will attempt to $rd read thousands of replies after
  only a handful of commands were pipelined, likely blocking the client waiting for replies that were
  never enqueued.
• This can cause test timeouts/hangs across multiple defrag tests (including the newly added
  replication/defrag test).
Agent prompt 
### Issue description
The helper `discard_replies_every` uses an inverted condition (`!= 0`), causing it to read/discard replies on almost every loop iteration. With `redis_deferring_client`, this can block waiting for replies that are not pending.

### Issue Context
Several tests pipeline commands in tight loops and periodically drain replies to avoid excessive buffering. The previous pattern was “every 10000 ops, read 10000 replies”.

### Fix Focus Areas
- tests/unit/memefficiency.tcl[70-76]
- tests/unit/memefficiency.tcl[345-373]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

proc test_active_defrag {type} {
if {[string match {*jemalloc*} [s mem_allocator]] && [r debug mallctl arenas.page] <= 8192} {
test "Active defrag main dictionary: $type" {
Expand Down Expand Up @@ -339,11 +347,7 @@ run_solo {defrag} {
$rd hset bighash $j [concat "asdfasdfasdf" $j]

incr count
if {$count % 10000 == 0} {
for {set k 0} {$k < 10000} {incr k} {
$rd read ; # Discard replies
}
}
discard_replies_every $rd $count 10000 10000
}
# creating that big hash, increased used_memory, so the relative frag goes down
set expected_frag 1.3
Expand All @@ -355,11 +359,7 @@ run_solo {defrag} {
$rd setrange $j 150 a

incr count
if {$count % 10000 == 0} {
for {set k 0} {$k < 10000} {incr k} {
$rd read ; # Discard replies
}
}
discard_replies_every $rd $count 10000 10000
}
assert_equal [r dbsize] 500016

Expand All @@ -369,11 +369,7 @@ run_solo {defrag} {
$rd del $j

incr count
if {$count % 10000 == 0} {
for {set k 0} {$k < 10000} {incr k} {
$rd read ; # Discard replies
}
}
discard_replies_every $rd $count 10000 10000
}
assert_equal [r dbsize] 250016

Expand Down Expand Up @@ -769,12 +765,7 @@ run_solo {defrag} {
$rd lpush biglist2 $val

incr count
if {$count % 10000 == 0} {
for {set k 0} {$k < 10000} {incr k} {
$rd read ; # Discard replies
$rd read ; # Discard replies
}
}
discard_replies_every $rd $count 10000 20000
}

# create some fragmentation
Expand Down Expand Up @@ -884,11 +875,7 @@ run_solo {defrag} {
$rd setrange $j 600 x

incr count
if {$count % 10000 == 0} {
for {set k 0} {$k < 10000} {incr k} {
$rd read ; # Discard replies
}
}
discard_replies_every $rd $count 10000 10000
}

# create some fragmentation of 50%
Expand All @@ -898,11 +885,7 @@ run_solo {defrag} {
incr sent
incr j 1

if {$sent % 10000 == 0} {
for {set k 0} {$k < 10000} {incr k} {
$rd read ; # Discard replies
}
}
discard_replies_every $rd $sent 10000 10000
}

# create higher fragmentation in the first slab
Expand Down Expand Up @@ -959,6 +942,70 @@ run_solo {defrag} {
}
}

test "Active defrag can't be triggered during replicaof database flush. See issue #14267" {
start_server {tags {"repl"} overrides {save ""}} {
set master_host [srv 0 host]
set master_port [srv 0 port]

start_server {overrides {save ""}} {
set replica [srv 0 client]
set rd [redis_deferring_client 0]

$replica config set hz 100
$replica config set activedefrag no
$replica config set active-defrag-threshold-lower 5
$replica config set active-defrag-cycle-min 65
$replica config set active-defrag-cycle-max 75
$replica config set active-defrag-ignore-bytes 2mb

# add a mass of string keys
set count 0
for {set j 0} {$j < 500000} {incr j} {
$rd setrange $j 150 a

incr count
discard_replies_every $rd $count 10000 10000
}
assert_equal [$replica dbsize] 500000

# create some fragmentation
set count 0
for {set j 0} {$j < 500000} {incr j 2} {
$rd del $j

incr count
discard_replies_every $rd $count 10000 10000
}
$rd close
assert_equal [$replica dbsize] 250000

catch {$replica config set activedefrag yes} e
if {[$replica config get activedefrag] eq "activedefrag yes"} {
# Start replication sync which will flush the replica's database,
# then enable defrag to run concurrently with the database flush.
$replica replicaof $master_host $master_port

# wait for the active defrag to start working (decision once a second)
wait_for_condition 50 100 {
[s total_active_defrag_time] ne 0
} else {
after 120 ;# serverCron only updates the info once in 100ms
puts [$replica info memory]
puts [$replica info stats]
puts [$replica memory malloc-stats]
fail "defrag not started."
}

wait_for_sync $replica

# wait for the active defrag to stop working (db has been emptied during replication sync)
wait_for_defrag_stop 500 100
assert_equal [$replica dbsize] 0
}
}
}
} {} {defrag external:skip tsan:skip cluster}

start_cluster 1 0 {tags {"defrag external:skip tsan:skip cluster"} overrides {appendonly yes auto-aof-rewrite-percentage 0 save "" loglevel notice}} {
test_active_defrag "cluster"
}
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/type/stream.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,33 @@ start_server {
assert {[r XLEN mystream] == 1} ;# Successfully trimmed to 1 entries
}

test {XADD with ACKED option doesn't crash after DEBUG RELOAD} {
r DEL mystream
r XADD mystream 1-0 f v

# Create a consumer group and read one message
r XGROUP CREATE mystream mygroup 0
set records [r XREADGROUP GROUP mygroup consumer1 COUNT 1 STREAMS mystream >]
assert_equal [lindex [r XPENDING mystream mygroup] 0] 1

# After reload, the reference relationship between consumer groups and messages
# is correctly rebuilt, so the previously read but unacked message still cannot be deleted.
r DEBUG RELOAD
r XADD mystream MAXLEN = 1 ACKED 2-0 f v
assert_equal [r XLEN mystream] 2

# Acknowledge the read message so the PEL becomes empty
r XACK mystream mygroup [lindex [lindex [lindex [lindex $records 0] 1] 0] 0]
assert {[lindex [r XPENDING mystream mygroup] 0] == 0}

# After reload, since PEL is empty, no cgroup references will be recreated.
r DEBUG RELOAD

# ACKED option should work correctly even without cgroup references.
r XADD mystream MAXLEN = 1 ACKED 3-0 f v
assert_equal [r XLEN mystream] 2
} {} {needs:debug}

test {XADD with MAXLEN option and DELREF option} {
r DEL mystream
r XADD mystream 1-0 f v
Expand Down
Loading