dmvk commented on a change in pull request #18689:
URL: https://github.com/apache/flink/pull/18689#discussion_r805206533
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adaptive/StateWithExecutionGraph.java
##########
@@ -148,6 +162,33 @@ public Logger getLogger() {
return logger;
}
+ protected Throwable extractError(TaskExecutionStateTransition
taskExecutionStateTransition) {
+ Throwable cause =
taskExecutionStateTransition.getError(userCodeClassLoader);
+ if (cause == null) {
+ cause = new FlinkException("Unknown failure cause. Probably
related to FLINK-21376.");
+ }
+ return cause;
+ }
+
+ protected Optional<ExecutionVertexID> extractExecutionVertexID(
+ TaskExecutionStateTransition taskExecutionStateTransition) {
+ return
executionGraph.getExecutionVertexId(taskExecutionStateTransition.getID());
+ }
+
+ protected static Optional<RootExceptionHistoryEntry> convertFailures(
+ Function<ExecutionVertexID, Optional<ExecutionVertex>> lookup,
Review comment:
There is a similar method in the `Failure` class. I guess giving the
parameter a more descriptive name would do the trick
```
/**
* Convert the failure into an {@link ExceptionHistoryEntry}.
*
* @param findExecutionFn Function that looks up additional information
on the {@link
* ExecutionVertexID}, which can be used for constructing the {@link
ExceptionHistoryEntry}.
* @return The {@link ExceptionHistoryEntry} derived for this failure.
*/
public abstract ExceptionHistoryEntry toExceptionHistoryEntry(
Function<ExecutionVertexID, Execution> findExecutionFn);
```
One additional thought about naming getters, it's good to distinguish
between getters that return an actual value (`getXXX`) and getters that return
an optional (`findXXX`).
--
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]
