[ 
https://issues.apache.org/jira/browse/FLINK-6130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15970972#comment-15970972
 ] 

ASF GitHub Bot commented on FLINK-6130:
---------------------------------------

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

    https://github.com/apache/flink/pull/3726#discussion_r111730519
  
    --- Diff: 
flink-yarn/src/main/java/org/apache/flink/yarn/YarnFlinkApplicationMasterRunner.java
 ---
    @@ -158,14 +160,16 @@ protected int runApplicationMaster(Configuration 
config) {
                                jobManagerRunner.start();
                                LOG.debug("Job Manager Runner started");
     
    +                           // wait for resource manager to finish and 
return a value for later to get
    +                           future = (Future<?>) 
resourceManager.getTerminationFuture();
    +
                                // ---- (5) start the web monitor
                                // TODO: add web monitor
                        }
    +                   Object object = future.value().get();
     
    -                   // wait for resource manager to finish
    -                   resourceManager.getTerminationFuture().get();
                        // everything started, we can wait until all is done or 
the process is killed
    -                   LOG.info("YARN Application Master finished");
    +                   LOG.info("YARN Application Master finished and the 
result is " + object.toString());
    --- End diff --
    
    I would use a log message formatter here:
    `LOG.info("YARN Application Master finished and the result is {}", object);`


> Consider calling resourceManager#getTerminationFuture() with lock held
> ----------------------------------------------------------------------
>
>                 Key: FLINK-6130
>                 URL: https://issues.apache.org/jira/browse/FLINK-6130
>             Project: Flink
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: mingleizhang
>            Priority: Minor
>
> In YarnFlinkApplicationMasterRunner#runApplicationMaster() :
> {code}
>       synchronized (lock) {
>         LOG.info("Starting High Availability Services");
> ...
>       }
>       // wait for resource manager to finish
>       resourceManager.getTerminationFuture().get();
> {code}
> resourceManager#getTerminationFuture() is called without holding lock.
> We should store the value returned from 
> resourceManager#getTerminationFuture() inside the synchronized block.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to