[
https://issues.apache.org/jira/browse/YARN-4675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15828780#comment-15828780
]
Varun Saxena commented on YARN-4675:
------------------------------------
Thanks Naga for the patch. Had a brief look at the patch.
# In RMContainerAllocator#getResources, we should be getting V2 Client from
RunningAppContext to update the timeline address.
# setTimelineServiceAddress(String) may not be required as an abstract method
in TimelineClient class.
# In ApplicationMaster in distributed shell, we have an empty if block with a
check for timeline service v2. Can be removed or were you intending to code
something there?
# There is a still a bit of V2 code in TimelineClientImpl. Polling for timeline
service address, one version of putObjects, etc. is used exclusively for ATSv2.
# I think we can move all the connection related and retry related stuff to
TimelineServiceHelper class. We still have it in TimelineClientImpl.
TimelineURLConnectionFactory exists in both the classes for instance. We can
probably give a more suitable name to this helper class as well if all we have
in here is connection establishment and retry related code.
# In ApplicationMaster#finish (Distributed shell code), we are stopping only v1
client.
# In ApplicationMaster#finish, we check for not null only for v1 client while
publishing some entities. This would mean on finish, v2 entities wont be
published. We should make relevant checks.
# Same problem as above in NMCallBackHandler#onContainerStarted. We can
probably check for similar issues at other places in code too.
# In JobHistoryEventHandler#serviceStop, we should call
timelineV2Client.stop(); instead of timelineV2Client.start();
Can probably have a more detailed review upon next patch update.
> Reorganize TimeClientImpl into TimeClientV1Impl and TimeClientV2Impl
> --------------------------------------------------------------------
>
> Key: YARN-4675
> URL: https://issues.apache.org/jira/browse/YARN-4675
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Reporter: Naganarasimha G R
> Assignee: Naganarasimha G R
> Labels: YARN-5355, yarn-5355-merge-blocker
> Attachments: YARN-4675.v2.002.patch, YARN-4675-YARN-2928.v1.001.patch
>
>
> We need to reorganize TimeClientImpl into TimeClientV1Impl ,
> TimeClientV2Impl and if required a base class, so that its clear which part
> of the code belongs to which version and thus better maintainable.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
