[backport] Backport sweep for 9.2 - #4731
valkeyrie-ops[bot] wants to merge 3 commits into
Conversation
A NACK for an old election could be counted against the current one, since the receiver only checked the sender's header epoch. Carry the rejected request's epoch in the NACK and only count exact matches. Fixes #4627. **Wire format change.** `clusterMsgDataFailoverNack` grows from 1 byte (`reason`) to 16 bytes (`epoch`, `reason`, 7 reserved). `CLUSTER_NODE_FAILOVER_AUTH_NACK_SUPPORTED` is unchanged, and `clusterIsValidPacket` requires an exact length for this type, so a node with this change and a node without it will drop each other's NACKs (one `LL_WARNING` per dropped packet) and fall back to the timeout-based election retry that predates #3833. Nothing released sets that capability bit (#3833 is only on `unstable`), so this only affects mixed `unstable` builds; it is not a rolling-upgrade concern for any released version. Signed-off-by: Dante Knowles <xdk@amazon.com> Backport-Source-PR: 4654
Follow-up to #3853, addresses #3853 (comment). The LZ4 contexts go through the custom zmalloc allocator, so their memory is inside `used_memory` but not attributed to anything and shows up under `used_memory_dataset`. There are two of them: the per-replica `LZ4F_cctx` on the primary, and the stream reader on the replica (`LZ4F_dctx` plus a retained input buffer), which lives on the server struct so per-client accounting never sees it. `zmalloc_size(ctx)` doesn't work here. The context struct is ~200 bytes and the staging buffers it allocates lazily are ~80 KiB. So the allocator callbacks now count the bytes into a `ctx_memory` field on the owning stream, via `LZ4F_CustomMem.opaqueState`. `getClientMemoryUsage()` picks up the compressor context in the existing replica block, and charges the replica-side reader to the primary client, so it shows up in `mem_clients_normal` like other client memory. Added a unit test that checks the counter grows when the streaming buffers are allocated and drops back to zero on free. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> Backport-Source-PR: 4702
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 9.2 #4731 +/- ##
==========================================
- Coverage 80.74% 80.71% -0.03%
==========================================
Files 192 192
Lines 100803 100853 +50
==========================================
+ Hits 81390 81403 +13
- Misses 19413 19450 +37
🚀 New features to boost your workflow:
|
|
Automatic follow-up for the current backport head. I did not push a fix: fix not applied (agent declined or made no edits) Looked at the failure from this run. Evidence |
|
Automatic follow-up for the current backport head. Fixed Throttling tears down when disabling config in tests/integration/throttle-repl.tcl and pushed Fixing the failure from this run. Root cause: The throttle-repl integration test 'Throttling tears down when disabling config' timed out ('*** [TIMEOUT]: ... in tests/integration/throttle-repl.tcl', Test Summary: 5805 passed, 1 failed) because the release branch carries the pre-deflake version of throttle-repl.tcl, which upstream already fixed in the deflake commit. Verified by: ported upstream fix; awaiting this PR's normal CI Review: Ported upstream commit 1c847e8 with its original authorship; this PORT path relies on the PR's normal CI as the verification authority. Other checks also failed in that run; re-invoke with the same command to address the next one:
This is a port of an upstream fix; this PR's normal CI is the verification authority. I do not merge. The fix was pushed as |
Fixes #4724, #4722, #4721, #4720, #4719 ### valgrind Tests defined in throttle-repl are time sensitive, running them under valgrind makes them highly unstable. valgrind:skip was added to the tests ### Throttling not protect a replica above the hard COB limit setting `soft_time = 60`, as this test is related to hard limit, there is no need to exercise any soft limit checks ### Throttling protects a replica above the soft COB limit setting `soft_time = 1`, as using 0 results in a possible race condition - Soft limit is hit first time at the last 100ms tick of Second X - At the first tick of Second X+1, the soft limit is hit again - `trendCal` is not able to install throttler in time (Doesn't have enough samples) - `throttleRepl_isClientExemptFromCobLimits` check fails as no throttler is installed - Connection is disconnected due to soft limit Setting `soft_time = 1` solves this race as it give `trendCal` enough time to install the throttler while exercising the `throttleRepl_isClientExemptFromCobLimits` code path ### Throttling tears down when failover happened After the replica is resumed, the 2 nodes will create a replica chain - Node1 is replica of Node2 - Node2 is replica of Node1 `$replica replicaof no one` was added to create a clear primary between the 2 nodes --------- Signed-off-by: Bara' Hasheesh <bara.hasheesh@gmail.com> (cherry picked from commit 1c847e8)
|
Automatic follow-up for the current backport head. I did not push a fix: fix not applied (agent declined or made no edits) Looked at the failure from this run. EvidenceOther checks also failed in that run; re-invoke with the same command to address the next one:
|
Backport sweep for 9.2
Automated cherry-picks from PRs marked "To be backported".
Applied
Generated by valkey-ci-agent using Claude Code.