[
https://issues.apache.org/jira/browse/YARN-3044?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14538437#comment-14538437
]
Zhijie Shen commented on YARN-3044:
-----------------------------------
Sorry to put my comments at last minute:
1. I'm still not sure why it is necessary to have RMContainerEntity. Whether
the container entity comes from RM or NM, it's about container's info. Any
reason we want differentiate both? At reader side, if I want to list all
containers of an app, should I return RMContainerEntity or ContainerEntity? I
incline to only having ContainerEntity, but RM and NM may put different
info/event about it based on their knowledge.
2. Should v1 and v2 publisher only differentiate at publishXXXXEvent, however,
it seems that we duplicate code more than that. And perhaps defining and
implementing SystemMetricsEvent.toTimelineEvent can further cleanup the code.
3. I saw v2 is going to send config, but where the config is coming from. Did
we conclude who and how to send the config? IAC, sending config seems to be
half done. And we can use {{entity.addConfigs(event.getConfig());}}. No need to
iterate over config collection and put each config one-by-one.
4. yarn.system-metrics-publisher.rm.publish.container-metrics ->
yarn.rm.system-metrics-publisher.emit-container-events?
{code}
374 public static final String RM_PUBLISH_CONTAINER_METRICS_ENABLED =
YARN_PREFIX
375 + "system-metrics-publisher.rm.publish.container-metrics";
376 public static final boolean
DEFAULT_RM_PUBLISH_CONTAINER_METRICS_ENABLED =
377 false;
{code}
Moreover, I also think we should not have
"yarn.system-metrics-publisher.enabled" too, and reuse the existing config. And
it's not limited to RM metrics publisher, but all existing ATS service. IMHO,
the better practice is to reuse the existing config. And we can have a global
config (or env var) timeline-service.version to determine the service is
enabled with v1 or v2 implementation. Anyway, it's a separate problem, I'll
file a separate jira for it.
5. Methods/innner classes in SystemMetricsPublisher don't need to be changed to
"public". Default is enough to access them?
> [Event producers] Implement RM writing app lifecycle events to ATS
> ------------------------------------------------------------------
>
> Key: YARN-3044
> URL: https://issues.apache.org/jira/browse/YARN-3044
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: timelineserver
> Reporter: Sangjin Lee
> Assignee: Naganarasimha G R
> Labels: BB2015-05-TBR
> Attachments: YARN-3044-YARN-2928.004.patch,
> YARN-3044-YARN-2928.005.patch, YARN-3044-YARN-2928.006.patch,
> YARN-3044-YARN-2928.007.patch, YARN-3044.20150325-1.patch,
> YARN-3044.20150406-1.patch, YARN-3044.20150416-1.patch
>
>
> Per design in YARN-2928, implement RM writing app lifecycle events to ATS.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
