zhijiangW commented on a change in pull request #8133: [FLINK-12146][network]
Remove unregister task from NetworkEnvironment to simplify the interface of
ShuffleService
URL: https://github.com/apache/flink/pull/8133#discussion_r276114525
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java
##########
@@ -1458,26 +1456,15 @@ private static AbstractInvokable
loadAndInstantiateInvokable(
private static class TaskCanceler implements Runnable {
private final Logger logger;
+ private final Task task;
private final AbstractInvokable invokable;
private final Thread executer;
- private final String taskName;
- private final ResultPartition[] producedPartitions;
- private final InputGate[] inputGates;
-
- public TaskCanceler(
- Logger logger,
- AbstractInvokable invokable,
- Thread executer,
- String taskName,
- ResultPartition[] producedPartitions,
- InputGate[] inputGates) {
+ TaskCanceler(Logger logger, Task task, AbstractInvokable
invokable, Thread executer) {
Review comment:
The motivation is for reusing the `closeOrFailNetworkResources` and avoid
static method and pass the arrays of `ResultPartition` and `InputGate`
explicitly. The previous `AbstractInvokable` could also be replaced and got
from new `Task` parameter.
I think the previous introduced `AbstractInvokable` here is also not a good
way considering exposing more public methods besides
`AbstractInvokable#cancel()`. Comparing with `Task` parameter, we might add
more public methods to do so. I agree with reverting this change to pass the
previous specific three parameters here even though the
`closeOrFailNetworkResources` might seem ugly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
