[GitHub] [incubator-paimon] hzjhjjyy commented on a diff in pull request #2021: [flink] Allow dynamic configuration of 'continuous.discovery-interval' using hints in Flink lookup joins.

["hzjhjjyy (via GitHub)" <gi...@apache.org>]

hzjhjjyy commented on code in PR #2021:
URL: https://github.com/apache/incubator-paimon/pull/2021#discussion_r1329080543


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/RocksDBOptions.java:
##########
@@ -55,6 +56,12 @@ public class RocksDBOptions {
                     .defaultValue(10_000L)
                     .withDescription("The maximum number of rows to store in the cache.");
 
+    public static final ConfigOption<Duration> LOOKUP_CONTINUOUS_DISCOVERY_INTERVAL =
+            key("lookup.continuous.discovery-interval")
+                    .durationType()
+                    .defaultValue(Duration.ofSeconds(10))

Review Comment:
   > Can this be `noDefaultValue`, and fallback to `continuous.discovery-interval`?
   
   That's acceptable, and after reviewing the relevant code, I have the following considerations:
   
   1. While `ConfigOption` indeed includes the `withFallbackKeys` method, I haven't observed any code using this method at the moment. This suggests that even after configuring it, additional changes would be required in other parts of the code, such as `Options#get`.
   
   2. If use `withFallbackKeys`, it would introduce `CoreOptions` into `RocksDBOptions`, which might make `RocksDBOptions` appear cluttered (a standalone configuration class referencing another standalone configuration class).
   
   As a result, I added `continuous.discovery-interval` as an alternative option in `FileStoreLookupFunction`. (Originally, it was already referencing `CoreOptions.CONTINUOUS_DISCOVERY_INTERVAL` before my modifications.)
   
   PTAL, thx



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org