Tsuyoshi Ozawa updated YARN-2921:
    Attachment: YARN-2921.008.patch

[~leftnoteasy] thank you for the comments.

  1. why minWaitMSec is needed?

If minWaitMsec is removed, some test fails currently. It looks that some tests 
depend on sleep time in MockRM/MockAM. We can fix it on another JIRA.

  2. Why fail the method if if (waitedMsecs >= timeoutMsecs) is true? I think 
it should check now-state against expected state.
* In two MockRM.waitForState method, I think we should also check 
app.getState() instead of time, correct?

Let me explain. The state check is done in the while statement. After that, the 
additional state transitions can occur. It results false-positive assertions - 
waitFor() waits for the targeted state, but assertions can fail. To prevent 
this kind of false positive assertions, I don't add the state check after the 
while loop. Does it make sense to you? 

In TestRMRestart, you can use GenericTestUtils.waitFor instead.

> MockRM#waitForState methods can be too slow and flaky
> -----------------------------------------------------
>                 Key: YARN-2921
>                 URL: https://issues.apache.org/jira/browse/YARN-2921
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: test
>    Affects Versions: 2.6.0, 2.7.0
>            Reporter: Karthik Kambatla
>            Assignee: Tsuyoshi Ozawa
>         Attachments: YARN-2921.001.patch, YARN-2921.002.patch, 
> YARN-2921.003.patch, YARN-2921.004.patch, YARN-2921.005.patch, 
> YARN-2921.006.patch, YARN-2921.007.patch, YARN-2921.008.patch
> MockRM#waitForState methods currently sleep for too long (2 seconds and 1 
> second). This leads to slow tests and sometimes failures if the 
> App/AppAttempt moves to another state. 

This message was sent by Atlassian JIRA

Reply via email to