[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13609553#comment-13609553
]
Hadoop QA commented on YARN-417:
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12574689/YARN-417-6.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 tests included appear to have a timeout.{color}
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/560//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/560//console
This message is automatically generated.
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-6.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13609576#comment-13609576 ] Bikas Saha commented on YARN-417: - +1. The new locking also makes sure that the queue.put is outside the lock and wont get synch blocked upon a bounded queue. Thanks for being patient with my reviews! 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-6.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13608004#comment-13608004
]
Hadoop QA commented on YARN-417:
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12574596/YARN-417-5.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 tests included appear to have a timeout.{color}
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/550//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/550//console
This message is automatically generated.
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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606532#comment-13606532 ] Bikas Saha commented on YARN-417: - The client can make a call to stop() before making a call to unregister (bad code, race in code, client got reboot from RM etc) In that case the code may crash according to the sequence I described earlier, wont it? so the safety of unregister having being called before stop is not guaranteed, right? Calling client.stop() after heartbeat.join() is the right thing to do irrespective of any safety offered by some other method call (that may or may not happen). Let us do that unless there is a good reason not to. There was a burden on the app to call heartbeat periodically. We are removing that burden in this jira. A related burden was to update progress periodically which was tied to the heartbeat. From what I see, remembering to update the progress periodically is still a burden on the app. Is it not so? 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606643#comment-13606643 ] Sandy Ryza commented on YARN-417: - bq. The client can make a call to stop() before making a call to unregister (bad code, race in code, client got reboot from RM etc) In that case the code may crash according to the sequence I described earlier, wont it? so the safety of unregister having being called before stop is not guaranteed, right? Calling client.stop() after heartbeat.join() is the right thing to do irrespective of any safety offered by some other method call (that may or may not happen). Let us do that unless there is a good reason not to. I agree. bq. There was a burden on the app to call heartbeat periodically. We are removing that burden in this jira. A related burden was to update progress periodically which was tied to the heartbeat. From what I see, remembering to update the progress periodically is still a burden on the app. Is it not so? The app does not need to update progress periodically, it only needs to update it in response to events that would change its progress, which it needs to do anyway. This of course can be accomplished just as easily by updating a user-stored progress variable and returning it to a getProgress callback. The reason that I am defending the former is that currently we have a clear separation that callbacks are for information relayed from the RM, and AMRMClientAsync direct method calls like newContainerRequest, setProgress, and registerApplicationMaster are for information to the RM. Having gotten that out, I'll accept whatever approach seems best to you, Bikas. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606761#comment-13606761 ] Bikas Saha commented on YARN-417: - I see. I agree its a difference in design perspective. For now, lets get in the current approach because it looks good overall. We can always make changes/improvements as needed. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13606944#comment-13606944
]
Bikas Saha commented on YARN-417:
-
Looking at the patch again, in order to make it consistent with the callback vs
set approach, the following getters need to become new/existing callbacks,
right? Callback to client when the RM tells us about a change in
availableResource/clusterNodeCount?
{code}
+ /**
+ * Get the currently available resources in the cluster.
+ * A valid value is available after a call to allocate has been made
+ * @return Currently available resources
+ */
+ public Resource getClusterAvailableResources() {
+return client.getClusterAvailableResources();
+ }
+
+ /**
+ * Get the current number of nodes in the cluster.
+ * A valid values is available after a call to allocate has been made
+ * @return Current number of nodes in the cluster
+ */
+ public int getClusterNodeCount() {
+return client.getClusterNodeCount();
+ }
{code}
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607000#comment-13607000 ] Sandy Ryza commented on YARN-417: - That's a good point. I'm worried about taking it in this direction, though, for reasons of future compatibility. If we decide to later include additional information with an AMResponse, staying consistent with this approach would require us to add additional methods to the callback handler, which would break API compatibility. I don't have a clear answer on the best way to deal with this. One option would be to make the callback handler an abstract class and not force users to implement methods like getClusterAvailableResources. Another would be to have a callback that returns the entire AMResponse. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607063#comment-13607063 ] Bikas Saha commented on YARN-417: - Hmmm. Lets fix the last few comments above and commit whats in there, given that its a useful first step. This is evolving and using it while writing applications will suggest directions. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13607067#comment-13607067 ] Bikas Saha commented on YARN-417: - On that note, we should annotate the classes/interfaces with @Unstable and @Evolving tags in the final patch. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13605415#comment-13605415 ] Sandy Ryza commented on YARN-417: - bq. Is it related to the exception being thrown when stop() is called on the handler thread? Is this guaranteed bad behavior and so we need to throw a runtime exception immediately? It is guaranteed bad behavior - a thread that tries to join with itself will wait indefinitely. bq. I think we need to call client.stop() after the heartbeat thread has stopped. If unregisterApplicationMaster() is called before stop, this is handled by setting keepRunning to false there and synchronizing on the AMRMClient in run(). bq. Didnt quite get the assert inside the loop. Perhaps you meant takeCompletedContainers()? It just makes sure that the allocate callback isn't called twice, which I guess is a pretty weak assert because there's no good reason that would happen. I can take it out if you think it makes things less clear. bq. I think updating progress needs to be its own callback since its possible that no container allocations and completions happen for a long time and thus the heartbeats show no progress to the RM. When AMRMClientAsync#setProgress is called, the updated progress will be transmitted with the next heartbeat. This can be called whenever something occurs that advances the app's progress, not just on callbacks. Would this not work with some ways of using this client that I am not thinking of? 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13603164#comment-13603164
]
Bikas Saha commented on YARN-417:
-
What is the deadlock here? Its late night and I cant see it :P
Is it related to the exception being thrown when stop() is called on the
handler thread? Is this guaranteed bad behavior and so we need to throw a
runtime exception immediately?
I think we need to call client.stop() after the heartbeat thread has stopped.
otherwise, the heartbeat thread can call client.allocate() in between the
current client.stop() and keepRunning=false, right?
{code}
+ /**
+ * Tells the heartbeat and handler threads to stop and waits for them to
+ * terminate. Calling this method from the callback handler thread would
cause
+ * deadlock, and thus should be avoided.
+ */
+ @Override
+ public void stop() {
+if (Thread.currentThread() == handlerThread) {
+ throw new YarnException(Cannot call stop from callback handler
thread!);
+}
+client.stop();
+keepRunning = false;
+try {
+ heartbeatThread.join();
{code}
Didnt quite get the assert inside the loop. Perhaps you meant
takeCompletedContainers()?
{code}
+// wait for the allocated containers from the first heartbeat's response
+while (callbackHandler.takeAllocatedContainers() == null) {
+ Assert.assertEquals(null, callbackHandler.takeAllocatedContainers());
+ Thread.sleep(10);
+}
{code}
I think updating progress needs to be its own callback since its possible that
no container allocations and completions happen for a long time and thus the
heartbeats show no progress to the RM.
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13599380#comment-13599380
]
Hadoop QA commented on YARN-417:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12573184/YARN-417-4.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 tests included appear to have a timeout.{color}
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:red}-1 eclipse:eclipse{color}. The patch failed to build with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/496//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/496//console
This message is automatically generated.
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13598084#comment-13598084 ] Sandy Ryza commented on YARN-417: - Updated patch includes the changes suggested by Bikas above. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13598106#comment-13598106
]
Hadoop QA commented on YARN-417:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12572936/YARN-417-4.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 tests included appear to have a timeout.{color}
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common:
org.apache.hadoop.yarn.client.TestAMRMClientAsync
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/490//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/490//console
This message is automatically generated.
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13594879#comment-13594879
]
Chris Riccomini commented on YARN-417:
--
Hey Bikas,
Not sure what you mean here:
bq. available resource should be first thing notified to client because it can
affect how it allocates the new containers
Are you suggesting reversing the order of this code:
{code}
+ListContainerStatus completed =
+amResponse.getCompletedContainersStatuses();
+if (!completed.isEmpty()) {
+ handler.onContainersCompleted(completed);
+}
+
+ListContainer allocated = amResponse.getAllocatedContainers();
+if (!allocated.isEmpty()) {
+ handler.onContainersAllocated(allocated);
+}
{code}
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13594892#comment-13594892 ] Bikas Saha commented on YARN-417: - No. There is a field in the response thats currently missing from the callback - availableResources. that should be the first callback. That show how much spare resources are available to the app and can be used to change strategy. eg. MR uses it to determine how many reduces it can start optimistically while leaving enough space to complete the tail end of the map phase. In some ways, the availableResources, completedContainers and allocatedContainers are a unit of information. We might be making it harder to reason about them by breaking the information into separate chunks of onAllocated(), onCompleted() etc and providing them at different times. Of course this matters more to sophisticated schedulers. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13595378#comment-13595378 ] Hitesh Shah commented on YARN-417: -- @Bikas, @Chris, is there a reason as to why all information needs to be sent back via callbacks? Could the AsyncAMRMClient cache the last known available resources information obtained from the RM and provide a function to access it as needed. Assuming the application master has been written to handle events, it is very likely that when deciding what to ask the next time around, it could simply query the current available resources and make a decision based on that info. +1 to Bikas's comment on separate callbacks for the allocation and completion sets - complex AMs may find that the order of callbacks create problems as each set will be handled separately without an overall picture. For simpler AMs, this may suffice which I am assuming is the goal here. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13593689#comment-13593689 ] Bikas Saha commented on YARN-417: - Please do use join and make sure all the threads are complete and get cleaned up. It may be a convenience to call stop() in the callback but I dont think we should encourage that. It might be fine now but not future proof. Its not uncommon for API's to require users to be well behaved. As far as such use cases are concerned, since we have made this class an AbstractService, the common use case would be to init and start the service in the beginning and the stop it in the end, similar to other services. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13594316#comment-13594316 ] Bikas Saha commented on YARN-417: - available resource should be first thing notified to client because it can affect how it allocates the new containers. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13592371#comment-13592371 ] Chris Riccomini commented on YARN-417: -- Looks good to me! 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13592777#comment-13592777
]
Bikas Saha commented on YARN-417:
-
Calling client.registerApp() before client.start() and client.stop() before
client.unregister() is not in line with the Service interface. Services need to
be started before used and stopped after using them. Also, adding a number of
services as part of a composite service is a common pattern. In that, all
services are added, inited, started, used and then stopped . The composite
service takes care of ordering between services. In such use cases, it may not
possible to call interface methods out of order as is being done here. We could
enhance the heartbeater to not heartbeat until register is called. or we could
start the heartbeater after registration is complete. The latter approach makes
more sense to me.
I am surprised that the DistShell code is calling resourceManager.stop() and
then resourceManager.unregister() because stop() eventually call
AMRMClientImpl.stop() that shuts down the proxy. After that, unregister() call
on AMRMClientImpl should fail.
Why are we calling client.start() in the init() method and not at the beginning
of the start method()? Perhaps related to the above comment.
{code}
+ @Override
+ public void init(Configuration conf) {
+super.init(conf);
+client.init(conf);
+client.start();
+ }
{code}
Why not wait for the handlerThread to join()? The comment does not match the
code for the heartbeat thread.
{code}
+ /**
+ * Tells the heartbeat thread to stop, but does not wait for it to return.
+ */
+ @Override
+ public void stop() {
+client.stop();
+keepRunning = false;
+try {
+ heartbeatThread.join();
+} catch (InterruptedException ex) {
+ LOG.error(Error joining with heartbeat thread, ex);
+}
+handlerThread.interrupt();
+ }
{code}
In general, it would be good to spend some thought on the thread safety of the
new class. Both external calls from the app and the internal producer/consumer
race between the heartbeat and callback threads. During startup, execution and
shutdown. I havent thought through them but the almost complete absence of any
synchronization made be wonder if it was by design.
I would prefer queue.put() which blocks on capacity instead of queue.add() to
mirror queue.take().
Could save some time using wait/notify? Important for end to end tests time.
{done}
+while (!done) {
+ try {
+Thread.sleep(1000);
+ } catch (InterruptedException ex) {}
+}
{done}
Looks like this is only for tests. If yes, how about making it package private
and annotating with @Private and @VisibleForTesting.
{code}
+ public AMRMClientAsync(AMRMClient client, int intervalMs,
+ CallbackHandler callbackHandler) {
{code}
A committer once told me that the philosophy behind BuilderUtils it to pass all
members of the object being built and use it as a completely defined
constructor so that folks dont miss passing any member fields by accident. So I
guess nodeUpdates and reboot should also be passed in as arguments.
{code}
+ public static AMResponse newAMResponse(
+ ListContainerStatus completedContainers,
+ ListContainer allocatedContainers) {
{code}
I would like the test code to not exemplify incorrect use of the class. The
test is calling allocate without call register and it all works. Maybe if we
fixed the first comment in this review then it wont allow such incorrect usage.
Secondly, folks tend to look at test code to see usage of a class and so
showing incorrect usage is not a good idea IMO.
{code}
+AMRMClientAsync asyncClient = new AMRMClientAsync(client, 200,
callbackHandler);
+asyncClient.init(conf);
+asyncClient.start();
+
+while (callbackHandler.takeAllocatedContainers() == null) {
+
{code}
This code can lead to a flaky test. If I understand the flow correctly the
following can happen. CallbackHandler populates allocatedContainers and OS
pauses it. In the meanwhile heartbeater has already given completedContainers.
The main thread then takesAllocatedContainers and it pauses. The
CallbackHandler then returns and onCompletedContainers() is called which
populates completed containers. Then it pauses. The main thread executes
takeCompletedContainers() which returns non-null and the Assert fails. Is this
a correct understanding? If yes, we should make sure that the test does not end
up being flaky. In general sleep() should be avoided because it makes tests
slow and tend to be flaky. I agree in some case, sleep is hard to avoid when
the test is running an inline service whose timing we cannot control or when
the effort to do so is too large. But in this case where all the code is test
code or mock code, we could avoid sleeping.
{code}
+while (callbackHandler.takeAllocatedContainers() == null) {
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13593023#comment-13593023 ] Sandy Ryza commented on YARN-417: - Thanks for the detailed comments, Bikas. Other than what's discussed below, I'll make the changes you suggest (switch to wait/notify, you're right about the race in the test, will have the heartbeater start in the register method, etc.) bq. Why not wait for the handlerThread to join()? My thought was that the user should be able to call stop() from the callback handler and not deadlock. Even if we were to explicitly warn against this, users would be likely to try it anyway and encounter difficulties. Regarding synchronization, I had put some thought into it, and my understanding is that it should work without synchronized methods. A coarse version of the thinking behind this is: * All the methods of AMRMClientAsync other than init(), start(), and stop() do not touch any variables in AMRMClientAsync and delegate to AMRMClient. AMRMClient handles the interleaving of any of these methods with each other, and interleaving them with start(), stop(), and init(). * If any of these methods are interleaved with stop(), there will be no problem. * Calling any of these methods before or at the same time as init() or start() is incorrect use of the class, and can cause problems even if the methods are synchronized. Additionally, after the start/register change you proposed, all that init() and start will do is delegate to AMRMClient anyway. Let me know if you see anything I'm missing. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13591533#comment-13591533
]
Hadoop QA commented on YARN-417:
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12571758/YARN-417-3.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 tests included appear to have a timeout.{color}
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/456//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/456//console
This message is automatically generated.
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13591027#comment-13591027
]
Hadoop QA commented on YARN-417:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12571664/YARN-417-2.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/453//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/453//console
This message is automatically generated.
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13589785#comment-13589785 ] Sandy Ryza commented on YARN-417: - bq. I think if ContainerExitCodes needs to be added then it should be its own jira Will move the container exit codes in a separate JIRA. bq. The helper function would have helped because containers contain information set by 2 entities... The issue is that there is not a ton of information for a helper function to interpret. From what I can tell, The framework only defines two special exit codes, and does not distinguish between OOMs and other kinds of container failures, or between killing a container because it was preempted or because the RM lost track of it. These exit codes are platform independent, and any other exit codes can be both application and platform dependent, so the AMRMClientAsync wouldn't know how to interpret them. As ContainerStatuses coming from the RM are only in the context of container completions, ContainerState provides no extra information. Additional information can sometimes be found in the diagnostics strings, but if the reasons that containers die are to be codified, I don't think it should be done by interpreting strings at the API level. bq. Why is client.start() being called in init? client.stop() is being called in stop(). registerApplicationMaster needs to be called after setting up the RM proxy, which occurs in AMRMClient#start, but before starting the heartbeater, which occurs in AMRMClientAsync#start. Another way to accomplish this would be to move the code in AMRMClientImpl#start to AMRMClientImpl#init, which also seems reasonable to me. A third way would be to call registerApplicationMaster from AMRMClientAsync#start. bq. I am wary of calling back on the heartbeat thread itself. Will add a handling thread. bq. Not waiting for the thread to join()? Why interrupt()? Thread needs to be stopped first so that it stops calling into the client. or else it can call into a client that has already stopped. Good point. My reason was that I've seen this as convention other places in YARN (see NodeStatusUpdaterImpl, for example), and that it would allow stop to be called from onContainerCompleted without deadlock, but with the handling thread, the latter shouldn't be a problem, so I'll change it. 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13588501#comment-13588501
]
Karthik Kambatla commented on YARN-417:
---
Thanks Bikas, the interface looks good. One comment though - shouldn't we move
{{ContainerCompletionStatus}} and {{getContainerCompletionStatus}} to
{{ContainerStatus}}?
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.java, 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13588552#comment-13588552 ] Sandy Ryza commented on YARN-417: - Thanks for the interface proposal, Bikas. It looks good to me. Having a separate method invocation for each completed container shouldn't have significant performance impact, as Java inlines even virtual methods when it needs to (http://www.quora.com/How-many-CPU-instructions-are-typical-for-Java-method-call-overhead), but the single call with the list doesn't seem any worse to me. Nits: * missing an onReboot method * ContainerCompletionStatus is describing a cause more than a state, so a name like ContainerCompletionReason fits a little better to me * agree with Karthik that it would be much more intuitive for getContainerCompletionStatus to be in ContainerStatus. Is there a strong reason against this? Attaching an updated proposal 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.java, 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13588671#comment-13588671 ] Chris Riccomini commented on YARN-417: -- Hey Guys, I also agree with the comments and nits that Sandy/Karthik pointed out. Just so I'm clear, the way this would be used a main thread would be: 1. instantiate 2. call init 3. call start 4. call register 5. make initial container request 6. wait until containers complete I think what I'd probably end up doing for #6 is just using a countdown latch that I'd wait on, and the callback decrements whenever a container completes. Probably good enough. Cheers, Chris 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13589177#comment-13589177 ] Sandy Ryza commented on YARN-417: - That's sounds right Chris. Will include that in the class's doc. I've thought a little more about the ContainerCompletionReason, and I'm not sure it's necessary, as there are already constants in YarnConfiguration for the special exit codes, and there are only two, ABORTED_CONTAINER_EXIT_STATUS and DISK_FAILED. As these don't really have to do with configuration, it might make sense to move them to a ContainerExitCodes class, and just point to that class in the doc for ContainerStatus#getExitCode and CallbackHandler#onContainersCompleted 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13589182#comment-13589182 ] Sandy Ryza commented on YARN-417: - Uploaded a second cut with what was discussed above. One more thought: would it make sense to take ContainerRequest out of AMRMClient as its now used in places where AMRMClient is not? 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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13589185#comment-13589185
]
Hadoop QA commented on YARN-417:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12571354/YARN-417-1.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:red}-1 javac{color:red}. The patch appears to cause the build to
fail.
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/445//console
This message is automatically generated.
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.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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13587225#comment-13587225 ] Chris Riccomini commented on YARN-417: -- Hey Sandy, I think skipping onInit and onShutdown is fine. That stuff can be handled in the method that's actually instantiating/running the heartbeater. So, to be concise: onReboot, onContainerLost (-100 return code), onContainerFailed (return code != 0 and != -100), onContainerSuccess (0 return code), onContainerAllocated. One other callback that is worth considering is onContainerOutOfMemory (-1 return code). I'm not sure how fine grained we want to get, but all these nuanced return codes that are hidden in the Java docs and whatnot make it difficult to write AMs. Doing as much work as possible for the framework developer would be a nice thing, in my opinion. One thing that we need to be careful about is how a user would request containers using this callback approach and the AMRM. This line, from the Javadocs, scares me, App should not make concurrent allocate requests. May cause request loss. I think this means that AMRMClient.allocate can only be called in the heartbeat loop, and the user should use AMRMClient.addContainerRequest and AMRMClient.releaseAssignedContainer in the callbacks whenever it wants to get or release new containers. Bikas, is this a correct reading? Cheers, Chris 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: 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13587233#comment-13587233
]
Chris Riccomini commented on YARN-417:
--
My first comment would be to see if we can provide this functionality via a
single object instead of a combination of AMRMClient, Heartbeater and
AllocationListener. Every new object and its combination with others
increases mental complexity.
Yeah, this was my initial inclination as well. I wanted some way to eliminate
the AMRMClient (applicationMasterHelper, in my case) by having the callbacks
return objects that signaled what they wanted. I couldn't come up with a scheme
that was simpler than just using the AMRMClient, though. If you guys can come
up with something that works, and is clean, I'm all ears.
Also, while callbacks are a reasonable approach, it would be good to spend a
little time thinking about other notification mechanisms like events or
futures.
Hmm, this is an interesting thought. Generally, I find futures easier to work
with, but as Sandy pointed out, I'm not sure how well it fits this model, where
you get updates that trickle in. It almost seems like a CountDownLatch for a
resource request could be used, but that seems more complex to deal with than a
callback. To riff on Sandy's API example, I could imagine something like:
{code}
ListContainer myAllocatedContainers =
heartbeater.submitContainerRequests().getContainers(3);
{code}
To wait for three containers before proceeding. You could also have
getContainers(), which would wait for all. This is very CountDownLatch-ish. I'm
not entirely sure how you might model the onReboot and failure/complete
scenarios, though. Thoughts? My imagination is failing me.
Something else to think about. completedContainers and allocatedContainers
are essentially coming at the same time. Breaking them into 2 calls may open
up ordering issues.
I had the same inclination that Sandy did, which was that I can see the need
for onComplete* before onAllocated, so that I can properly re-assign work to
allocated containers if an existing container failed, but beyond that, I
couldn't come up with a reason why you might want onAllocated before
onComplete*. I guess the question isn't so much about ordering, it's really: is
there ever a case where you need to know both what was allocated AND what was
completed at the same time, or in total (i.e. not one at a time)? I can't think
of anything.
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: 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13587312#comment-13587312 ] Bikas Saha commented on YARN-417: - Yes. add/release should be used instead of allocate because allocate makes an RPC that is subject to failures and race conditions. Its all handled in AMRMClient. While I like onContainer*() we need to think about proliferation of such methods. A huge method listing tends to make me cautious because I have to understand all those methods. Also, I have a gut feeling that there might be a lot of user code replication for each callback. I like it when code is in one place. How about an onContainerGoodName() and then provide a helper method that tells whether the container is completed, successfull, killed, outofmemory. IMO, this might be more flexible and less verbose. And the user handling logic is in one place. One other thought, is that where possible, the callbacks should return collection/list of objects. Say, we have a large job and we are getting 100's of containers. For perf, it might be better to not have 100's of calls for onContainerAllocated() and onContainerCompleted(). Now that we have 2 example codes and some discussion, lets post a concrete interface declaration with javadoc. That will help focus the discussion and we can converge quickly. 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: 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13587320#comment-13587320 ] Karthik Kambatla commented on YARN-417: --- How about following Netty kind of asynchrony? YARNContainerFuture for waiting purposes, and onContainerComplete(ContainerStatus) for asynchronous notification? 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: 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13586051#comment-13586051 ] Bikas Saha commented on YARN-417: - Sandy, it will be beneficial if you can mention if the patch is work in progress (like this one seems) or not. And also describe it a little bit more clearly. 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: YARN-417.patch 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[ https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13586092#comment-13586092 ] Bikas Saha commented on YARN-417: - My first comment would be to see if we can provide this functionality via a single object instead of a combination of AMRMClient, Heartbeater and AllocationListener. Every new object and its combination with others increases mental complexity. Also, while callbacks are a reasonable approach, it would be good to spend a little time thinking about other notification mechanisms like events or futures. Something else to think about. completedContainers and allocatedContainers are essentially coming at the same time. Breaking them into 2 calls may open up ordering issues. What is the use of allocationResponse? If we call it on every heartbeat then we are implicitly exposing it anyways. I can see the point about flexibility of going one way or the other, but in this case, I think we should take a stand about what exactly we want to expose to the user to make their life easy. Lets figure out what the user interaction surface looks like and carefully create new APIs. 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: YARN-417.patch 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13586702#comment-13586702
]
Sandy Ryza commented on YARN-417:
-
Thanks for the comments Bikas, and for your perspective, Chris.
Chris,
Your code brings up that we should probably have a reboot callback as well. I
also like the idea of onContainerLost/onContainerFail/onContainerSuccess. Do
you have an opinion on whether more of the lifecycle should be handled here?
My initial inclination would be to do that separately/on top of the async
client.
Bikas,
To reduce the number of classes a user needs to deal with, we could have
Heartbeater wrap the AMRMClient methods and rename it something like
AsyncAMRMClient. To also eliminate the AllocationListener interface, we could
have the user subclass Heartbeater? For the latter, though, in this case to me
registering callbacks seems like an easier mental model than subclassing, as it
better separates the client from the user code in the user's mind. But I don't
feel super strongly about this.
The futures idea is interesting - if submitContainerRequest returned a future
it would be cool that someone could scheduler containers just using:
{code}
ListContainer myAllocatedContainers =
heartbeater.submitContainerRequests().getContainers();
{code}
However, I don't think this captures enough of a majority of use cases, as many
YARN apps will want to start containers asynchronously as they trickle back in.
Regarding the callbacks, I didn't notice that the info returned in AMResponse
is stored in AMRMClient. As the user can get access to it that way, I agree,
Bikas, it's probably clearer to take out the allocateResponse method.
Regarding the completedContainers and allocatedContainers as two methods or
one, I don't feel super strongly about either approach, but if all else is
equal, to me it seems easier for the developer to think about them separately,
especially with reboot and if we want to do the
onContainerLost/onContainerFail/onContainerSuccess that Chris mentioned. In
java awt/swing, mouseReleased and mouseClicked events come at the same time,
rather than mouseReleased containing an isClick param, and this always seemed
intuitive to me. I can see that it might be useful to know what containers
were finished before knowing what containers were allocated, but I can't come
up with a reason for the other way around.
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: 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
[jira] [Commented] (YARN-417) Add a poller that allows the AM to receive notifications when it is assigned containers
[
https://issues.apache.org/jira/browse/YARN-417?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13585482#comment-13585482
]
Hadoop QA commented on YARN-417:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12570682/YARN-417.patch
against trunk revision .
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 1.3.9) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/425//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/425//console
This message is automatically generated.
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: New Feature
Components: api, applications
Affects Versions: 2.0.3-alpha
Reporter: Sandy Ryza
Assignee: Sandy Ryza
Attachments: YARN-417.patch
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
