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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
