hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sangjin Lee (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-4356) ensure the timeline service v.2 is disabled cleanly and has no impact when it's turned off
Date Thu, 10 Dec 2015 01:25:11 GMT

    [ https://issues.apache.org/jira/browse/YARN-4356?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15049817#comment-15049817

Sangjin Lee commented on YARN-4356:

Thanks [~djp] for your comments. I addressed most of your comments in the new patch. The following
is a response to your comments.

bq. We should also add back types as requirement/convension for generics.

This is the java 7 diamond operator (<>) which is a shorthand for inferring types. The
type information is NOT removed. It's inferred by the compiler, and the compiler produces
the same bytecode as specifying the types.

bq. This check of null is unnecessary as the only caller - NMCollectorService is only running
under v2 is enabled. If for some reason, we get NPE here which is still better than we ignore
it silently.

That's a good catch. I agree that it's a little better not to check for null here. It's changed
in the latest patch.

bq. Please remove this unrelated change out for more focus and better tracking.

Agreed. I originally removed it because it was an unused private method, but I put it back

bq. Just a reminder, keepAliveApps is a wrong list to identify running apps on specific node.
YARN-3586 (with patch) is already filed to fix this. We can either merge that patch in or
rebase that patch when this patch done.

Got it. Can we proceed with the current patch and get that fix once YARN-3586 goes in?

In TimelineServiceV2Publisher.java,
- * This class is responsible for posting application, appattempt & Container
+ * This class is responsible for posting application, appattempt &amp; Container
Why we need this change?

This is addressing a javadoc error. The ampersand ("&") is a special character for javadoc,
and it breaks javadoc. It needs to be entity-escaped:
[ERROR] /testptch/hadoop/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/metrics/TimelineServiceV2Publisher.java:61:
error: bad HTML entity
[ERROR] * This class is responsible for posting application, appattempt & Container

bq. We should disable PerNodeTimelineCollectorsAuxService if we don't enable timeline service
v2. Isn't it? If so, I think this is not a necessary change and we should remove.

This is used for the test method launchServer(). This method is invoked directly by a unit
test (thus the @VisibleForTesting annotation). The same for TimelineReaderServer.

bq. In addition, I think we should split the change that duplicated with YARN-3623 and cherry-pick
it from trunk/branch-2 when that patch get commit in.

That's fine. I still put up the patch that includes a version of that because without it things
won't even compile. I will wait until YARN-3623 goes in before I remove that piece from this
patch, then this can get committed.

Let me know if this answers your questions.

> ensure the timeline service v.2 is disabled cleanly and has no impact when it's turned
> ------------------------------------------------------------------------------------------
>                 Key: YARN-4356
>                 URL: https://issues.apache.org/jira/browse/YARN-4356
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-2928
>            Reporter: Sangjin Lee
>            Assignee: Sangjin Lee
>            Priority: Critical
>              Labels: yarn-2928-1st-milestone
>         Attachments: YARN-4356-feature-YARN-2928.002.patch, YARN-4356-feature-YARN-2928.003.patch,
YARN-4356-feature-YARN-2928.004.patch, YARN-4356-feature-YARN-2928.005.patch, YARN-4356-feature-YARN-2928.poc.001.patch
> For us to be able to merge the first milestone drop to trunk, we want to ensure that
once disabled the timeline service v.2 has no impact from the server side to the client side.
If the timeline service is not enabled, no action should be done. If v.1 is enabled but not
v.2, v.1 should behave the same as it does before the merge.

This message was sent by Atlassian JIRA

View raw message