twalthr commented on code in PR #24869:
URL: https://github.com/apache/flink/pull/24869#discussion_r1631120470
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/operators/aggregate/window/buffers/RecordsWindowBuffer.java:
##########
@@ -164,12 +177,24 @@ public WindowBuffer create(
return new RecordsWindowBuffer(
Review Comment:
How often is this method called during runtime? Do we need to cache calls to
ExecutionConfig to avoid string-parsing and conversion overhead?
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/ExecutionConfigOptions.java:
##########
@@ -315,6 +316,57 @@ public class ExecutionConfigOptions {
.withDescription(
"Sets the window elements buffer size limit used
in group window agg operator.");
+ public static final ConfigOption<MemorySize> GLOBAL_AGG_BUFFER_SIZE =
+ ConfigOptions.key("table.exec.window-agg.global.buffer-size")
Review Comment:
This seems to be a new category of execution config options because they are
read during runtime. It might make sense to call them
`table.runtime.window-agg.global.buffer-size` and create a dedicated section
below?
##########
flink-core/src/main/java/org/apache/flink/api/common/ExecutionConfig.java:
##########
@@ -1271,6 +1276,8 @@ public InlineElement getDescription() {
* @param classLoader a class loader to use when loading classes
*/
public void configure(ReadableConfig configuration, ClassLoader
classLoader) {
+ configuration.toMap().forEach(this.configuration::setString);
+
Review Comment:
How about we simply add a `set(ConfigOption)` and `set(String)` to
`ExecutionConfig`? It is not really necessary that ExecutionConfig picks up all
configuration from `ReadableConfig configuration`. For a later runtime patching
of ExecutionConfig, we could only use the `set(String)`.
--
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]
