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

Siddharth Seth commented on YARN-357:
-------------------------------------

bq. Thanks for the review! I struggled quite a bit with writing these tests. I 
am indeed expecting the barrier to cause the test to timeout. How do you 
suggest I add an explicit error? Log the assert since there's not much else I 
can (easily) do? 
A flag to track the error in the thread should work ?

I couldn't get chaining to work with void return methods. The mock hangs on a 
poll for repeated calls to a method with a doAnswer if there aren't more 
doAnswers registered with it, but since I couldn't get the chaining to work 
up-front, I added the chain after the method is hit the first time.
bq. Chaining seemed to work ok, as long as the previous invocation had 
completed. Even without chaining, the mock does not seem to handle multiple 
parallel invocations. Struggled a bit with this as well - which is what makes 
me a little wary of using mockito for this case 
(http://code.google.com/p/mockito/wiki/FAQ).

bq. How would a custom event handler work better in this case? I think it'd 
just mimic the current behavior? Could we maybe have the separate jira, for 
removing other syncs, improve the tests in general? Or could you help me out by 
updating the tests since I'm unclear what to do?
It'd behave the same, except would not use mockito for the EventHandler mock. 
Sure, I'll try updating the unit test.
                
> App submission should not be synchronized
> -----------------------------------------
>
>                 Key: YARN-357
>                 URL: https://issues.apache.org/jira/browse/YARN-357
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: resourcemanager
>    Affects Versions: 0.23.3, 3.0.0, 2.0.0-alpha
>            Reporter: Daryn Sharp
>            Assignee: Daryn Sharp
>         Attachments: YARN-357.patch, YARN-357.patch
>
>
> MAPREDUCE-2953 fixed a race condition with querying of app status by making 
> {{RMClientService#submitApplication}} synchronously invoke 
> {{RMAppManager#submitApplication}}. However, the {{synchronized}} keyword was 
> also added to {{RMAppManager#submitApplication}} with the comment:
> bq. I made the submitApplication synchronized to keep it consistent with the 
> other routines in RMAppManager although I do not believe it needs it since 
> the rmapp datastructure is already a concurrentMap and I don't see anything 
> else that would be an issue.
> It's been observed that app submission latency is being unnecessarily 
> impacted.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to