hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bikas Saha (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
Date Wed, 20 Mar 2013 21:53:15 GMT

    [ 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

Mime
View raw message