Junping Du commented on YARN-2209:

bq. I think the essential problem here is whether throwing new sub exception 
which may not be handled before is an acceptable incompatible change, and 
therefore whether it is worth trading it for code refactoring. Thoughts?
I think it depends on if any behavior get changed (code functionality, log 
exception, etc.) in old client side. If old client doesn't aware this new sub 
exception and still catch it as the parent exception and do the same logic 
which seems to be fine. However, unfortunately, I concerned the case here 
doesn't belongs to this. Let's assume user's AM copy code from 
RMContainerAllocator, it treat YARNException and RESYNC/SHUTDOWN in response 
differently (with different exceptions to indicate different reasons). Now, the 
same reason that cause RESYNC/SHUTDOWN previously get thrown YARNExcetpion 
(though a new sub exception) in new RM and will be handle as the same as other 
reasons. So now the old application can only see one exception which is 
inconsistent behavior.

bq. So the main point is, regardless how application is earlier handling the 
AMCommand, it should continue to work with this change. Existing YARN 
applications will not break because of this. 
IMO, breaking application doesn't means to break app's functionality only. 
Application's inconsistent behavior in the same exception belongs to this case 
also. In general, we may think the latter one doesn't sounds so serious as 
previous one but it affect user's experience (especially they meet exception 
before). It may worth to do so for serious bug fix or significant feature, but 
we should have more justifications for code refactor work.

> Replace AM resync/shutdown command with corresponding exceptions
> ----------------------------------------------------------------
>                 Key: YARN-2209
>                 URL: https://issues.apache.org/jira/browse/YARN-2209
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Jian He
>            Assignee: Jian He
>         Attachments: YARN-2209.1.patch, YARN-2209.2.patch, YARN-2209.3.patch, 
> YARN-2209.4.patch, YARN-2209.5.patch
> YARN-1365 introduced an ApplicationMasterNotRegisteredException to indicate 
> application to re-register on RM restart. we should do the same for 
> AMS#allocate call also.

This message was sent by Atlassian JIRA

Reply via email to