[
https://issues.apache.org/jira/browse/YARN-3240?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14335106#comment-14335106
]
Junping Du commented on YARN-3240:
----------------------------------
Thanks [~zjshen] for the patch! I am reviewing this patch now, and a couple of
comments so far:
{code}
+ //TODO: It needs to be updated by the discovery service
private URI resURI;
{code}
Looks like we are creating one TimelineClient for every application so we have
multiple TimelineClients within NM. Do we think about the other way - one
TimelineClient can talk to different app URLs (put url as a parameter in every
call, so client can be more stateless)? I don't have any preference here and I
think compatibility with old client could be a good reason here. But just
curious on our decisions here.
In addition, I think we need a new constructor to take resURI as a parameter
because this is not get from configuration now but get from caller of
TimelineClient who know the resource details (address of aggregator).
And a setter is also needed to resURI because when caller (AM or NMs) have any
failure in PUT/POST (as IOException so far), its retry logic will notify RM to
recovery (through heartbeat or allocate request, addressed in YARN-3039) and
set it back afterwards.
{code}
catch (RuntimeException re) {
+ // runtime exception is expected if the client cannot connect the server
+ String msg =
+ "Failed to get the response from the timeline server.";
+ LOG.error(msg, re);
+ throw new IOException(re);
+ }
+ if (resp == null ||
+ resp.getClientResponseStatus() != ClientResponse.Status.OK) {
+ String msg =
+ "Failed to get the response from the timeline server.";
+ LOG.error(msg);
+ if (LOG.isDebugEnabled() && resp != null) {
+ String output = resp.getEntity(String.class);
+ LOG.debug("HTTP error code: " + resp.getStatus()
+ + " Server response : \n" + output);
+ }
+ throw new YarnException(msg);
+ }
{code}
Looks like we are differentiate 404 and 500 here with IOException and
YarnException which looks fine to me. Do we plan to have different handling
logic (in caller part) for two failure cases?
Other looks good to me.
> [Data Mode] Implement client API to put generic entities
> --------------------------------------------------------
>
> Key: YARN-3240
> URL: https://issues.apache.org/jira/browse/YARN-3240
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Reporter: Zhijie Shen
> Assignee: Zhijie Shen
> Attachments: YARN-3240.1.patch, YARN-3240.2.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
