SmirAlex commented on PR #20919: URL: https://github.com/apache/flink/pull/20919#issuecomment-1333174160
> But just on a side note (independent of this PR's change): I might be wrong but it feels like we could have implemented `reloadCache` in an async fashion returning a `CompletableFuture`. Because when investigating the call hierachy: We're calling `reloadCache` in a `Runnable#run` which we execute in `ReloadTriggerContext#triggerReload` anyway asynchronously. I'm just mentioning it because I'm curious what your view is on that. Actually it's not pretty clear for me why `ReloadTriggerContext#triggerReload` executes asynchronously (returns a future). Probably to give users (developers of custom reload triggers) more flexibility. But anyway, this API was proposed by @PatrickRen and was accepted in [FLIP-221](https://cwiki.apache.org/confluence/display/FLINK/FLIP-221:+Abstraction+for+lookup+source+cache+and+metric), so I followed the agreement. > we could have implemented `reloadCache` in an async fashion returning a `CompletableFuture` Yea, we could, but the benefits are no so clear for me. As I see it, we would have the same `CompletableFuture.runAsync()`, but in `CacheLoader`. IMHO, this class will just become little more overloaded after that. But actually I can do such change if needed. -- 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]
