Repository navigation
perf: streamline grouped sorted-set validation and point insertion - #358
Merged
thweetkomputer merged 2 commits intoOct 19, 2026
Merged
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit details
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Grouped sorted-set point writes rebuild a duplicate-member hash set during final encoding and sort the entire ordered leaf when inserting one member. This removes that repeated work while preserving validation at the boundary where mutable data becomes a checked after-image.
Behavior before and after
Command semantics and the durable format stay the same. A validated immutable ordered leaf can now be encoded without repeating duplicate-member hashing. Single-member
ZADDinserts at its ordered position and splices a checked member-index leaf when it fits; larger, batched and split cases retain their existing paths.Implementation
lower_boundfor a single new member, preserving score/member ordering.Design decisions and alternatives
Validation is reused only through an immutable checked representation. Skipping validation for arbitrary encoded input would weaken corruption detection. Fast paths remain narrow so batch operations and structural changes use the established implementation.
Documentation and comments
Updated nearby comments, including the absent-member validation paths. No module, lifecycle or durable-format claim changes, so architecture documentation is unchanged.
Test plan
Linux x86-64 GCC 13 Debug: 125 unit tests and 33 grouped end-to-end tests passed. Commands below were run from the test build directory;
lavikwas the corresponding Debug server.git diff --checkand clang-format 23.1.1 checks passed for changed C++ files. The full repository suite was not run. Runtime validation used32bdd6b4; the final001442e8commit changes only a nearby comment.A single 80-connection crossover with 8,192 keys per size is archived in the benchmark report. Full-window QPS changed by +28.2% for grouped 1 MiB ZINCRBY and +3.0% for grouped ZREM/ZADD. However, candidate ZINCRBY slowed from roughly 36.6k QPS during the first 40 seconds to 27.1k during the final 20 seconds; the aggregate is not a demonstrated steady-state gain. Compact 8 KiB points changed by -5.6% and -3.2%, but their resident background data differed. The fixed main-then-candidate order and continuing data state limit causal interpretation. Raw results, time series and limitations are in the linked report.
Risk assessment
The main risk is accidentally treating mutable or insufficiently validated state as checked. The checked representation is immutable, raw/recovery paths keep full validation, and tests cover rejected malformed input plus point-write recovery. Performance observations remain preliminary because of the limitations above.
Rollback plan
Revert these two commits; no data migration or format downgrade is needed.
Reviewer guide
Start with the checked ordered-leaf API and encoder, then inspect single-member planning in
grouped/zset.cppand member-index insertion ingrouped/zset_members.cpp. The tests exercise both fast and fallback paths.Follow-up work
A separate one-round benchmark of #357 stacked on this branch is now in the linked report: all fourteen 8 KiB commands at 80 connections completed successfully. Throughput differences versus the earlier main run range from -1.7% to +2.8%; process CPU decreased for SETRANGE and ZINCRBY by 18.2% and 9.3%, respectively, with polling included. These measurements do not isolate either patch’s contribution. The #357 code is not included in this PR.