azagrebin commented on a change in pull request #8459: [FLINK-12476] [State
TTL] Consider setting a default background cleanup strategy in StateTtlConfig
URL: https://github.com/apache/flink/pull/8459#discussion_r285373468
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/state/ttl/TtlStateFactory.java
##########
@@ -202,6 +204,15 @@ private IS createReducingState() throws Exception {
}
private TtlIncrementalCleanup<K, N, TTLSV> getTtlIncrementalCleanup() {
+ //if cleanup in background and user did not specify the cleanup
strategy,
+ //then build the incremental cleanup strategy and apply the
default value
+ if (ttlConfig.isCleanupInBackground() &&
+
ttlConfig.getCleanupStrategies().getIncrementalCleanupStrategy() == null) {
+ ttlConfig.getCleanupStrategies().activate(
Review comment:
I would be against modifying `ttlConfig` in any way. It is supposed to be
immutable, once created by its builder. This protects against implicit changes
of `ttlConfig` which is created by user. We should actually make
`CleanupStrategies.strategies/activate` private or better even move
`CleanupStrategies.strategies` to builder (instead of having there
`CleanupStrategies`) and remove `CleanupStrategies.activate`. The builder
should then also build immutable `CleanupStrategies` for `StateTtlConfig` at
the end.
If we move `isCleanupInBackground` to `CleanupStrategies` with its getter
(where it rather belongs anyways) we can actually just modify
`CleanupStrategies.getIncrementalCleanupStrategy/getRocksdbCompactFilterCleanupStrategy`
to return default strategies as `strategies.getOrDefault` if they are not set
but `isCleanupInBackground` is set. The default strategy singletons can be
static final constants in respective classes. The backends and private field
modifiers can also stay untouched but any strategy and backend can still
independently decide what to use if `isCleanupInBackground` is set.
`CleanupStrategies.inFullSnapshot/inRocksdbCompactFilter` should use the
respective getters != null then, not `CleanupStrategies.strategies` directly.
----------------------------------------------------------------
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
