KAFKA-8007: Avoid copying on fetch in InMemoryWindowStore #6335
Conversation
|
WIP planning to flesh out the unit tests given the increased complexity of the iterators, also looking into microbenchmarks to get a sense of the potential improvement here. |
|
@ableegoldman is this ready to review now or is still WIP? If yes please feel free to edit the title to include |
|
Waiting for #6328 to be merged so I can rebase for cleaner review, but yes it is ready |
|
Call for review @guozhangwang @mjsax @vvcephei @bbejeck |
|
@ableegoldman could you rebase the PR? |
…b spelling error)
ableegoldman
force-pushed
the
CopylessInMemoryWindowStore
branch
from
4172c8c
to
1bf13c6
Compare
|
Rebased (thanks for merging #6328) |
| final Windowed<Bytes> windowedK = new Windowed<>(key, new TimeWindow(startTimestamp, startTimestamp + windowSize)); | ||
| return new KeyValue<>(windowedK, value); | ||
| long minLiveTime = Math.max(0L, this.observedStreamTime - this.retentionPeriod + 1); | ||
| for (final InMemoryWindowStoreIteratorWrapper it : openIterators) { |
| InMemoryWindowStoreIteratorWrapper(final Bytes keyFrom, | ||
| final Bytes keyTo, | ||
| final Iterator<Map.Entry<Long, ConcurrentNavigableMap<Bytes, byte[]>>> segmentIterator) { | ||
| this.allKeys = keyFrom == null; |
There was a problem hiding this comment.
To be more strict, should this be keyFrom == null && keyTo == null? I know that today they either are both null or neither are null, but it is future-risk vulnerable if we ever change the apis to allow null values on one side.
There was a problem hiding this comment.
Fair point, will update
|
Java 11 failed retest this please |
|
Test result already deleted. Java8 failed. Java11 passed. Retest this please. |
|
Since #6328 is merged, I'm going to merge this one now as well. |
|
Merged #6335 to trunk |
Rewrote the InMemoryWindowStore implementation by moving the work of a fetch to the iterator, and cleaned up the iterators as well. Reviewers: Guozhang Wang <wangguoz@gmail.com>, Bill Bejeck <bbejeck@gmail.com>
Rewrote the InMemoryWindowStore implementation by moving the work of a fetch to the iterator, and cleaned up the iterators as well.
Blocked by KAFKA-7918
Committer Checklist (excluded from commit message)