pvary commented on code in PR #18149:
URL: https://github.com/apache/iceberg/pull/18149#discussion_r4492311110
##########
flink/v2.3/flink/src/main/java/org/apache/iceberg/flink/source/lookup/IcebergFullCachingLookupFunction.java:
##########
@@ -51,11 +54,14 @@
* table, after projection and pushed-down filters are applied.
*
* <p>The cache is loaded from the current snapshot either when the function
is opened or on the
- * first lookup, depending on {@link
IcebergLookupOptions#FULL_CACHE_EAGER_LOAD}. It is never
- * refreshed, so later changes to the table are not visible.
+ * first lookup, depending on {@link
IcebergLookupOptions#FULL_CACHE_EAGER_LOAD}. When a {@link
+ * CacheReloadTrigger} is configured, the cache is reloaded on the trigger's
schedule: a reload that
+ * finds no new snapshot keeps the loaded cache, and a reload that fails fails
the next lookup
+ * instead of serving the previously loaded cache.
*/
@Internal
-public class IcebergFullCachingLookupFunction extends LookupFunction {
+public class IcebergFullCachingLookupFunction extends LookupFunction
+ implements CacheReloadTrigger.Context {
Review Comment:
My bad. What do you think still keeping the `CacheReloadTrigger.Context`
methods out of the `IcebergFullCachingLookupFunction` interface and using a new
class instead `ReloadContext implements CacheReloadTrigger.Context`? Would this
be better separation of concerns? Or, it is very common for Flink functions to
implement the context too?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
