KAFKA-20597: Fix prefixScan overflow for prefixes with no upper bound in in-memory state stores#22789
Open
edenfunf wants to merge 2 commits into
Open
KAFKA-20597: Fix prefixScan overflow for prefixes with no upper bound in in-memory state stores#22789edenfunf wants to merge 2 commits into
edenfunf wants to merge 2 commits into
Conversation
… in in-memory state stores When a prefix has no lexicographically larger successor (e.g. it is all 0xFF bytes), incrementing it to compute the exclusive upper bound of the scan range overflows. RocksDBStore already handled this by treating the overflow as an unbounded range, but InMemoryKeyValueStore, MemoryNavigableLRUCache and CachingKeyValueStore called ByteUtils.increment() directly and threw IndexOutOfBoundsException. Centralize the overflow-safe increment as ByteUtils.incrementWithoutOverflow(), which returns null on overflow, and reuse it from the RocksDB code paths that previously defined a private copy. The in-memory stores now treat a null upper bound as "scan to the end of the keyspace" (tailMap), matching RocksDB. Add a regression test to AbstractKeyValueStoreTest so it runs against every KeyValueStore implementation, using IntegerSerializer's encoding of -1 (0xFFFFFFFF) to reproduce the overflow, plus unit tests for ByteUtils.incrementWithoutOverflow().
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.
Problem
When a prefix has no lexicographically larger successor (e.g. it is all
0xFFbytes),prefixScancomputes the exclusive upper bound of its range by incrementing the prefix, which overflows.RocksDBStorealready handles this by treating the overflow as an unbounded range, butInMemoryKeyValueStore,MemoryNavigableLRUCacheandCachingKeyValueStorecalledByteUtils.increment()directly and threwIndexOutOfBoundsException.Fix
ByteUtils.incrementWithoutOverflow(), which returnsnullon overflow. The RocksDB code paths that previously defined a private copy now reuse it.nullupper bound as "scan to the end of the keyspace" (tailMap), matching the behaviourRocksDBStorealready had.ByteUtils.increment()is left unchanged:RocksDBStore.deleteRange()intentionally relies on it throwing, since RocksDB'sdeleteRange()has no null-upper-bound form.Testing
prefixScanShouldReturnMatchesWhenPrefixHasNoUpperBoundtoAbstractKeyValueStoreTest, so it runs against everyKeyValueStoreimplementation. It usesIntegerSerializer's encoding of-1(0xFFFFFFFF) to reproduce the overflow. Verified the test fails on the unpatched in-memory stores and passes with the fix, while the RocksDB-backed stores pass in both cases.ByteUtils.incrementWithoutOverflow().Committer Checklist (excluded from commit message)