Varun Saxena commented on YARN-4393:

[~ozawa], are these tests failing ? Maybe there is some other problem.
Because dispatcher.await() would only make sense if we are verifying an event 
which we expect to be sent to dispatcher or verifying something which happens 
upon processing of such an event.
In the instances pointed above, I think it should be fine even without 

1. In testRecovery, the statements pointed out above, we are first checking for 
whether localization has started or not for a resource and then posting 
ResourceLocallizedEvent to localizer tracker. We are not verifying what happens 
due to handling of that event. If you notice we do have a dispatcher.await() 
right after these set of statements. And after that only we check the state of 
resource. So this single dispatcher.await() should be enough here.

2. In testResourceRelease also, the statement being verified is that whether 
cleanupPrivLocalizers has been called. This method is called when we call 
ResourceLocalizationService#handle with ContainerLocalizationCleanupEvent 
directly. Because handle() will call handleCleanupContainerResources() where 
this method is called. We do not need to wait here as we will be able to verify 
this method invocation as soon as call to handle() finishes. And we do have 
dispatcher.await() for later verifications.
 spyService.handle(new ContainerLocalizationCleanupEvent(c, req)); // <-- here!

3. For testFailedDirsResourceRelease too, same explanation holds true. Even the 
delete statements being verified below the code snippet pointed out by you will 
be called in handleCleanupContainerResources(). So we do not need to wait. And 
we have dispatcher.await() after these verifications.

Thoughts ?

> TestResourceLocalizationService#testFailedDirsResourceRelease fails 
> intermittently
> ----------------------------------------------------------------------------------
>                 Key: YARN-4393
>                 URL: https://issues.apache.org/jira/browse/YARN-4393
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 2.7.1
>            Reporter: Varun Saxena
>            Assignee: Varun Saxena
>             Fix For: 2.7.3
>         Attachments: YARN-4393.01.patch
> [~ozawa] pointed out this failure on YARN-4380.
> Check 
> https://issues.apache.org/jira/browse/YARN-4380?focusedCommentId=15023773&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15023773
> {noformat}
> sts run: 14, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 5.518 sec <<< 
> FAILURE! - in 
> org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService
> testFailedDirsResourceRelease(org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService)
>  Time elapsed: 0.093 sec <<< FAILURE!
> org.mockito.exceptions.verification.junit.ArgumentsAreDifferent:
> Argument(s) are different! Wanted:
> eventHandler.handle(
> <custom argument matcher>
> );
> -> at 
> org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService.testFailedDirsResourceRelease(TestResourceLocalizationService.java:2632)
> Actual invocation has different arguments:
> eventHandler.handle(
> );
> -> at 
> org.apache.hadoop.yarn.event.AsyncDispatcher.dispatch(AsyncDispatcher.java:183)
> at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
> at 
> sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
> at 
> sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
> at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
> at 
> org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService.testFailedDirsResourceRelease(TestResourceLocalizationService.java:2632)
> {noformat}

This message was sent by Atlassian JIRA

Reply via email to