Redis 8.2.1#6
Conversation
…s#14274) Fix redis#14267 This bug was introduced by redis#13495 ### Summary When a replica clears a large database, it periodically calls processEventsWhileBlocked() in the replicationEmptyDbCallback() callback during the key deletion process. If defragmentation is enabled, this means that active defrag can be triggered while the database is being deleted. The defragmentation process may also modify the database at this time, which could lead to crashes when the database is accessed after defragmentation. Code Path: ``` replicationEmptyDbCallback() -> processEventsWhileBlocked() -> whileBlockedCron() -> defragWhileBlocked() ``` ### Solution This PR temporarily disables active defrag before emptying the database, then restores the active defrag setting after the empty is complete. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…fter reload (redis#14276) This bug was introduced by redis#14130 found by @oranagra ### Summary Because `s->cgroup_ref` is created at runtime the first time a consumer group is linked with a message, but it is not released when all references are removed. However, after `debug reload` or restart, if the PEL is empty (meaning no consumer group is referencing any message), `s->cgroup_ref` will not be recreated. As a result, when executing XADD or XTRIM with `ACKED` option and checking whether a message that is being read but has not been ACKed can be deleted, the cgroup_ref being NULL will cause a crash. ### Code Path ``` xaddCommand -> streamTrim -> streamEntryIsReferenced ``` ### Solution Check if `s->cgroup_ref` is NULL in streamEntryIsReferenced().
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d3ed35ce8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| emptyData(-1, empty_db_flags, replicationEmptyDbCallback); | ||
|
|
||
| /* Restore the original active defragmentation setting. */ | ||
| server.active_defrag_enabled = 1; |
There was a problem hiding this comment.
Here active_defrag_enabled is restored to 1 unconditionally after emptyData, which changes the replica’s runtime configuration whenever defrag was disabled before the sync. This means a replica configured with activedefrag no will unexpectedly end up with defrag enabled after a replication sync. Consider restoring the saved value so the pre-sync configuration is preserved.
Useful? React with 👍 / 👎.
| /* Check if the message is in any consumer group's PEL */ | ||
| if (!s->cgroups_ref) return 1; | ||
| unsigned char buf[sizeof(streamID)]; |
There was a problem hiding this comment.
When s->cgroups_ref is NULL, this now returns “referenced” instead of “not referenced.” After DEBUG RELOAD with an empty PEL, cgroups_ref is not rebuilt, so ACKED trimming will refuse to delete fully acknowledged entries. That causes XADD ... MAXLEN ... ACKED to stop honoring the maxlen limit and lets the stream grow indefinitely in that scenario.
Useful? React with 👍 / 👎.
| proc discard_replies_every {rd count frequency discard_num} { | ||
| if {$count % $frequency != 0} { | ||
| for {set k 0} {$k < $discard_num} {incr k} { |
There was a problem hiding this comment.
The new helper discards replies whenever count % frequency != 0, which runs on almost every iteration. With a deferring client there will only be count replies available, so attempting to read discard_num replies on the first few iterations will block and hang the test. The original behavior only discarded at exact multiples to keep the pipeline bounded.
Useful? React with 👍 / 👎.
3 participants
Benchmark PR from agentic-review-benchmarks#6