[thweetkomputer]

Context

Dense Sorted Set pages can be read and decoded twice by a single-member ZADD, ZINCRBY or ZREM. The optional probe cache compares its 64 KiB retention limit with admitted decoder scratch, so an ordinary 8 KiB page containing many short members is discarded before mutation planning reuses it.

Behavior before and after

Point mutations retain up to two ordinary ordered pages through planning, avoiding the repeated source/destination read. Memory pressure drops this optional retention and retries the ordinary path. Full scratch accounting and command behavior are preserved.

Implementation

• Bound direct single-member probe retention by encoded page size and a two-page cap, while keeping each page's full memory reservation.
• Release optional pages and their reservations before retrying a failed read/plan admission; only a final failure records an OOM rejection.
• Add dense-member regressions for source and destination-boundary reuse, plus injected optional-retention admission failure under a 128 MiB limit.

Design decisions and alternatives

Reuse is command-local. Batch operations, externally supplied probes and oversized pages retain their existing policy. This introduces no shared cache or mutex.

Documentation and comments

Ownership, bounds and fallback behavior are documented beside the implementation. No architecture claim or durable format changes.

Test plan

Ran against the unmodified implementation with the new tests: both dense-page regressions failed at the expected injected second plan read. With this change:

build-test/lavik_grouped_ordered_write_e2e_test build-test/lavik \
  --gtest_filter='GroupedSortedSetWriteE2e.*-GroupedSortedSetWriteE2e.AggregateAbove512MiBRemainsBoundedAndRecovers'

30 tests passed, including all three new regressions, dual-index consistency and clean restart checks. GCC 13 Release/SPDK build, clang-format 23.1.1 and git diff --check passed.

One cross-check against base 9c68079, using the same persistent dataset: 8,192 ZSets, 32,768 members of 32 B per key, 80 connections, 90-second warmup and 60-second measurement per point.

Workload	Base throughput	This change	Change	p99.9 ms, base → change
ZINCRBY	24,191 QPS	33,122 QPS	+36.9%	32.002 → 26.800
Sequential ZREM + ZADD	19,389 pairs/s	20,387 pairs/s	+5.1%	36.641 → 32.944

All requests succeeded; command counts and the checkpoint/state chain reconcile. CPU occupancy was approximately unchanged. The paired workload improvement came from ZREM; ZADD mean latency increased from 1.628 to 1.733 ms. These are one-run observations in fixed base→candidate order on an evolving dataset.

Risk assessment

Optional retention increases the live scratch of point edits. The page/count bounds and admission fallback constrain it; corruption checks and full memory charges remain in place. Runtime probe-hit and device-I/O reductions were not separately measured.

Rollback plan

Revert this commit; no migration or data-format change is required.

Reviewer guide

Start with the point-write retention and reserve_work logic in zset.cpp, then the three dense-page regressions in zset_write_e2e_test.cpp.

Follow-up work

Reduce redundant ordered-page validation and remove whole-page sorting/decoding for eligible single-member insertions in a follow-up branch.

Summary by CodeRabbit

• Bug Fixes
  • Improved sorted-set updates by reusing nearby pages when possible, reducing repeated reads during writes.
  • Sorted-set writes can now fall back to a standard memory reservation when retaining cached pages is not feasible, avoiding unnecessary out-of-memory failures.
  • Added coverage for sorted-set consistency across durability and recovery.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 14fb12f4-7a04-4345-bc39-700e4ca4d41b

📥 Commits

Reviewing files that changed from the base of the PR and between 6b7de97 and d54fec4.

📒 Files selected for processing: 2

• src/storage/engine/grouped/zset.cpp
• tests/grouped/zset_write_e2e_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

Point writes to grouped sorted sets can retain qualifying probed pages during work admission. If the optional reservation fails, the code clears those pages and retries through the ordinary budget path. End-to-end tests cover planner reads, reservation failures, index consistency, and recovery.

Changes

Sorted-Set Probe Retention

Layer / File(s)	Summary
Point-write admission and reservation src/storage/engine/grouped/zset.cpp	Single-member operations can retain up to two qualifying probe pages. Serial page reads and planner reservations use reserve_work, which clears cached pages and retries through the ordinary budget path if the optional reservation fails.
Planner-read and recovery validation tests/grouped/zset_write_e2e_test.cpp	Tests seed and inspect dense sorted sets, exercise planner-read and reservation failures, and verify sorted-set contents across recovery.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to d54fe

The change reuses dense sorted-set pages during single-member writes and falls back to the ordinary memory budget if the optional reservation fails. No concrete merge-blocking risk was found.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main performance change: reusing dense sorted-set pages during point mutations.
Description check		The description includes all required sections and provides clear context, behavior changes, implementation details, design decisions, testing results, risks, rollback steps, reviewer guidance, and fo…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description includes all required sections and provides clear context, behavior changes, implementation details, design decisions, testing results, risks, rollback steps, reviewer guidance, and follow-up work.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

_{I’m a rabbit beside the sorted-set rows,
I watch cached pages as the work budget flows.
When one reservation fails, the pages clear,
The ordinary path makes its next attempt here.
Then I check each score through recovery’s door.}

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