1996fanrui commented on code in PR #23635:
URL: https://github.com/apache/flink/pull/23635#discussion_r1381212518
##########
flink-core/src/main/java/org/apache/flink/configuration/TaskManagerOptions.java:
##########
@@ -708,6 +708,20 @@ public class TaskManagerOptions {
"Time we wait for the timers in milliseconds to
finish all pending timer threads"
+ " when the stream task is cancelled.");
+ public static final ConfigOption<TaskManagerLoadBalanceMode>
TASK_MANAGER_LOAD_BALANCE_MODE =
Review Comment:
Thanks @RocMarshal for the analysis!
How about creating a JIRA to add `taskmanager.load-balance.mode` and
deprecate `cluster.evenly-spread-out-slots` first?
- It don't introduce any feature, just using the new option and deprecate
old option.
- It should be the first JIRA, and `TaskManagerLoadBalanceMode` just have 2
enums: `None` and `Slots`.
- In the second JIRA(maybe this PR), you can start your feature: adding the
`Tasks`, and supports it.
I think this process is clearer and easier than the process you proposed,
and it don't effect users side.
--------------------------
Besides how to implement this, I have a question about compatibility of
`taskmanager.load-balance.mode` and `cluster.evenly-spread-out-slots`.
As I understand :
- `taskmanager.load-balance.mode : Slots` ==
`cluster.evenly-spread-out-slots: true`
- `taskmanager.load-balance.mode : None` ==
`cluster.evenly-spread-out-slots: false` (By default.)
After this FLIP is merged, if a job with `cluster.evenly-spread-out-slots:
true`, and didn't set the `taskmanager.load-balance.mode`. Should we change the
default value from `None` to `Slots`?
- If yes, we can support old users directly.
- If no, users must set `taskmanager.load-balance.mode : Slots` manually if
they want to the `evenly-spread-out-slots` feature.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
