[
https://issues.apache.org/jira/browse/YARN-2837?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14225490#comment-14225490
]
Li Lu commented on YARN-2837:
-----------------------------
Hi [~zjshen], I reviewed your patch. Generally it looks good. I have the
following comments:
1. If I understand this patch correctly, storing and loading timeline server
status is asymmetric: stores happen each time
TimelineDelegationTokenSecretManager's methods are called, while loads only
happen when TimelineDelegationTokenSecretManagerService starts. This may look
counterintuitive, but it makes sense because in this way the state store
service can keep the status updated. One question is, are we planning to
support more types of status information in the future? Though totally fine for
now, having separate state store methods may not be a scalable approach if we
want to support more status info (if we do not, I think it's fine).
2. The name of MemoryTimelineServerStateStoreService.java looks weird. Other
similar files are all named as \*TimelineServiceStateStoreService. Seems like a
typo because the class inside is also named as \*TimelineService\*. (Names are
too long to notice the difference?)
3. Line 244 of LeveldbTimelineServiceStateStoreService.java is too long. It has
86 characters.
4. From Line 291 of LeveldbTimelineServiceStateStoreService.java:
{code}
/**
* Creates a domain entity key with column name suffix, of the form
* TOKEN_MASTER_KEY_ENTRY_PREFIX + sequence number.
*/
{code}
This is inconsistent with the actual case in the code. In the code it shows
that the master key's key in Leveldb is its key id.
5. I noticed you made the KeyBuilder class in LeveldbTimelineStore.java to be
public, and then annotated it as {{@Private}}. Shall we further restrict its
visibility to YARN only (by {{@LimitedPrivate}})? I think we just need
KeyBuilder to be "public" to be used by
LeveldbTimelineServiceStateStoreService, right?
> Timeline server needs to recover the timeline DT when restarting
> ----------------------------------------------------------------
>
> Key: YARN-2837
> URL: https://issues.apache.org/jira/browse/YARN-2837
> Project: Hadoop YARN
> Issue Type: New Feature
> Components: timelineserver
> Reporter: Zhijie Shen
> Assignee: Zhijie Shen
> Priority: Blocker
> Attachments: YARN-2837.1.patch
>
>
> Timeline server needs to recover the stateful information when restarting as
> RM/NM/JHS does now. So far the stateful information only includes the
> timeline DT. Without recovery, the timeline DT of the existing YARN apps is
> not long valid, and cannot be renewed any more after the timeline server is
> restarted.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
