pnowojski 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_r276106172
##########
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:
Why are you introducing circular dependency here between `Task` and
`TaskCanceler`? There are various reasons why this is bad, including: is it
necessary to expose 27 public methods (including things like
`startTaskThread()` or `run()`) to the `TaskCanceler`?
In various different places we are trying to get away from this pattern of
passing `StreamingTask` everywhere.
----------------------------------------------------------------
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