echauchot commented on code in PR #24021:
URL: https://github.com/apache/flink/pull/24021#discussion_r1465086482
##########
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveScheduler.java:
##########
@@ -177,6 +177,103 @@ public class AdaptiveScheduler
private static final Logger LOG =
LoggerFactory.getLogger(AdaptiveScheduler.class);
+ /**
+ * Consolidated settings for the adaptive scheduler. This class is used to
avoid passing around
+ * multiple config options.
+ */
+ public static class Settings {
Review Comment:
> I don't see where the SLOT_REQUEST_TIMEOUT is used by AS, can you please
give me a pointer on what you have in mind?
I had in mind [this
line](https://github.com/dmvk/flink/blob/1f8b6548ee3686d38a23024b0de474aaf869007d/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/DefaultSlotPoolServiceSchedulerFactory.java#L200)
But, still, it might not be a good idea to put it in the AdaptiveScheduler
Settings because it is used outside of AS as well for example:
`ConfigurationUtils#getStandaloneClusterStartupPeriodTime` and in
`JobMasterConfiguration`
> For MIN_PARALLELISM_INCREASE, I've intentionally kept that local to
EnforceMinimalIncreaseRescalingController. Ultimately the interface should be
swappable (that's why it's behind an interface and constructor accepts
configuration).
This parameter is meaningful only in the context of
`EnforceMinimalIncreaseRescalingController` for the other implementation
`EnforceParallelismChangeRescalingController` it is not. I agree, we should not
polluate the interface with implementation private concerns.
> OT: I'm also starting to doubt we still need it. We've talked about it
briefly with @zentol and it mostly seems to be a leftover from early work on
reactive mode.
OT?
--
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]
