aiborodin commented on code in PR #13340:
URL: https://github.com/apache/iceberg/pull/13340#discussion_r2160768670


##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicIcebergSink.java:
##########
@@ -315,6 +316,16 @@ public Builder<T> cacheRefreshMs(long refreshMs) {
       return this;
     }
 
+    /**
+     * Maximum input {@link org.apache.iceberg.Schema} objects to cache per 
each Iceberg table. The
+     * cache improves Dynamic Sink performance by storing {@link 
org.apache.iceberg.Schema}
+     * comparison results.
+     */
+    public Builder<T> inputSchemasPerTableCacheMaxSize(int 
inputSchemasPerTableCacheMaximumSize) {
+      this.inputSchemasPerTableCacheMaximumSize = 
inputSchemasPerTableCacheMaximumSize;
+      return this;
+    }
+

Review Comment:
   I am feeling confident about adding this config because it covers a 
performance-critical part of the code, so users should be able to tune it. The 
fact that we already print a warning 
[here](https://github.com/apache/iceberg/blob/main/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/TableMetadataCache.java#L248)
 witnesses this.
   
   The existing `cacheMaxSize` seems to be overloaded by setting three 
completely independent variables:
   1. How many tables to remember
   2. How many serialisers to cache across all tables
   3. How many key selectors to cache across all tables
   
   `inputSchemasPerTableCacheMaxSize` is unrelated to all of the above because 
it sets how many input schemas to cache on a per-table basis. 
   Re-using `cacheMaxSize` also means we have to keep the default of `1000` 
input schemas, which is 100x more than the current hard-coded 
`TableMetadataCache.MAX_SCHEMA_COMPARISON_RESULTS_TO_CACHE = 10`. It also 
doesn't make sense to cache so many user-supplied schemas by default.
   
   If we decide to remove the config in the future, we can always find some 
safe deprecation path.



-- 
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]

Reply via email to