[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13608279#comment-13608279
]
Bikas Saha commented on YARN-417:
---------------------------------
IMO, the locking intent will be more clear if we set keepRunning inside the
lock because essentially that is also a shared value that we are guarding. The
client.allocate() and client.unregister() are themselves already synchronized
on the inner rmClient.
{code}
+ public void unregisterApplicationMaster(FinalApplicationStatus appStatus,
+ String appMessage, String appTrackingUrl) throws YarnRemoteException {
+ keepRunning = false;
+ synchronized (client) {
+ client.unregisterApplicationMaster(appStatus, appMessage,
appTrackingUrl);
+ }
{code}
I guess now the outer while loop can actually become a while(true) with the
inner check for if(keepRunning) causing a break when it fails. I like this
pattern because then, when I read the code, I clearly see that the outer loop
is purely a run-to-infinity loop and I dont have to keep that condition in mind
when I try to grok the inner if condition that actually controls the loop
action. What do you think?
{code}
+ while (keepRunning) {
+ try {
+ AllocateResponse response;
+ synchronized (client) {
+ // ensure we don't send heartbeats after unregistering
+ if (keepRunning) {
+ response = client.allocate(progress);
{code}
Your comments on usage of the async client dont mention anything about the
callbacks in the exemplary code flow (which is essentially the new changes in
this jira) :)
The patch needs to be rebased because YARN-396 went in that merged AMResponse
into AllocateResponse.
> Add a poller that allows the AM to receive notifications when it is assigned
> containers
> ---------------------------------------------------------------------------------------
>
> Key: YARN-417
> URL: https://issues.apache.org/jira/browse/YARN-417
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: api, applications
> Affects Versions: 2.0.3-alpha
> Reporter: Sandy Ryza
> Assignee: Sandy Ryza
> Attachments: AMRMClientAsync-1.java, AMRMClientAsync.java,
> YARN-417-1.patch, YARN-417-2.patch, YARN-417-3.patch, YARN-417-4.patch,
> YARN-417-4.patch, YARN-417-5.patch, YARN-417.patch, YarnAppMaster.java,
> YarnAppMasterListener.java
>
>
> Writing AMs would be easier for some if they did not have to handle
> heartbeating to the RM on their own.
--
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
