KAFKA-20497: Add readOnly(IsolationLevel) to CachingWindowStore#22314
Conversation
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
nicktelford
force-pushed
the
KIP-892/iq-isolation-caching-window
branch
from
bbbc1ba to
7569338
Compare
bbejeck
left a comment
bbejeck
left a comment
There was a problem hiding this comment.
Thanks for the PR @nicktelford overall looks good. I have a question about the moving the synchronized method modfier.
Also the org.apache.kafka.streams.state.internals.CachingPersistentWindowStoreTest#shouldNotThrowInvalidBackwardRangeExceptionWithNegativeFromKey is failing
nicktelford
force-pushed
the
KIP-892/iq-isolation-caching-window
branch
2 times, most recently
from
a547da6 to
0fecb55
Compare
@bbejeck Fixed the synchronisation, as requested, and rebased on trunk, which should fix the tests. |
CachingWindowStore had no isolation-level awareness, so callers could not bypass the cache to read only committed data, nor could they obtain a merged (cache + store) view via a typed ReadOnly interface. This adds readOnly(IsolationLevel) to CachingWindowStore. For READ_COMMITTED, the call is forwarded directly to the wrapped store, which excludes anything still in the cache. For READ_UNCOMMITTED, a private ReadOnlyView is returned; it delegates fetches to the underlying READ_UNCOMMITTED view of the wrapped store but threads the results through the existing internal merge helpers (fetchInternal, fetchKeyRangeInternal, fetchAllInternal, allInternal), so cache entries are merged with stored entries in the same way as the public API. As a prerequisite, the duplicated fetch/backwardFetch/all/backwardAll bodies are consolidated into those shared private helpers, reducing the surface for future divergence and making the ReadOnlyView delegation straightforward. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nicktelford
force-pushed
the
KIP-892/iq-isolation-caching-window
branch
from
0fecb55 to
a15e31f
Compare
bbejeck
left a comment
bbejeck
left a comment
There was a problem hiding this comment.
Thanks for the update @nicktelford LGTM
|
merged #22314 into trunk |
2 participants
The cache holds uncommitted writes that must not be visible under
READ_COMMITTED, so that isolation level bypasses the cache entirely and
delegates straight to the inner store's readOnly view. READ_UNCOMMITTED
requires a merged view of both cache and store through a ReadOnlyView.
The ReadOnlyView converts Instant arguments to epoch-milliseconds before
calling the shared fetchInternal/fetchKeyRangeInternal/fetchAllInternal
helpers rather than converting at the merge boundary; the cache key
schema operates in longs throughout, so converting early avoids repeated
Instant-to-long conversions in iterator hot paths.
KAFKA-20497
Reviewers: Bill Bejeck bbejeck@apache.org