[thweetkomputer]

Context

#435 made ZRANK and ZREVRANK read the member score during group-lock planning, then load and scan the same member leaf again during rank projection.

Behavior before and after

Reuse the completed lookup, including a confirmed miss, while retaining the member-group S lock. Rank still uses the common directory captured after acquiring the ordered-group holds, so concurrent changes to unrelated prefix counts remain consistent.

Implementation

Pass the protected lookup into the existing executor and skip the redundant member read, scan and temporary input map. Add a cold-read regression covering forward/reverse ranks, hits and misses.

Design decisions and alternatives

Reuse only the score or absence protected by the member hold. Whole-key fallback performs a fresh lookup after releasing the partial locks.

Documentation and comments

Documented the borrowed lookup's lifetime and protection, and why rank uses the final directory. The existing architecture invariants remain accurate; no architecture or format changes are needed.

Test plan

• Debug build passed.
• 20 focused unit tests passed. The new regression verifies two physical reads for a fixture hit (member plus ordered page), and one for a miss.
• Seven grouped ZSET E2E tests passed, covering concurrent publications, tied scores, selected-page routing, nil replies and recovery.
• Formatting and whitespace checks passed.
• The original throughput comparisons have not been rerun; this PR does not claim a quantified throughput improvement.

Exact validation commands

cmake --build build_debug --target lavik lavik_unit_tests lavik_grouped_ordered_write_e2e_test --parallel 4

build_debug/lavik_unit_tests '--gtest_filter=StorageGroupedReadTest.*:StorageGroupedReadDeathTest.*:GroupedLockTest.*:SortedSetPublicationTest.*:SortedSetPublicationDeathTest.*'

NO_PROXY=127.0.0.1,localhost no_proxy=127.0.0.1,localhost ctest --test-dir build_debug --output-on-failure --stop-on-failure -R '^lavik_grouped_ordered_write_e2e\.GroupedSortedSetWriteE2e\.(IndexedPointAndBatchWritesKeepTiesAndBothIndexesAcrossRecovery|RangeReadsKeepOneCutWhileOtherGroupsPublish|MemberScoreProjectionPreservesOrderAndRecovery|MemberScoresUsePrefixPagesWithoutOrderedReads|MemberRanksSkipUnrelatedPagesAfterRecovery|MemberPrefixesSkipUnneededMidpointReads|BoundedRangeAliasesRanksBoundsAndMixedScoreLex)$'

pre-commit run --files src/storage/engine/impl.h src/storage/engine/grouped/zset.cpp tests/storage_write_buffer_test.cpp

git diff --check origin/main...HEAD

Risk assessment

The reused lookup must remain protected through ordered-lock waits and projection. The caller owns its storage and member hold until the awaited executor finishes; existing concurrent-read regressions cover the common-directory boundary.

Rollback plan

Revert this commit; no data migration is required.

Reviewer guide

Review the lookup handoff in TryConcurrentSortedSetPageRead, its use in ExecuteGroupedSortedSetLocked, and RanksReadEachRequiredPayloadOnce.

Follow-up work

Rerun the original ZSET throughput comparisons to quantify the recovered performance.

[coderabbitai]

Caution

Review failed

The pull request is closed.

• 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