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

Reply via email to