Github user GJL commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5938#discussion_r184848405
  
    --- Diff: 
flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterClient.java ---
    @@ -265,6 +264,20 @@ public void waitForClusterToBeReady() {
                }
        }
     
    +   @Override
    +   public void shutDownCluster() {
    +           LOG.info("Sending shutdown request to the Application Master");
    +           try {
    +                   final Future<Object> response = 
Patterns.ask(applicationClient.get(),
    +                           new 
YarnMessages.LocalStopYarnSession(ApplicationStatus.SUCCEEDED,
    --- End diff --
    
    Always `SUCCEEDED` because previous logic in 1.4.2 used
    ```
    YarnApplicationState appState = lastReport.getYarnApplicationState();
                        ApplicationStatus status =
                                (appState == YarnApplicationState.FAILED || 
appState == YarnApplicationState.KILLED) ?
                                        ApplicationStatus.FAILED : 
ApplicationStatus.SUCCEEDED;
                        if (status != ApplicationStatus.SUCCEEDED) {
                                LOG.warn("YARN reported application state {}", 
appState);
                                LOG.warn("Diagnostics: {}", 
lastReport.getDiagnostics());
                        }
                        return status;
    ```
    which does not make sense imo.


---

Reply via email to