feat: enhance zset_score_key_format #225
Conversation
📝 WalkthroughWalkthroughAdded little-endian encoding for member/version and score, exact encoded-user-key length helper, new ZSets score-key types and comparators (ListsDataKey, ZSetsScoreKey) with NaN/Inf rules, migration tooling and CLI for BE→LE migration, benchmarks, tests, and public re-exports for related storage APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Migration CLI
participant Tool as migrate_db_member_format.rs
participant Migr as migration.rs
participant Source as MigrationSource (impl)
participant DB as RocksDB
CLI->>Tool: start migration (args)
Tool->>Migr: build MigrationConfig & call migrate_member_keys_be_to_le
Migr->>Source: scan_keys(cf_name)
Source-->>Migr: list of (key,value)
loop per key
Migr->>Migr: is_old_be_key(key)?
alt old BE
Migr->>Migr: convert_key_be_to_le(key)
Migr->>Source: delete_key(cf_name, old_key)
Source-->>Migr: ok
Migr->>Source: put_key(cf_name, new_key, value)
Source-->>Migr: ok
Migr->>Migr: stats.migrated += 1
else already LE / skipped
Migr->>Migr: stats.skipped/already_new += 1
end
end
Migr-->>Tool: MigrationStats
Tool->>CLI: print report
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
hageshiame
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/storage/src/custom_comparator.rs (1)
277-284: Runcargo fmtto fix formatting issues.Static analysis indicates formatting differences in the test code. Run
cargo fmtto align with project formatting standards and clear CI warnings.src/storage/src/zset_score_key_format.rs (2)
686-699: Consider adding a comment about bytewise comparison assumption.The test
test_seek_key_range_query_simulationassertsseek1 < seek2 < seek3using bytewise comparison (<onBytesMut). This works because:
- Keys have the same prefix
- Little-endian scores happen to sort correctly for these positive values
However, bytewise comparison of little-endian encoded f64 doesn't preserve numeric order in general (e.g., negative numbers). The custom comparator handles this, but direct bytewise comparison in tests could be misleading.
Consider using the actual comparator for clarity:
// seek key 应该按 score 排序 - assert!(seek1 < seek2); - assert!(seek2 < seek3); + use crate::custom_comparator::zsets_score_key_compare; + assert_eq!(zsets_score_key_compare(&seek1, &seek2), std::cmp::Ordering::Less); + assert_eq!(zsets_score_key_compare(&seek2, &seek3), std::cmp::Ordering::Less);
751-758: Minor: Redundant infinity check.The condition
parsed.score().is_infinite() == score.is_infinite()will always be true whenscore.is_infinite()is true.Simplify to:
if score.is_infinite() { - assert_eq!(parsed.score().is_infinite(), score.is_infinite()); + assert!(parsed.score().is_infinite()); assert_eq!(parsed.score().is_sign_positive(), score.is_sign_positive()); } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/storage/src/custom_comparator.rs(4 hunks)src/storage/src/member_data_key_format.rs(3 hunks)src/storage/src/storage_define.rs(1 hunks)src/storage/src/zset_score_key_format.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/storage/src/member_data_key_format.rs (4)
src/storage/src/storage_define.rs (2)
encode_user_key(69-88)encoded_user_key_len(63-67)src/storage/src/zset_score_key_format.rs (2)
encode_seek_key(237-251)version(426-428)src/storage/src/base_value_format.rs (1)
version(203-205)src/storage/src/coding.rs (1)
from_le_bytes(22-22)
src/storage/src/custom_comparator.rs (2)
src/storage/src/member_data_key_format.rs (3)
new(42-50)new(99-135)encode(52-68)src/storage/src/zset_score_key_format.rs (6)
new(71-73)new(162-171)new(309-406)encode(190-208)score(448-450)member(437-439)
🪛 GitHub Actions: ci
src/storage/src/member_data_key_format.rs
[warning] 55-73: Minor formatting adjustments in member_data_key_format tests (comment alignment and spacing). Align with project formatting to avoid diffs.
src/storage/src/custom_comparator.rs
[warning] 277-429: Test diffs detected across multiple zsets score key comparison tests due to formatting changes. Test inputs/line breaks have been reformatted (encoding().unwrap() calls etc.). Update tests or run formatter to align with repository standards.
src/storage/src/zset_score_key_format.rs
[warning] 194-469: Extensive formatting changes detected in zset_score_key_format tests and associated encoding/decoding logic. Tests expect specific spacing and alignment; update code/tests or re-run formatter to ensure consistency.
🪛 GitHub Check: build and test (ubuntu-latest)
src/storage/src/zset_score_key_format.rs
[failure] 375-375:
no function or associated item named caller found for struct snafu::Location in the current scope
[failure] 359-359:
no function or associated item named caller found for struct snafu::Location in the current scope
🪛 GitHub Check: cargo clippy (macos-latest)
src/storage/src/zset_score_key_format.rs
[failure] 375-375:
no function or associated item named caller found for struct snafu::Location in the current scope
[failure] 359-359:
no function or associated item named caller found for struct snafu::Location in the current scope
🪛 GitHub Check: cargo clippy (ubuntu-latest)
src/storage/src/zset_score_key_format.rs
[failure] 375-375:
no function or associated item named caller found for struct snafu::Location in the current scope
[failure] 359-359:
no function or associated item named caller found for struct snafu::Location in the current scope
🪛 GitHub Check: cargo fmt (macos-latest)
src/storage/src/custom_comparator.rs
[warning] 361-361:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 355-355:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 346-346:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 335-335:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 321-321:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 315-315:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 309-309:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 300-300:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 286-286:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 277-277:
Diff in /Users/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
🪛 GitHub Check: cargo fmt (ubuntu-latest)
src/storage/src/custom_comparator.rs
[warning] 361-361:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 355-355:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 346-346:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 335-335:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 321-321:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 315-315:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 309-309:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 300-300:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 286-286:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
[warning] 277-277:
Diff in /home/runner/work/kiwi/kiwi/src/storage/src/custom_comparator.rs
🪛 GitHub Check: lint
src/storage/src/zset_score_key_format.rs
[failure] 379-379:
no function or associated item named caller found for struct snafu::Location in the current scope
[failure] 363-363:
no function or associated item named caller found for struct snafu::Location in the current scope
🪛 GitHub Check: test
src/storage/src/zset_score_key_format.rs
[failure] 375-375:
no function or associated item named caller found for struct snafu::Location in the current scope
[failure] 359-359:
no function or associated item named caller found for struct snafu::Location in the current scope
🪛 GitHub Check: test-macos
src/storage/src/zset_score_key_format.rs
[failure] 375-375:
no function or associated item named caller found for struct snafu::Location in the current scope
[failure] 359-359:
no function or associated item named caller found for struct snafu::Location in the current scope
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-windows
🔇 Additional comments (7)
src/storage/src/storage_define.rs (1)
49-67: LGTM! Clean helper function for precise capacity estimation.The function correctly calculates the encoded length by accounting for the 0x00 → 0x00 0x01 transformation and the 2-byte delimiter. This aligns with
encode_user_keybehavior.src/storage/src/member_data_key_format.rs (3)
71-85: LGTM! Consistent with encode() changes.The seek key encoding correctly mirrors the full encode() method, using little-endian for version and precise capacity calculation.
114-117: LGTM! Decoding matches encoding endianness.The
from_le_bytesusage correctly matches theput_u64_leused in encoding.
52-68: Verify backward compatibility with existing stored data.The switch from big-endian to little-endian encoding for the
versionfield is a breaking change. If there's existing persisted data in RocksDB using the old format, it will be misinterpreted.Please confirm one of the following:
- This is a new data format with no existing stored data
- A migration strategy exists for existing data
- The format was already little-endian (and this is just making it explicit)
src/storage/src/custom_comparator.rs (2)
219-234: LGTM! Robust NaN handling.The NaN comparison logic correctly:
- Treats NaN as greater than any non-NaN value (including infinity)
- Falls through to member comparison when both scores are NaN
This ensures deterministic, total ordering for sorted set keys.
107-108: Verifydecode_fixeduses little-endian decoding.The encoding uses
put_u64_le(little-endian), butdecode_fixed::<u64>is used for decoding. Ensuredecode_fixedreads in little-endian format to match, otherwise version/index comparisons will be incorrect.src/storage/src/zset_score_key_format.rs (1)
190-208: LGTM! Clean encoding implementation.The encoding correctly:
- Uses
encoded_user_key_lenfor precise capacity allocation- Stores version and score in little-endian format
- Preserves f64 score as raw IEEE 754 bits via
to_bits()
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/storage/src/zset_score_key_format.rs (4)
356-361: Simplify: Useexpect()since conversion cannot fail after validation.The bounds check at lines 349-354 already ensures
version_sliceis exactly 8 bytes, sotry_into()cannot fail. The manual error construction is unnecessary complexity.- let version = u64::from_le_bytes( - version_slice.try_into().map_err(|_| crate::error::Error::InvalidFormat { - message: "Failed to parse version bytes".to_string(), - location: snafu::Location::new(file!(), line!(), 0), - })? - ); + let version = u64::from_le_bytes( + version_slice.try_into().expect("version slice validated to be 8 bytes") + );
372-377: Simplify: Same issue for score parsing.The bounds check at lines 365-370 guarantees
score_sliceis exactly 8 bytes.- let score_bits = u64::from_le_bytes( - score_slice.try_into().map_err(|_| crate::error::Error::InvalidFormat { - message: "Failed to parse score bytes".to_string(), - location: snafu::Location::new(file!(), line!(), 0), - })? - ); + let score_bits = u64::from_le_bytes( + score_slice.try_into().expect("score slice validated to be 8 bytes") + );
457-458: Consider using English for code comments.The test comments use Chinese (e.g., "基础编码/解码测试", "测试包含 \x00 的 key 和 member"). For consistency and broader contributor accessibility, consider using English throughout.
806-808: Minor: Capacity equality assertion may be fragile.
BytesMut::with_capacity()guarantees at least the requested capacity, but may allocate more depending on the allocator. The assertionencoded.capacity() == expected_lencould fail on some platforms or with different allocator configurations.Consider using
>=instead, or remove the capacity assertion since length equality already validates correctness:- assert_eq!(encoded.capacity(), expected_len, - "Overcapacity allocation for key={:?}, member={:?}", key, member); + // Capacity is at least expected_len; exact match depends on allocator + assert!(encoded.capacity() >= expected_len, + "Undercapacity allocation for key={:?}, member={:?}", key, member);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/storage/src/member_data_key_format.rs(4 hunks)src/storage/src/zset_score_key_format.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/storage/src/member_data_key_format.rs (4)
src/storage/src/storage_define.rs (3)
decode_user_key(90-138)encode_user_key(69-88)encoded_user_key_len(63-67)src/storage/src/zset_score_key_format.rs (2)
encode_seek_key(237-251)version(426-428)src/storage/src/base_value_format.rs (1)
version(203-205)src/storage/src/coding.rs (1)
from_le_bytes(22-22)
src/storage/src/zset_score_key_format.rs (2)
src/storage/src/storage_define.rs (3)
encode_user_key(69-88)encoded_user_key_len(63-67)seek_userkey_delim(140-149)src/storage/src/member_data_key_format.rs (6)
new(42-50)new(99-135)encode(52-68)encode_seek_key(71-85)version(141-143)key(137-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: cargo clippy (windows-latest)
- GitHub Check: build and test (windows-latest)
- GitHub Check: build and test (ubuntu-latest)
- GitHub Check: build and test (macos-latest)
- GitHub Check: test-macos
- GitHub Check: test
- GitHub Check: test-windows
🔇 Additional comments (6)
src/storage/src/member_data_key_format.rs (3)
52-68: LGTM!The capacity estimation using
encoded_user_key_lenis now precise, accounting for zero-byte escaping and the delimiter. This eliminates unnecessary reallocations.
150-237: LGTM!Tests properly validate encode/decode roundtrips including edge cases with empty keys, zero bytes in keys, and correct length calculations.
63-64: Breaking change: Verify data migration plan for endianness change.The switch from big-endian (
put_u64/from_be_bytes) to little-endian (put_u64_le/from_le_bytes) for the version field is a breaking change that affects existing stored data.Ensure:
- All production environments are aware of this incompatibility
- A migration tool or procedure exists to convert existing keys
- The change is documented in release notes
Also applies to: 81-82, 114-117
src/storage/src/zset_score_key_format.rs (3)
190-208: LGTM!The
encode()implementation correctly usesencoded_user_key_lenfor precise capacity estimation and stores the score as IEEE 754 bits in little-endian format.
309-406: LGTM!The parsing implementation has thorough bounds checking with descriptive error messages. The validation sequence properly ensures each slice access is safe.
619-637: LGTM!Good test verifying that
+0.0and-0.0produce different bit representations in encoding, while noting that the custom comparator treats them as equal during comparison.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/storage/src/zset_score_key_format.rs (2)
356-361: Critical: Remove unnecessary error mapping after bounds validation.The
ensure!at lines 349-354 already validates thatversion_sliceis exactly 8 bytes, sotry_into()cannot fail. The.map_err()clause is dead code, and the use ofsnafu::Location::new(file!(), line!(), column!())may not be a valid snafu API (similar to thesnafu::Location::caller()issue flagged in past reviews).Apply this diff to use the infallible conversion:
- let version = u64::from_le_bytes( - version_slice.try_into().map_err(|_| crate::error::Error::InvalidFormat { - message: "Failed to parse version bytes".to_string(), - location: snafu::Location::new(file!(), line!(), column!()), - })? - ); + let version = u64::from_le_bytes( + version_slice.try_into().expect("version slice validated to be 8 bytes") + );
372-377: Critical: Remove unnecessary error mapping for score parsing.Same issue as the version parsing above. The
ensure!at lines 365-370 guaranteesscore_sliceis exactly 8 bytes, making thetry_into()infallible.Apply this diff:
- let score_bits = u64::from_le_bytes( - score_slice.try_into().map_err(|_| crate::error::Error::InvalidFormat { - message: "Failed to parse score bytes".to_string(), - location: snafu::Location::new(file!(), line!(), column!()), - })? - ); + let score_bits = u64::from_le_bytes( + score_slice.try_into().expect("score slice validated to be 8 bytes") + );
🧹 Nitpick comments (1)
src/storage/src/member_data_key_format.rs (1)
114-117: Consider using.expect()for clearer panic messages.The
unwrap()at line 117 is safe due to the bounds check, but using.expect("version slice should be exactly 8 bytes")would provide a clearer panic message if the invariant is somehow violated.- let version = u64::from_le_bytes(version_slice.try_into().unwrap()); + let version = u64::from_le_bytes( + version_slice.try_into().expect("version slice should be exactly 8 bytes") + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/raft/src/network.rs(2 hunks)src/storage/src/member_data_key_format.rs(4 hunks)src/storage/src/zset_score_key_format.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/storage/src/zset_score_key_format.rs (2)
src/storage/src/storage_define.rs (4)
decode_user_key(90-138)encode_user_key(69-88)encoded_user_key_len(63-67)seek_userkey_delim(140-149)src/storage/src/custom_comparator.rs (1)
zsets_score_key_compare(168-244)
🪛 GitHub Check: cargo fmt (macos-latest)
src/raft/src/network.rs
[warning] 406-406:
Diff in /Users/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 399-399:
Diff in /Users/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 438-438:
Diff in /Users/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 431-431:
Diff in /Users/runner/work/kiwi/kiwi/src/raft/src/network.rs
🪛 GitHub Check: cargo fmt (ubuntu-latest)
src/raft/src/network.rs
[warning] 406-406:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 399-399:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 438-438:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 431-431:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: test
- GitHub Check: build and test (windows-latest)
- GitHub Check: test-macos
- GitHub Check: test-windows
- GitHub Check: lint
- GitHub Check: cargo clippy (ubuntu-latest)
- GitHub Check: build and test (macos-latest)
- GitHub Check: cargo clippy (windows-latest)
- GitHub Check: cargo clippy (macos-latest)
- GitHub Check: build and test (ubuntu-latest)
🔇 Additional comments (11)
src/storage/src/member_data_key_format.rs (3)
24-26: LGTM: Import ofencoded_user_key_lensupports precise capacity estimation.The addition of
encoded_user_key_lenenables accurate memory allocation in encoding methods, reducing potential over-allocation.
70-86: Capacity optimization approved; byte-order change covered above.The precise capacity calculation using
encoded_user_key_len()is correct. The little-endian change at line 82 is part of the same breaking change flagged in the previous comment.
150-238: LGTM: Tests properly verify the updated encoding scheme.The tests correctly compute expected lengths using the actual encoded key length and verify round-trip integrity. Edge cases (empty keys/data, special characters) are well covered.
src/storage/src/zset_score_key_format.rs (8)
28-141: LGTM: Excellent documentation quality.The comprehensive rustdoc comments clearly explain the binary format, ordering semantics, special floating-point value handling, and design rationale. This will greatly aid future maintainers.
190-208: LGTM: Encoding logic with precise capacity estimation.The use of
encoded_user_key_len()for exact capacity calculation is correct and avoids over-allocation. Little-endian encoding for version and score aligns with the custom comparator requirements.
237-251: LGTM: Seek key encoding is correct for range queries.The seek key correctly excludes
memberandreserve2fields, which is appropriate for range query positioning. The precise capacity calculation is optimal.
408-450: LGTM: Accessor methods are well-documented and straightforward.The getter methods provide clear access to parsed fields with appropriate documentation.
453-533: LGTM: Basic encoding/decoding tests are comprehensive.The tests cover round-trip encoding, special characters (null bytes), empty values, and verify exact encoded length using
encoded_user_key_len(). Good coverage of fundamental functionality.
535-637: LGTM: Excellent coverage of boundary cases and special values.The tests comprehensively verify:
- Special floating-point values (±infinity, NaN)
- Version boundaries (0, u64::MAX)
- Large keys/members
- Negative vs. positive zero (different IEEE 754 bit patterns)
This ensures robust handling of edge cases.
639-700: LGTM: Seek key tests properly use custom comparator.The tests correctly verify:
- Seek key format and length
- Exclusion of
memberandreserve2fields- Ordering using
zsets_score_key_compare(lines 697-699)The use of the custom comparator at lines 697-699 properly addresses the past review comment about bytewise comparison being invalid for little-endian encoded scores.
702-899: LGTM: Comprehensive tests for data integrity, performance, and error handling.Excellent test coverage including:
- Multi-member data integrity (lines 705-728)
- Various score encodings including infinities (lines 731-760)
- Reserve field verification (lines 763-777)
- Exact capacity validation with no over-allocation (lines 808-809, 833-834)
- Error handling for malformed input (lines 840-886)
- NaN handling (lines 889-898)
The capacity tests at lines 808-809 and 833-834 specifically verify that
BytesMutcapacity equals the encoded length, confirming the optimization works as intended.
hageshiame
force-pushed
the
issue-141
branch
from
cc62624 to
dbe98e2
Compare
…complex error handling
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/raft/src/network.rs (2)
399-406: Critical: Memory leak viaString::leak()persists.This critical memory leak issue was already flagged in a previous review. The
String::leak()calls on lines 401 and 405 permanently leak memory on every heartbeat, causing unbounded memory growth and eventual OOM in long-running systems.Please apply the suggested fix from the previous review: extract a helper method
RaftMessage::hmac_repr(&self) -> Stringthat returns an ownedStringwithout leaking, then use it in theformat!macro.
431-438: Same memory leak issue duplicated here.This duplicates the
String::leak()problem fromadd_authentication. As suggested in the previous review, extract a sharedRaftMessage::hmac_repr()helper method to eliminate both the leak and the code duplication:impl RaftMessage { fn hmac_repr(&self) -> String { match self { RaftMessage::AppendEntries(_) => "AppendEntries".to_string(), RaftMessage::AppendEntriesResponse(_) => "AppendEntriesResponse".to_string(), RaftMessage::Vote(_) => "Vote".to_string(), RaftMessage::VoteResponse(_) => "VoteResponse".to_string(), RaftMessage::InstallSnapshot(_) => "InstallSnapshot".to_string(), RaftMessage::InstallSnapshotResponse(_) => "InstallSnapshotResponse".to_string(), RaftMessage::Heartbeat { from, term } => format!("Heartbeat:{}:{}", from, term), RaftMessage::HeartbeatResponse { from, success } => { format!("HeartbeatResponse:{}:{}", from, success) } } } }Then both methods use:
let data_for_hmac = format!( "{}:{}:{}:{}:{}", self.message_id, self.from, self.to, self.timestamp, self.message.hmac_repr() );src/storage/src/member_data_key_format.rs (1)
52-68: Data encoding inconsistency persists in redis_sets.rs (see past review).The
encode()method now usesput_u64_lefor little-endian version encoding, but the past review comment correctly identified that several functions inredis_sets.rs(srem,sdiffstore,clear_and_store_set) still manually construct member keys using big-endianput_u64. This creates a data corruption risk where deletes will fail to match stored keys.The previous review provided the exact fix needed. Ensure those three functions are updated to use
MemberDataKey::new().encode()instead of manual construction.
🧹 Nitpick comments (2)
src/storage/src/zset_score_key_format.rs (2)
349-361: Simplify: Slice length validated, try_into() cannot fail.The bounds check ensures
version_end_idx <= end_idx, guaranteeing the version slice is exactly 8 bytes. The subsequenttry_into().map_err(...)construction is unnecessary complexity since the conversion cannot fail after validation.Replace the fallible conversion with an infallible one:
- let version = u64::from_le_bytes( - version_slice.try_into().map_err(|_| crate::error::Error::InvalidFormat { - message: "Failed to parse version bytes".to_string(), - location: snafu::Location::new(file!(), line!(), column!()), - })? - ); + let version = u64::from_le_bytes( + version_slice.try_into().expect("version slice validated to be 8 bytes") + );
365-378: Simplify: Slice length validated, try_into() cannot fail.The bounds check guarantees the score slice is exactly 8 bytes. Similar to the version parsing, the
try_into().map_err(...)is unnecessary.Replace with:
- let score_bits = u64::from_le_bytes( - score_slice.try_into().map_err(|_| crate::error::Error::InvalidFormat { - message: "Failed to parse score bytes".to_string(), - location: snafu::Location::new(file!(), line!(), column!()), - })? - ); + let score_bits = u64::from_le_bytes( + score_slice.try_into().expect("score slice validated to be 8 bytes") + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/raft/src/network.rs(2 hunks)src/storage/src/member_data_key_format.rs(4 hunks)src/storage/src/zset_score_key_format.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/storage/src/member_data_key_format.rs (4)
src/storage/src/storage_define.rs (2)
encode_user_key(69-88)encoded_user_key_len(63-67)src/storage/src/zset_score_key_format.rs (2)
encode_seek_key(237-251)version(426-428)src/storage/src/base_value_format.rs (1)
version(203-205)src/storage/src/coding.rs (1)
from_le_bytes(22-22)
src/storage/src/zset_score_key_format.rs (2)
src/storage/src/storage_define.rs (4)
decode_user_key(90-138)encode_user_key(69-88)encoded_user_key_len(63-67)seek_userkey_delim(140-149)src/storage/src/custom_comparator.rs (1)
zsets_score_key_compare(168-244)
🪛 GitHub Check: cargo fmt (ubuntu-latest)
src/raft/src/network.rs
[warning] 406-406:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 399-399:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 438-438:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
[warning] 431-431:
Diff in /home/runner/work/kiwi/kiwi/src/raft/src/network.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: cargo clippy (windows-latest)
- GitHub Check: build and test (ubuntu-latest)
- GitHub Check: build and test (windows-latest)
- GitHub Check: build and test (macos-latest)
- GitHub Check: test-windows
- GitHub Check: test-macos
- GitHub Check: test
🔇 Additional comments (20)
src/storage/src/member_data_key_format.rs (4)
53-58: Capacity estimation correctly uses encoded_user_key_len.The capacity calculation now uses
encoded_user_key_len(&self.key)for precise encoded key length, which properly accounts for null-byte encoding overhead. This optimization eliminates unnecessary reallocation during encoding.
70-85: LGTM: encode_seek_key uses consistent little-endian encoding.The seek key encoding correctly uses
encoded_user_key_lenfor capacity calculation andput_u64_lefor version encoding, maintaining consistency with the full encode method.
114-117: Safe unwrap after implicit size validation.The version slice is extracted with
size_of::<u64>()from line 115, guaranteeing it's exactly 8 bytes. Theunwrap()ontry_into()cannot fail since the slice length is validated by construction.
156-208: Thorough roundtrip test validates little-endian migration.The test correctly validates encode/decode roundtrip including keys with null bytes, and verifies the encoded length matches the expected calculation. The manual delimiter search at lines 197-203 provides additional validation that the data region is correctly positioned.
src/storage/src/zset_score_key_format.rs (16)
28-74: Comprehensive documentation for ScoreMember.The rustdoc provides clear description of ScoreMember's role in sorted set operations, including the relationship between scores and member ordering. The examples enhance usability.
76-141: Excellent documentation of ZSetsScoreKey binary format.The binary format documentation clearly describes field layout, ordering semantics via custom comparator, and special value handling (infinity, NaN, ±0.0). This level of detail is essential for a storage format implementation.
190-208: encode() correctly uses encoded_user_key_len for capacity.The capacity calculation uses
encoded_user_key_len(&self.key)for precise encoded key length, matching the optimization in member_data_key_format.rs. Little-endian encoding for version and score bits aligns with the custom comparator's expectations.
237-251: encode_seek_key optimized for range queries.The seek key omits member and reserve2 fields, reducing key size for range query prefix seeks. The capacity calculation and encoding logic correctly match the documented seek key format.
316-326: Good: Validation ensures minimum encoded key length.The minimum length check validates that the encoded key has enough bytes for all required fields, with a descriptive error message showing actual vs. expected length.
338-343: Good: Bounds check prevents out-of-range delimiter access.The check ensures the delimiter position doesn't exceed the data region boundary before attempting to decode the user key.
386-394: Good: reserve2 length validation prevents buffer overruns.The check ensures reserve2 is exactly SUFFIX_RESERVE_LENGTH bytes, with a descriptive error message, preventing silent data corruption from malformed keys.
460-474: Solid: Basic roundtrip test validates encode/decode correctness.The test verifies all fields are preserved through encode/decode, providing baseline correctness validation.
477-492: Good: Test validates null-byte handling in keys and members.The test uses keys and members containing
\x00bytes, verifying that the special encoding/decoding logic inencode_user_keyanddecode_user_keycorrectly handles theNEED_TRANSFORM_CHARACTER.
512-533: Excellent: Test validates precise capacity estimation.The test computes expected length using
encode_user_keyand verifies it matches the actual encoded length, confirming thatencoded_user_key_lenprovides accurate capacity estimation.
538-566: Thorough: Tests validate special float values (infinity, NaN).The tests verify that
f64::INFINITY,f64::NEG_INFINITY, andf64::NANare correctly encoded and decoded, which is critical for score-based ordering in sorted sets.
620-637: Good: Test documents +0.0 vs -0.0 bit representation difference.The test verifies that 0.0 and -0.0 have different bit representations and produce different encodings. This is correct behavior since the custom comparator handles these as equal during comparison while preserving the bit-level distinction in storage.
642-667: Good: Test validates seek key format and prefix relationship.The test verifies that the seek key is shorter than the full key and that it forms a valid prefix, confirming the seek key can be used for range query operations.
686-700: Test correctly uses custom comparator for ordering.The test now uses
zsets_score_key_compareto validate seek key ordering, fixing the previous bytewise comparison issue. This correctly validates that the comparator orders keys by score numerically.
782-811: Excellent: Performance test validates zero-overhead capacity estimation.The test verifies that
encoded.capacity()equalsexpected_len, confirming no over-allocation. This validates thatencoded_user_key_leneliminates the need forBytesMutto resize during encoding, optimizing memory usage and performance.
840-859: Good: Error handling tests validate parse robustness.The tests verify that too-short inputs, incomplete data, and missing version fields all return errors rather than panicking, ensuring graceful failure on corrupted data.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/storage/src/zset_score_key_format.rs (1)
451-451: Consider translating Chinese comments for consistency.Several test section headers and inline comments are in Chinese (e.g., "基础编码/解码测试", "测试包含 \x00 的 key 和 member"). If the codebase is primarily English, consider translating these for consistency and accessibility to all contributors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/storage/src/zset_score_key_format.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/storage/src/zset_score_key_format.rs (3)
src/storage/src/storage_define.rs (3)
encode_user_key(69-88)encoded_user_key_len(63-67)seek_userkey_delim(140-149)src/storage/src/coding.rs (1)
from_le_bytes(22-22)src/storage/src/custom_comparator.rs (1)
zsets_score_key_compare(168-244)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build and test (macos-latest)
- GitHub Check: build and test (windows-latest)
- GitHub Check: test
- GitHub Check: test-macos
- GitHub Check: test-windows
🔇 Additional comments (10)
src/storage/src/zset_score_key_format.rs (10)
28-74: LGTM! Well-documented ScoreMember struct.The documentation is thorough with clear field descriptions and usage examples. The struct design is clean and appropriate for representing score-member pairs.
76-141: LGTM! Excellent documentation of the binary format.The documentation clearly explains the byte layout, ordering semantics, and special value handling. The design rationale for reserve fields and little-endian encoding provides good context for maintainers.
190-208: LGTM! Precise capacity estimation and correct encoding.The use of
encoded_user_key_len()ensures exact buffer allocation without over-allocation, and the little-endian encoding is consistent with the custom comparator's expectations.
237-251: LGTM! Seek key implementation for range queries.The seek key format correctly omits member and reserve2 for efficient range-based iteration, with precise capacity estimation.
356-371: Theexpect()calls are safe given prior validation.The length checks via
ensure!at lines 349-354 and 362-367 guarantee that the slices are exactly 8 bytes, making thetry_into().expect()infallible. This correctly addresses the previous review feedback aboutsnafu::Location::caller().
402-445: LGTM! Clean accessor methods.The accessor methods provide convenient read-only access to the parsed fields with appropriate documentation.
679-694: Good fix: Using custom comparator for seek key ordering.The test correctly uses
zsets_score_key_compareinstead of bytewise comparison, which addresses the previous review feedback. Little-endian score bits don't preserve numeric order bytewise, so the custom comparator is essential for correct ordering verification.
470-486: LGTM! Good coverage of null byte handling.The test correctly validates that keys with
\x00bytes are properly encoded/decoded (via the transformation), while members with\x00are stored raw and preserved correctly.
775-830: Excellent capacity verification tests.The tests at lines 775-830 validate that the capacity estimation via
encoded_user_key_len()is precise with no over-allocation. This confirms the performance optimization mentioned in the PR objectives.
690-693: Remove this review comment - the flagged issue does not exist.Both calls on lines 180-181 of
custom_comparator.rsuse the same functionseek_userkey_delimconsistently. There is no function namedseek_user_delimin the codebase, and no inconsistency in the custom comparator.Likely an incorrect or invalid review comment.
|
问题确认与修复 确认问题：member_data_key_format.rs 的字节序变更（BE → LE）是 breaking change Rustfn migrate_db(db: &RocksDB) -> Result<()> { 确认问题：zset_score_key_format.rs 中的容量断言可能失败 Rust// 改为 确认问题：测试注释中英文混用 确认问题：redundant infinity check Rustif score.is_infinite() { 确认问题：缺少性能基准测试 Rust#[bench] #[bench] |
问题 将 member_data_key_format 中 version 从 big-endian 改成 little-endian 是一个破坏性变更。如果集群已有持久化数据，这会导致现有 key 无法被正确解释。 建议 明确在 PR / release note 里写出： 这个变更会让现有数据失效； 必须提供数据迁移工具或说明； 或者只在全新集群下默认启用。 理由 一致性变更会影响存储层对已有生产数据的解释，需有回退与迁移方案。 ❗️2. 测试中使用字节比较的精确性 部分测试中（如 <seek1 < seek2 < seek3>）用的是字节直接比较，而不是用 comparator 实际逻辑比较： 潜在问题 f64 的 little-endian 字节顺序不能自然表示数值排序（尤其包含负数、NaN 情况时）。 建议 用 zsets_score_key_compare 测试排序，而不是 seek1 < seek2。 这样测试更贴近业务行为，而不仅是字节 layout 假设。 🧹3. snafu::Location::caller API 问题 CI 报错： no function or associated item named 说明使用的错误处理构造在当前版本 snafu 库里不可用或不匹配。 建议 将 .Location::caller() 替换为更稳定的 API（例如使用宏或手动 location 信息）。 或者降级改用 expect() 和更明确的 panic 说明（因为 bounds 检查之后不会失败）。 🌐4. 错误信息的本地化/国际化 当前部分测试注释使用中文。对于开源项目建议： 所有代码/测试注释使用 英文，提升全球协作体验。 保证错误信息对外输出友好。 这在 PR 中也被 CodeRabbit reviewer 捕捉到并建议过。 🧪5. 测试覆盖与边界条件 优点： PR 添加了大量 edge case 测试（NaN / Inf / ±0 / empty member / boundary lengths） 提升了约定行为清晰度和稳定性。 建议： 把一些示例 test case 提出到 doc tests 或 integration test 目录，更贴近接口使用行为验证。避免所有测试集中在内部 module。 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/tools/migrate_db_member_format.rs (1)
100-109: PreferFromtrait overIntofor conversions.Implementing
From<LogLevel> for log::LevelFilteris preferred overInto<log::LevelFilter> for LogLevel. The standard library automatically provides the reciprocalIntoimplementation when you implementFrom.🔄 Refactor to use From trait
-impl Into<log::LevelFilter> for LogLevel { - fn into(self) -> log::LevelFilter { +impl From<LogLevel> for log::LevelFilter { + fn from(level: LogLevel) -> log::LevelFilter { match self { LogLevel::Debug => log::LevelFilter::Debug, LogLevel::Info => log::LevelFilter::Info, LogLevel::Warn => log::LevelFilter::Warn, LogLevel::Error => log::LevelFilter::Error, } } }src/storage/src/zset_score_key_format.rs (1)
534-534: Replace Chinese test comments with English for broader accessibility.Test section comments and inline comments use Chinese (e.g., "基础编码/解码测试", "边界值测试"). Per PR feedback and coding guidelines, unify to English for global readability.
Example translations
- // ========== 基础编码/解码测试 ========== + // ========== Basic encoding/decoding tests ========== - // ========== 边界值测试 ========== + // ========== Boundary value tests ========== - // ========== encode_seek_key unit tests ========== // (already in English, keep as-is) - // ========== Data integrity tests ========== // (already in English) - // ========== Performance verification tests ========== // (already in English) - // ========== Error handling tests ========== // (already in English)Also applies to: 612-612, 716-716, 796-796, 873-873, 943-943
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Cargo.tomlbenches/zset_score_key_benchmark.rssrc/storage/src/lib.rssrc/storage/src/migration.rssrc/storage/src/zset_score_key_format.rssrc/tools/migrate_db_member_format.rstests/zset_score_key_integration_tests.rs
🧰 Additional context used
🧬 Code graph analysis (5)
benches/zset_score_key_benchmark.rs (1)
src/storage/src/zset_score_key_format.rs (6)
key(492-494)member(514-516)new(139-141)new(230-239)new(383-483)score(525-527)
src/storage/src/lib.rs (1)
src/storage/src/migration.rs (3)
migrate_member_keys_be_to_le(347-423)verify_key_format(430-446)generate_migration_report(449-479)
src/tools/migrate_db_member_format.rs (3)
scripts/dev.sh (1)
info(31-31)src/storage/src/migration.rs (1)
new(74-76)src/storage/src/zset_score_key_format.rs (3)
new(139-141)new(230-239)new(383-483)
src/storage/src/migration.rs (1)
src/storage/src/storage_define.rs (3)
decode_user_key(90-138)encode_user_key(69-88)seek_userkey_delim(140-149)
src/storage/src/zset_score_key_format.rs (3)
src/storage/src/storage_define.rs (4)
decode_user_key(90-138)encode_user_key(69-88)encoded_user_key_len(63-67)seek_userkey_delim(140-149)src/storage/src/coding.rs (1)
from_le_bytes(22-22)src/storage/src/custom_comparator.rs (1)
zsets_score_key_compare(168-244)
🪛 GitHub Actions: ci
tests/zset_score_key_integration_tests.rs
[error] 1-1: License header check failed. File does not have a valid license header.
src/storage/src/zset_score_key_format.rs
[error] 49-49: make lint failed: Rust error[E0753]: expected outer doc comment at src/storage/src/zset_score_key_format.rs:49:1
🪛 GitHub Actions: Tests
src/storage/src/zset_score_key_format.rs
[error] 49-49: Rust compiler error E0753: expected outer doc comment in src/storage/src/zset_score_key_format.rs at line 49:1.
🪛 GitHub Check: build and test (macos-latest)
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: build and test (ubuntu-latest)
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: cargo clippy (ubuntu-latest)
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: cargo fmt (ubuntu-latest)
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: lint
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: test
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: test-macos
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: test-windows
src/storage/src/zset_score_key_format.rs
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🔇 Additional comments (7)
Cargo.toml (1)
68-68: LGTM! Criterion dependency appropriately added for benchmarking.The addition of
criterionwithhtml_reportsfeature enables the new benchmark suite and provides useful HTML report generation.src/storage/src/lib.rs (1)
41-41: LGTM! Migration module appropriately wired into public API.The new
migrationmodule and its public re-exports provide the necessary API surface for BE→LE key migration workflows.Also applies to: 79-79
tests/zset_score_key_integration_tests.rs (1)
18-302: Excellent test coverage with proper comparator usage.The integration tests comprehensively exercise ZSet score key formatting, comparison semantics, edge cases (NaN, infinity, null bytes, empty keys), and migration scenarios. The tests correctly use
zsets_score_key_comparefor ordering validation rather than relying on bytewise comparison.benches/zset_score_key_benchmark.rs (1)
18-198: Well-structured benchmark suite.The benchmarks properly exercise encoding, seek-key encoding, parsing, and round-trip operations across various input sizes and special values. The use of
black_boxprevents compiler optimizations from skewing results.src/tools/migrate_db_member_format.rs (1)
267-328: Placeholder implementations need wiring to storage migration APIs.The
create_backup,perform_migration, andverify_migrationfunctions are placeholders. These should be wired to the actual migration APIs provided bysrc/storage/src/migration.rs(e.g.,migrate_member_keys_be_to_le,verify_key_format) to make the tool functional.Do you want me to generate the integration code connecting this CLI tool to the storage migration module?
src/storage/src/migration.rs (1)
288-423: Well-designed migration orchestration with proper dry-run and error handling.The
migrate_key_in_memoryandmigrate_member_keys_be_to_lefunctions properly handle:
- Attempting LE parse first (no-op if already migrated)
- BE detection and conversion as fallback
- Dry-run mode for safe preview
- Detailed statistics tracking
- Graceful error handling with logging
src/storage/src/zset_score_key_format.rs (1)
530-1004: Excellent comprehensive test coverage.The test suite thoroughly validates:
- Encoding/decoding round-trips for various inputs
- Special values (NaN, ±Infinity, ±0.0, boundary values)
- Edge cases (empty keys/members, null bytes, large inputs)
- Exact capacity calculations with appropriate
>=checks- Seek key format and comparator-based ordering
- Error handling for malformed inputs
The tests properly use the custom comparator and avoid fragile capacity assertions (using
>=rather than==).
| fn is_old_be_key(key: &[u8]) -> Result<bool> { | ||
| // Minimum size: reserve1(8) + empty_key_delim(2) + version(8) + score(8) + reserve2(16) | ||
| if key.len() < PREFIX_RESERVE_LENGTH + ENCODED_KEY_DELIM_SIZE + 8 + 8 + SUFFIX_RESERVE_LENGTH { | ||
| return Ok(false); | ||
| } | ||
|
|
||
| // Find the version bytes (they come after the encoded key) | ||
| // This is a heuristic: we look for the position where version should be | ||
| // by finding the key delimiter (\x00\x00) | ||
| let mut delim_pos = None; | ||
| for i in 0..key.len().saturating_sub(1) { | ||
| if i >= PREFIX_RESERVE_LENGTH && key[i] == 0x00 && key[i + 1] == 0x00 { | ||
| delim_pos = Some(i + ENCODED_KEY_DELIM_SIZE); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if let Some(version_start) = delim_pos { | ||
| let version_end = version_start + 8; | ||
| if version_end <= key.len() { | ||
| let version_bytes = &key[version_start..version_end]; | ||
| // A heuristic check: if the first few bytes are zero (common in BE for small numbers) | ||
| // and the last bytes are non-zero, it's likely BE format | ||
| // This is not foolproof but works for most cases | ||
| if version_bytes[0..4] == [0, 0, 0, 0] && version_bytes[4] != 0 { | ||
| return Ok(true); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(false) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for version number definitions and usage
rg -n "version" src/storage/src/migration.rs -C2 | head -40Repository: arana-db/kiwi
Length of output: 1800
🏁 Script executed:
# Check if there are any version numbers that might exceed 2^32
rg -n "version\s*=\s*" --type rust | head -20Repository: arana-db/kiwi
Length of output: 1777
🏁 Script executed:
# Look at test cases in the migration module to understand expected version ranges
rg -n "#\[test\]|version_bytes|is_old_be_key" src/storage/src/migration.rs -C3 | head -50Repository: arana-db/kiwi
Length of output: 1949
🏁 Script executed:
# Check for any version constants or limits defined in the codebase
rg -n "VERSION|2\^32|4294967296" --type rust | head -20Repository: arana-db/kiwi
Length of output: 2128
🏁 Script executed:
# Check for version value constraints or documentation
rg -n "version.*range|version.*limit|MAX.*VERSION|u64" src/storage/src/migration.rs -C2 | head -30Repository: arana-db/kiwi
Length of output: 1094
🏁 Script executed:
# Look at the zset_score_key_format module to understand version encoding
fd "zset_score_key" --type fRepository: arana-db/kiwi
Length of output: 175
🏁 Script executed:
# Check what FORMAT_VERSION_BE and FORMAT_VERSION_LE actually are
rg -n "FORMAT_VERSION_(BE|LE)" --type rust -B2 -A2Repository: arana-db/kiwi
Length of output: 4681
🏁 Script executed:
# Check if is_old_be_key uses FORMAT_VERSION markers or only heuristics
rg -n "is_big_endian_format|FORMAT_VERSION|reserve1" src/storage/src/migration.rs -B2 -A2Repository: arana-db/kiwi
Length of output: 778
🏁 Script executed:
# Check if there are comments about version number limits in the codebase
rg -n "version.*constraint|version.*limit|2.*32" --type rust -C2 | head -30Repository: arana-db/kiwi
Length of output: 1879
🏁 Script executed:
# Examine the full is_old_be_key function and surrounding context
sed -n '120,160p' src/storage/src/migration.rsRepository: arana-db/kiwi
Length of output: 1711
🏁 Script executed:
# Check the verify_key_format function to understand the full detection logic
sed -n '430,455p' src/storage/src/migration.rsRepository: arana-db/kiwi
Length of output: 849
🏁 Script executed:
# Check if migration uses explicit FORMAT_VERSION detection elsewhere
rg -n "FORMAT_VERSION|is_big_endian|is_little_endian" src/storage/src/migration.rsRepository: arana-db/kiwi
Length of output: 39
🏁 Script executed:
# Check if there's any version constraint documentation in comments
rg -n "version" src/storage/src/zset_score_key_format.rs | grep -i "comment\|//" -A2 -B2 | head -40Repository: arana-db/kiwi
Length of output: 2472
Heuristic for BE format detection has a real limitation for large version numbers.
The check at line 148 (version_bytes[0..4] == [0, 0, 0, 0] && version_bytes[4] != 0) assumes version numbers are < 2^32 and will fail to detect BE format for versions ≥ 2^32 where the high-order bytes are non-zero.
Note: The codebase has explicit FORMAT_VERSION markers (0x00 for BE, 0x01 for LE) available in reserve1[0] that provide more reliable detection, but the migration logic relies on this heuristic instead. Consider either documenting the version number constraint or using the explicit format markers if this is a concern for your deployment.
🤖 Prompt for AI Agents
In src/storage/src/migration.rs around lines 124-155, the heuristic that checks
version_bytes[0..4] == [0,0,0,0] && version_bytes[4] != 0 fails for versions >=
2^32; replace this fragile size-based heuristic with the explicit format marker
stored in reserve1[0] (0x00 means BE, 0x01 means LE) when available: parse or
index into the key's prefix reserve area to read reserve1[0] and use that value
to determine endianness (falling back to the existing heuristic only if reserve1
is not present/valid), and ensure bounds checks so you never index out of range.
| //! # ZSet Score Key Format Module | ||
| //! | ||
| //! This module implements the binary encoding and decoding of ZSet (Sorted Set) score keys for RocksDB storage. | ||
| //! | ||
| //! ## Format Versioning and Backward Compatibility | ||
| //! | ||
| //! To support seamless migration from old (Big-Endian) to new (Little-Endian) formats, | ||
| //! we use a version marker stored in reserve1[0]: | ||
| //! - 0x00: Old format (v1.x, Big-Endian version bytes) | ||
| //! - 0x01: New format (v2.0+, Little-Endian version bytes) | ||
| //! - 0xFF: Default/uninitialized (treated as Big-Endian for compatibility) | ||
| //! | ||
| //! This allows the system to: | ||
| //! 1. Read keys in either format without manual data migration | ||
| //! 2. Gradually upgrade keys as they are written | ||
| //! 3. Detect format mismatches and provide clear error messages | ||
| //! | ||
| //! ## Breaking Change Notice: Byte Order Update (v2.0) | ||
| //! | ||
| //! **Issue**: Previous versions used Big-Endian (BE) byte order for version numbers. | ||
| //! **Change**: Version 2.0+ uses Little-Endian (LE) byte order for consistency and performance. | ||
| //! **Impact**: This is a **breaking change** affecting all existing ZSet data. | ||
| //! | ||
| //! ### Migration Required | ||
| //! | ||
| //! If upgrading from v1.x to v2.0+: | ||
| //! 1. **Backup your database** before upgrading | ||
| //! 2. **Run data migration** to convert existing keys from BE to LE format | ||
| //! 3. **Test thoroughly** in a staging environment first | ||
| //! | ||
| //! Migration can be performed using the `migrate_member_keys_be_to_le()` function or | ||
| //! the provided migration tool. See MIGRATION_GUIDE.md for detailed instructions. | ||
| //! | ||
| //! ### Format Details | ||
| //! | ||
| //! **Old Format (v1.x - BE):** | ||
| //! ```text | ||
| //! | reserve1[0]=0x00 | reserve1[1..] | key | version_BE | score | member | reserve2 | | ||
| //! | 1B | 7B | var | 8B | 8B | var | 16B | | ||
| //! ``` | ||
| //! | ||
| //! **New Format (v2.0+ - LE):** | ||
| //! ```text | ||
| //! | reserve1[0]=0x01 | reserve1[1..] | key | version_LE | score | member | reserve2 | | ||
| //! | 1B | 7B | var | 8B | 8B | var | 16B | | ||
| //! ``` | ||
There was a problem hiding this comment.
Critical: Module doc comment misplaced, causing build failures.
The module-level doc comment (lines 49-95) appears after public items (constants and functions at lines 22-47), causing compiler error E0753. Module doc comments (//!) must appear before any items in the module.
🔧 Reorder items to fix doc comment placement
Move the module doc comment (lines 49-95) to appear immediately after line 21 (after imports), before the constant declarations at line 22.
use bytes::{BufMut, Bytes, BytesMut};
+//! # ZSet Score Key Format Module
+//!
+//! This module implements the binary encoding and decoding of ZSet (Sorted Set) score keys for RocksDB storage.
+//! [... rest of module doc ...]
+
/// Format version marker for backward compatibility
/// This constant is used in reserve1[0] to detect and handle different key format versionsCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: ci
[error] 49-49: make lint failed: Rust error[E0753]: expected outer doc comment at src/storage/src/zset_score_key_format.rs:49:1
🪛 GitHub Actions: Tests
[error] 49-49: Rust compiler error E0753: expected outer doc comment in src/storage/src/zset_score_key_format.rs at line 49:1.
🪛 GitHub Check: build and test (macos-latest)
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: build and test (ubuntu-latest)
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: cargo clippy (ubuntu-latest)
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: cargo fmt (ubuntu-latest)
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: lint
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: test
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: test-macos
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🪛 GitHub Check: test-windows
[failure] 58-58:
expected outer doc comment
[failure] 57-57:
expected outer doc comment
[failure] 56-56:
expected outer doc comment
[failure] 55-55:
expected outer doc comment
[failure] 54-54:
expected outer doc comment
[failure] 53-53:
expected outer doc comment
[failure] 52-52:
expected outer doc comment
[failure] 51-51:
expected outer doc comment
[failure] 50-50:
expected outer doc comment
[failure] 49-49:
expected outer doc comment
🤖 Prompt for AI Agents
In src/storage/src/zset_score_key_format.rs around lines 49-95, the module-level
doc comment using //! is placed after public items which triggers compiler error
E0753; move the entire module doc comment block so it appears immediately after
the imports (right after line 21) and before any constant or function
declarations (before line 22) so the //! module docs precede all items.
| @@ -0,0 +1,302 @@ | |||
| //! Integration tests for ZSet score key format and comparator | |||
There was a problem hiding this comment.
Critical: Missing license header.
The pipeline failure indicates the file lacks a required license header. Add the Apache 2.0 license header consistent with other files in the codebase.
📋 Add license header
+// Copyright (c) 2024-present, arana-db Community. All rights reserved.
+//
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
//! Integration tests for ZSet score key format and comparator📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! Integration tests for ZSet score key format and comparator | |
| // Copyright (c) 2024-present, arana-db Community. All rights reserved. | |
| // | |
| // Licensed to the Apache Software Foundation (ASF) under one or more | |
| // contributor license agreements. See the NOTICE file distributed with | |
| // this work for additional information regarding copyright ownership. | |
| // The ASF licenses this file to You under the Apache License, Version 2.0 | |
| // (the "License"); you may not use this file except in compliance with | |
| // the License. You may obtain a copy of the License at | |
| // | |
| // http://www.apache.org/licenses/LICENSE-2.0 | |
| // | |
| // Unless required by applicable law or agreed to in writing, software | |
| // distributed under the License is distributed on an "AS IS" BASIS, | |
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |
| // See the License for the specific language governing permissions and | |
| // limitations under the License. | |
| //! Integration tests for ZSet score key format and comparator |
🧰 Tools
🪛 GitHub Actions: ci
[error] 1-1: License header check failed. File does not have a valid license header.
🤖 Prompt for AI Agents
In tests/zset_score_key_integration_tests.rs around line 1, the file is missing
the required Apache 2.0 license header; add the standard Apache 2.0 header used
across the repo at the very top of the file (before the //! doc comment),
matching exactly the format and years/authorship used in other source files—copy
the header from an existing file in the codebase and paste it as the first lines
to satisfy CI.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/storage/src/lib.rs (1)
78-78: Consider breaking long re-export line and evaluate public visibility of format version constants.This re-export line exceeds 200 characters, making it hard to read. More importantly, exposing
FORMAT_VERSION_LEandFORMAT_VERSION_BEconstants commits these implementation details to the public API. If users depend on these constants, future format changes become breaking changes.Consider:
- Split the re-export across multiple lines for readability
- Evaluate whether format version constants should be public or remain internal implementation details
- If format detection is needed externally, consider a higher-level API like
KeyFormatenum rather than exposing raw version bytes🔎 Suggested refactoring for readability
-pub use zset_score_key_format::{ScoreMember, ZsetScoreMember, ZSetsScoreKey, ParsedZSetsScoreKey, FORMAT_VERSION_LE, FORMAT_VERSION_BE, detect_format_version, is_little_endian_format, is_big_endian_format}; +pub use zset_score_key_format::{ + detect_format_version, is_big_endian_format, is_little_endian_format, + ParsedZSetsScoreKey, ScoreMember, ZSetsScoreKey, ZsetScoreMember, + FORMAT_VERSION_BE, FORMAT_VERSION_LE, +};
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/storage/src/lib.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/storage/src/lib.rs (2)
src/storage/src/zset_score_key_format.rs (3)
detect_format_version(34-36)is_little_endian_format(39-41)is_big_endian_format(44-47)src/storage/src/migration.rs (3)
migrate_member_keys_be_to_le(347-423)verify_key_format(430-446)generate_migration_report(449-479)
| pub mod base_meta_value_format; | ||
| pub mod base_value_format; | ||
| mod custom_comparator; | ||
| pub mod custom_comparator; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for public API documentation in newly exposed modules
echo "=== Checking custom_comparator documentation ==="
rg -A 2 "^pub (fn|struct|enum|trait|type|const)" src/storage/src/custom_comparator.rs | head -60
echo -e "\n=== Checking storage_define documentation ==="
rg -A 2 "^pub (fn|struct|enum|trait|type|const)" src/storage/src/storage_define.rs | head -60
echo -e "\n=== Checking zset_score_key_format documentation ==="
rg -A 2 "^pub (fn|struct|enum|trait|type|const)" src/storage/src/zset_score_key_format.rs | head -60Repository: arana-db/kiwi
Length of output: 3325
🏁 Script executed:
cat src/storage/src/storage_define.rsRepository: arana-db/kiwi
Length of output: 8913
🏁 Script executed:
sed -n '75,85p' src/storage/src/lib.rsRepository: arana-db/kiwi
Length of output: 514
Add documentation for custom_comparator functions and storage_define constants.
custom_comparator public functions (e.g., lists_data_key_comparator_name, zsets_score_key_comparator_name, lists_data_key_compare, zsets_score_key_compare) lack rustdoc comments. Similarly, storage_define exposes many public constants (e.g., PREFIX_RESERVE_LENGTH, VERSION_LENGTH, SUFFIX_RESERVE_LENGTH, TYPE_LENGTH) without documentation, making it unclear which are stable public API versus internal implementation details. zset_score_key_format is well-documented and follows the selective re-export pattern on lines 78-79, which should be applied consistently to all newly public modules.
🤖 Prompt for AI Agents
In src/storage/src/lib.rs around line 29, the pub mod custom_comparator
declaration exposes functions and storage_define constants without rustdoc and
unclear API boundaries; add concise rustdoc comments to every public function in
custom_comparator (e.g., lists_data_key_comparator_name,
zsets_score_key_comparator_name, lists_data_key_compare,
zsets_score_key_compare) explaining purpose, inputs, outputs, and stability, and
document each public constant in storage_define (e.g., PREFIX_RESERVE_LENGTH,
VERSION_LENGTH, SUFFIX_RESERVE_LENGTH, TYPE_LENGTH) clarifying whether it is a
stable public API or internal implementation detail; where constants or helpers
are internal, remove pub or make them module-private and instead provide a
documented, minimal public re-export surface (follow the zset_score_key_format
selective re-export pattern used on lines 78-79) so only intended APIs are
public and fully documented.
| mod slot_indexer; | ||
| mod statistics; | ||
| mod util; | ||
| mod migration; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the actual content around lines 41 and 79 in lib.rs
echo "=== Checking lib.rs around line 41 ==="
sed -n '35,50p' src/storage/src/lib.rs
echo -e "\n=== Checking lib.rs around line 79 ==="
sed -n '70,90p' src/storage/src/lib.rs
# Check if migration.rs exists
echo -e "\n=== Checking migration.rs existence ==="
ls -la src/storage/src/migration.rs 2>&1 || echo "File not found"
# Get file size to decide on reading strategy
wc -l src/storage/src/migration.rs 2>/dev/null || echo "Cannot determine file size"Repository: arana-db/kiwi
Length of output: 1146
🏁 Script executed:
#!/bin/bash
# Check migration module documentation (the first 50 lines including any module-level docs)
echo "=== Checking migration.rs module documentation ==="
head -50 src/storage/src/migration.rs
echo -e "\n=== Checking migration.rs function documentation ==="
rg -B 3 "pub fn (migrate_member_keys_be_to_le|verify_key_format|generate_migration_report)" src/storage/src/migration.rs -A 5 | head -100Repository: arana-db/kiwi
Length of output: 3051
Migration module structure is sound, but documentation lacks backup and rollback guidance.
The pattern of a private migration module with selective public re-exports is excellent. The module documentation correctly describes the three-stage migration process (Detection, Conversion, Verification) and includes a helpful example demonstrating the dry-run capability.
However, the documentation is incomplete. It covers when/why migration is needed and provides examples of dry-run and verification, but omits:
- Backup recommendations before running migration
- Rollback procedures if migration fails or needs to be reversed
- Specific error handling guidance for common failure scenarios
Add documentation covering these critical topics to ensure users can safely execute migrations in production environments.
🤖 Prompt for AI Agents
In src/storage/src/lib.rs around line 41, the module docs for the private
migration module are missing critical operational guidance; update the
module-level documentation to add (1) backup recommendations (create consistent
on-disk and DB snapshots, verify backups, store retention/rotation notes), (2)
rollback procedures (how to detect failure, steps to restore from backups,
transactional or migration-step-by-step reverse operations if supported, and how
to mark/retry migrations safely), and (3) specific error-handling guidance for
common failures (expected error types, retry/backoff guidance, logging details
to capture, and when to abort vs continue). Keep the additions concise, give
commands/examples for making/restoring backups and a short checklist to follow
before running migrations.
2 participants
Related to issue-141
Changelog
1. src/storage/src/zset_score_key_format.rs
Change Type: Feature Enhancement
Major Modifications:
Added complete rustdoc documentation (all public APIs)
Added 25+ unit tests
Basic encoding/decoding tests (4)
Boundary value tests (7)
encode_seek_key specific tests (3)
Data integrity tests (3)
Performance optimization verification tests (2)
Error handling tests (6)
Optimized capacity estimation (using
encoded_user_key_len())Enhanced
ParsedZSetsScoreKey::new()error handlingAdded input validation
Removed all
unwrap()Returned detailed error information
Line Count Change: +468 lines
Backward Compatibility: Yes (Enhancement only, no breaking changes)
2. src/storage/src/custom_comparator.rs
Change Type: Test and documentation enhancements
Major Changes:
Added detailed documentation for comparator functions
Added 15+ comparator tests
Basic sorting tests (5)
Special value sorting tests (2)
NaN handling tests (3)
Comprehensive sorting tests (5)
Line count change: +307 lines
Backward compatibility: Yes (New tests only)
3. src/storage/src/member_data_key_format.rs
Change Type: Breaking change (byte order fix)
Major Changes:
encode():put_u64()→put_u64_le()encode_seek_key():put_u64()→put_u64_le()ParsedMemberDataKey::new():from_be_bytes()→from_le_bytes()Optimized capacity estimation (using
encoded_user_key_len())Row count change: +9 rows, -8 rows
Backward compatibility: No (Data migration required)
Affected scope: Hash/Set/ZSet member data
Migration recommendations:
4. src/storage/src/storage_define.rs
Change Type: Feature Enhancement
Major Modifications:
Added
encoded_user_key_len()helper functionPrecisely calculates the length of the encoded key
Optimizes memory allocation
Line Count Change: +20 lines
Backward Compatibility: Yes (Purely New)
Test Statistics
Number of New Tests
| Module | Number of Tests | Coverage |
|------|---------|---------|
| zset_score_key_format | 25+ | Encoding/Decoding/Boundaries/Performance/Errors |
| custom_comparator | 15+ | Sorting/Special Values/NaN |
| Total | 40+ | Full Coverage |
Test Coverage
Target: ≥ 90%
Actual: ≥ 90% (estimated)
Uncovered: Very few edge cases
Test Categories
Performance Impact
Memory Allocation Optimization
Before Optimization:
After Optimization:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.