YTDB-655: Zero-deserialization fix for index point lookups#962
Open
Andrii Rodionov (arodionov) wants to merge 9 commits into
Open
YTDB-655: Zero-deserialization fix for index point lookups#962Andrii Rodionov (arodionov) wants to merge 9 commits into
Andrii Rodionov (arodionov) wants to merge 9 commits into
Conversation
added 8 commits
April 13, 2026 11:46
Zero-deserialization field-by-field comparison for composite index keys. The non-WAL path delegates to per-type serializer compareInByteBuffer() for LONG/INTEGER/SHORT/STRING/BINARY and inlines FLOAT/DOUBLE (stored as int/long bits), BOOLEAN/BYTE, LINK (compacted format), and DECIMAL (BigDecimal fallback). The WAL-aware path inlines primitive reads via walChanges methods and falls back to deserialization for STRING/LINK/ BINARY/DECIMAL. 34 unit tests cover all 12 field types, null handling, prefix comparison, and WAL overlay variants.
… tests Add assert pageTypeId == searchTypeId in both compareInByteBuffer and WAL variant to catch serialization inconsistencies during development. Remove unnecessary @SuppressWarnings("unchecked") on compareDecimalFallback. Add 3 test methods: LINK position 0 boundary, asymmetric numberSize encoding widths, and non-zero buffer/key offsets.
These review and step files are working files that persist on disk between sessions but should not be committed to git.
…earch Reverse Track 4a: add serializeNativeAsWhole() after buildSearchKey() to serialize the search key into a byte array. Pass both serializedKey (for bucket.find(byte[], ...) and findBucketSerialized()) and searchKey (for scanLeafForVisible()'s userKeyPrefixMatches()) to getVisibleOptimistic and getVisiblePinned. This uses IndexMultiValuKeySerializer's new compareInByteBuffer() for zero-deserialization field-by-field comparison during the B-tree binary search, eliminating CompositeKey allocation on every binary search step.
…tests Add defensive assertions to compareInByteBuffer: non-negative keysCount in both non-WAL and WAL paths, and valid numberSize range (0-8) in readCompactedPosition/readCompactedPositionNative. Strengthen all test methods to assert comparison direction (< 0, > 0, == 0) at call sites instead of relying solely on the helper's internal sign-match check. Add three new boundary test cases: empty BINARY arrays (zero-length field data in offset advancement), multi-byte UTF-8 strings (2-byte and 3-byte sequences), and Integer.MIN_VALUE/MAX_VALUE boundaries.
Post-implementation artifacts updated to reflect the compareInByteBuffer optimization (Track 5): - design-final.md: added IndexMultiValuKeySerializer to class diagram, updated workflow to show find(byte[]) path, added zero-deserialization binary search section - adr.md: added D6 decision record, updated D3 evolution through Tracks 4a→5, added key discoveries #6 and #7, updated performance results
Track 5 step file, technical review, risk review, and updated implementation plan with Track 5 episode and strategy refresh.
Adds IndexGetBenchmark to measure raw engine.get() throughput on a UNIQUE index with 1M entries, isolating the index lookup cost from SQL parsing and transaction overhead. Includes single-key and batch-1k variants for measuring per-op and amortized throughput.
b88486f to
a9565f7
Compare
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements zero-deserialization field-by-field comparison for CompositeKey in IndexMultiValuKeySerializer, which optimizes index lookups by eliminating object allocations during binary search. The BTree implementation is updated to use this serialized-key search path. Feedback highlights a critical bug where the search key is not preprocessed before serialization, which is required for correct normalization of types like DATE. Additionally, the documentation for DECIMAL comparison should be updated to accurately describe its fallback deserialization behavior.
The preprocess step normalizes DATE keys (truncates time component) and LINK keys (unwraps Identifiable to RID) so the search key matches the stored representation. Without it, lookups on DATE-typed indexes with un-truncated timestamps would silently miss entries. For common types (STRING, INTEGER, LONG) preprocess is a no-op.
Test Count Gate Results✅ No baseline available yet — gate skipped (first run). |
Coverage Gate ResultsThresholds: 85% line, 70% branch Line Coverage: ✅ 95.7% (155/162 lines)
Branch Coverage: ✅ 94.0% (63/67 branches)
|
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.
Motivation:
After introducing snapshot-isolation for indexes (YTDB-523), the
engine.get()hot pathdeserialized every B-tree entry during binary search to compare keys — allocating temporary
objects on each comparison. Without the
compareInByteBufferoptimization, JMH benchmarksshowed a severe regression:
engineGet(ops/s)engineGetBatch1k(ops/ms)This PR adds
compareInByteBuffer()toIndexMultiValuKeySerializer, enabling field-by-fieldcomparison directly in the page byte buffer without deserializing entries. Combined with the
bucket.find(byte[])path ingetVisible(), this eliminates per-comparison allocationsduring B-tree descent.
After fix (this branch vs pre-SI baseline
89499755):engineGet(ops/s)engineGetBatch1k(ops/ms)The single-lookup regression is reduced from -43% to -5%. The batch regression (which
amplifies per-lookup overhead) is reduced from -60% to -28%, with the remaining cost
attributable to the version field comparison and snapshot visibility checks inherent to SI.
Changes
IndexMultiValuKeySerializer.compareInByteBuffer()— zero-allocation in-buffercomparison for composite keys during B-tree binary search
BTree.getVisible()switched tofind(byte[])— uses serialized key for binarysearch instead of deserializing + preprocessing
IndexGetBenchmark— JMH benchmark for directengine.get()throughput on a1M-entry UNIQUE index (single-key and batch-1k variants)
Test plan
./mvnw -pl core clean test)IndexMultiValuKeySerializerCompareTestcovers type-specific comparison, LINK edge cases, and type mismatch assertions