aliehsaeedii commented on code in PR #14626: URL: https://github.com/apache/kafka/pull/14626#discussion_r1384186993
########## streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBVersionedStore.java: ########## @@ -253,6 +259,86 @@ public VersionedRecord<byte[]> get(final Bytes key, final long asOfTimestamp) { return null; } + // Visible for testing + ValueIterator<VersionedRecord<byte[]>> get(final Bytes key, final long fromTimestamp, final long toTimestamp, final boolean isAscending) { + + Objects.requireNonNull(key, "key cannot be null"); + validateStoreOpen(); + + final List<VersionedRecord<byte[]>> queryResults = new ArrayList<>(); Review Comment: > Not sure if we should materialize the result in-memory -- it there is a long history, it might put a lot of memory pressure on the JVM. -- Instead, we could return an Iterator of "segment iterators" (only for segments that contain data, or might contain data) and to the iteration "lazy / on-demand" instead. > > It's an somewhat open question for a single-key query, but we don't want to materialize the result for range-key queries for sure... > > (Cf. range queries over windowed stores for which we do the same -- at least high level -- given how the segment store work, we might not want to use an actual RocksDBIterator -- cf my top level review comment) I won't change the code but will add the topic to the KIP. Thanks a lot. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org