SmirAlex commented on code in PR #20919:
URL: https://github.com/apache/flink/pull/20919#discussion_r1066988436
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/table/lookup/fullcache/LookupFullCache.java:
##########
@@ -57,7 +57,7 @@ public synchronized void open(CacheMetricGroup metricGroup) {
}
metricGroup.hitCounter(hitCounter);
metricGroup.missCounter(new SimpleCounter()); // always zero
- cacheLoader.open(metricGroup);
+ cacheLoader.initializeMetrics(metricGroup);
}
public synchronized void open(Configuration parameters) throws Exception {
Review Comment:
I agree that 2 `open` methods is a problem, not only because of naming, but
because of ambiguity in the order of calling these methods, and non-compliance
with the contract of `LookupCache#open`, which should fully open the cache in
one call. So I decided to keep just one `open` method and pass `Configuration`
in different method. As a drawback we now have redundant state in cache, but I
think it's a good compromise in our circumstances.
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/table/lookup/fullcache/LookupFullCache.java:
##########
@@ -109,7 +109,7 @@ public long size() {
@Override
public void close() throws Exception {
- reloadTrigger.close(); // firstly try to interrupt reload thread
+ reloadTrigger.close();
cacheLoader.close();
Review Comment:
Added comments
##########
flink-table/flink-table-runtime/src/test/java/org/apache/flink/table/runtime/functions/table/fullcache/inputformat/FullCacheTestInputFormat.java:
##########
@@ -86,20 +92,21 @@ public FullCacheTestInputFormat(
this.projectable = generatedProjection.isPresent();
this.generatedProjection = generatedProjection.orElse(null);
this.rowConverter = rowConverter;
- this.deltaNumSplits = 0;
+ this.numSplits = DEFAULT_NUM_SPLITS;
+ this.deltaNumSplits = DEFAULT_DELTA_NUM_SPLITS;
Review Comment:
Fixed
--
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]
