fix(persistence): a restart no longer flattens every compact encoding

[TinDang97]

The bug

RDB decode rebuilt every container in its full form, so a listpack hash, a listpack
list, an intset and a set listpack all came back flattened on the first reload:

key	before restart	after restart	redis 8.6.1 after DEBUG RELOAD
hash	listpack	hashtable	listpack
list	listpack	linkedlist	listpack
set (ints)	intset	hashtable	intset

Redis preserves every one of these. moon preserved none.

This is the critical path for the whole memory campaign. The #787 SADD-listpack win
(set 978.2 -> 404.5 B/key on Linux) reverted to 978.2 — 1.85x back to 4.48x vs redis
7.4.2 — the moment the server came back up. Every per-type encoding win is conditional on
this fix, and every RSS figure measured on a never-restarted server is an upper bound, not
a steady state.

The trap: two decoders, and the obvious one is not the one that runs

src/persistence/rdb.rs has two decode paths:

1. value_codec::decode_value_body — the discoverable one.
2. read_entry_zero_copy — a hand-rolled decoder that builds each container inline and
   never calls value_codec at all.

#2 is the path a restart actually takes, including load_from_bytes, the AOF RDB
preamble. My first attempt hooked only #1: everything compiled, the lib suite stayed
green, and the guard test still failed with all four types flattened. Both funnels are
covered now. (Three CompactValue::from_redis_value funnels exist in that file; the
string one needs no compaction.)

Change

value_codec::compact_after_decode re-derives the compact encoding against thresholds
already in the tree (LISTPACK_MAX_ENTRIES 128, LISTPACK_MAX_ELEMENT_SIZE 64,
INTSET_MAX_ENTRIES 512), applied at both RDB funnels.

No wire-format change. Every listpack variant already maps to the same ValueType tag
as its full form (value_codec::value_type_of), so files stay readable in both
directions. They are not byte-identical — a listpack preserves insertion order where a
HashMap does not; the type tag and field/value encoding are unchanged.

HashWithTtl is deliberately not compacted: a listpack carries no TTL sidecar.

Deliberately NOT the cold/spill path

ValueKind::classify_cold accepts only the canonical full forms, so a cold-decoded
SetListpack falls through its _ => Err(WrongType) arm and would answer WRONGTYPE for
a perfectly valid set. The first version of this fix compacted the shared
decode_value_body and turned 11 cold-tier tests red for exactly that reason. Compaction
is therefore opt-in (decode_value_body_compacting), RDB-path only.

Two named follow-ups: widening classify_cold so the cold tier compacts too, and the
redis_rdb.rs read side used by DUMP/RESTORE and replica full sync — the latter means a
replica can currently hold more memory than its master for identical data.

Tests — red/green

tests/restart_preserves_compact_encoding.rs writes one key of each compact type plus a
200-field hash as a negative control (without it, a bug that compacted everything
would pass), BGREWRITEAOFs, restarts a real server on the same --dir, and asserts the
target encoding of all five.

It asserts the post-restart encoding absolutely rather than before == after, because
before == after is branch-dependent: a small string set is a hashtable on main and a
listpack on #787's branch. Stated absolutely, the test is meaningful on both.

Red: h: was listpack, after restart hashtable / l: ... linkedlist / si: ... hashtable.
Green after the fix.

Refs #787
author: Tin Dang

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[TinDang97]

Three existing tests asserted the old behaviour

persistence::rdb::tests::test_round_trip_{hash,list,set} matched RedisValueRef::Hash/List/Set and panicked Expected hash the moment decode began returning the compact form. That is the same pattern as test_object_encoding_set_hashtable in #787, which literally asserted "SADD with non-integer members should create hashtable" — a test codifying the defect it should have caught.

They now assert the listpack variant and keep every data assertion, so they check strictly more than before: encoding and contents, including that list order survives the re-derivation.

Local gate on the final tree: fmt, clippy --all-targets -D warnings, clippy tokio, and the release lib suite — 5,137 passed / 0 failed. Integration test restart_preserves_compact_encoding red before the fix (all four types flattened) and green after.

[coderabbitai]

Warning

Review limit reached

Next included review available in 51 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 085d07a1-3069-442f-9287-76db58730beb

📥 Commits

Reviewing files that changed from the base of the PR and between d1bcb5a and 9b84406.

📒 Files selected for processing (4)

• CHANGELOG.md
• src/persistence/rdb.rs
• src/storage/value_codec.rs
• tests/restart_preserves_compact_encoding.rs

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}