mjsax commented on code in PR #13142: URL: https://github.com/apache/kafka/pull/13142#discussion_r1084826103
########## streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java: ########## @@ -114,6 +114,7 @@ public class RocksDBStore implements KeyValueStore<Bytes, byte[]>, BatchWritingS private boolean userSpecifiedStatistics = false; private final RocksDBMetricsRecorder metricsRecorder; + private final boolean userManagedIterators; Review Comment: "User" sounds a little bit like KS user to me. Maybe we can rename, eg, `selfManagedIterators` or similar (not sure what a good name would be). ########## streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java: ########## @@ -351,13 +360,23 @@ public <R> QueryResult<R> query( @Override public <PS extends Serializer<P>, P> KeyValueIterator<Bytes, byte[]> prefixScan(final P prefix, final PS prefixKeySerializer) { + if (userManagedIterators) { + throw new IllegalStateException("Must specify openIterators in call to prefixScan()"); + } + return prefixScan(prefix, prefixKeySerializer, openIterators); + } + + <PS extends Serializer<P>, P> KeyValueIterator<Bytes, byte[]> prefixScan(final P prefix, + final PS prefixKeySerializer, + final Set<KeyValueIterator<Bytes, byte[]>> openIterators) { Review Comment: Is it valid to call this method if `userManagedIterators` is false? (Similar elsewhere.) ########## streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBTimestampedStore.java: ########## @@ -383,7 +383,9 @@ public KeyValue<Bytes, byte[]> makeNext() { @Override public synchronized void close() { - openIterators.remove(this); + if (closeCallback != null) { Review Comment: Don't we require that we always have a registered `closeCallback`? If yes, it seems invalid that it's `null` and having this check might mask a bug, and thus we should rather not have it, but let it crash with a NPE? (Also elsewhere.) -- 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