StefanRRichter commented on a change in pull request #7866: [FLINK-11730]
[State Backends] Make HeapKeyedStateBackend follow the builder pattern
URL: https://github.com/apache/flink/pull/7866#discussion_r262005789
##########
File path:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBKeyedStateBackendBuilder.java
##########
@@ -192,7 +194,8 @@ public RocksDBKeyedStateBackendBuilder(
ttlTimeProvider,
metricGroup,
stateHandles,
- keyGroupCompressionDecorator
+ keyGroupCompressionDecorator,
+ cancelStreamRegistry
Review comment:
Pleas notice that this is also a semantic change of the this registry: I
think before when we passed in `cancelStreamRegistry`, the scope was only for
controlling streams and closeables during restore. The backend itself was
creating it's own `CloseableRegistry` object, which is not the same as the
`cancelStreamRegistry` used during the restore. The difference is that they
have different scopes and could be closed individually, e.g. you could close
the `cancelStreamRegistry` after the restore, but now this would also close the
backend's main registry. This leads to a small leak, because
`cancelStreamRegistry` is never unregistered. So closing the backend is invoked
twice as well, once through this registry that is never unregistered from the
task and the other one because we register the backend itself as a closeable to
the task, see `BackendRestorerProcedure::attemptCreateAndRestore`, L148. And
the matching unregister is `AbstractStreamOperator::dispose`, e.g. L345. So
that registry is effectively closed from two ends.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
