wuchong commented on code in PR #3623:
URL: https://github.com/apache/fluss/pull/3623#discussion_r3611944886
##########
fluss-common/src/main/java/org/apache/fluss/config/ConfigOptions.java:
##########
@@ -852,6 +852,15 @@ public class ConfigOptions {
+ "flush which acts as the log recovery
point. The default "
+ "setting is 60 seconds.");
+ public static final ConfigOption<Duration> LOG_RETENTION_CHECK_INTERVAL =
+ key("log.retention-check-interval")
Review Comment:
Could we rename this option to `log.retention.check-interval` and require it
to be strictly positive?
Using `log.retention` as the common prefix provides a consistent namespace
for future retention-related options, such as `log.retention.bytes` and
`log.retention.idle-timeout`.
I also don't think `0` should disable this task. The periodic check is the
only time-driven cleanup trigger for idle tables, so disabling it means
`table.log.ttl` can no longer be enforced reliably when no further log or
manifest events occur. Per-table retention can already be disabled with
`table.log.ttl = -1`, and Kafka's analogous `log.retention.check.interval.ms`
only accepts positive values. Unless we have a concrete use case for globally
disabling retention checks, please validate this option as `> 0` and remove the
`0`-disables semantics.
--
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]
