hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Junping Du (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-6877) Create an abstract log reader for extendability
Date Tue, 29 Aug 2017 21:38:00 GMT

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

Junping Du commented on YARN-6877:
----------------------------------

Thanks [~xgong] for working on a patch. 
The patch is huge. However, mostly are refactoring format binding methods from LogCLIHelpers,
LogToolUtils and AggregatedLogsBlock to new created extensible classes, so still straightforward.
Some review comments:

It seems we have two AggregatedLogsBlock now, both are extended from HtmlBlock. One is under
filecontroller package (new) while the other in web app package (old):
{noformat}
+@InterfaceAudience.LimitedPrivate({"YARN", "MapReduce"})
+public abstract class AggregatedLogsBlock extends HtmlBlock {
{noformat}
What's use case for both AggregatedLogsBlock here? Does old one get dropped/deprecated? If
not, we may need to think some other name for the new class to get rid of confusion here.

Some minor comments: 
In TestLogsCLI.java,
{noformat}
-    Configuration configuration = new Configuration();
+    Configuration configuration = new YarnConfiguration();
{noformat}
Why we need this change? I think both should be working here. Do I miss anything?

In many places, the code are missing space between between '}' and catch
{noformat}
}catch (NumberFormatException ne) {
{noformat}

In LogCLIHelpers.java, I saw we are mixing using err.println() and System.err.println(). Both
way should works fine but better to keep consistent here.

seems like no change in AggregatedLogsBlockForTest.java. If so, better remove it. 

> Create an abstract log reader for extendability
> -----------------------------------------------
>
>                 Key: YARN-6877
>                 URL: https://issues.apache.org/jira/browse/YARN-6877
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: YARN-6877-branch-2.001.patch, YARN-6877-trunk.001.patch, YARN-6877-trunk.002.patch,
YARN-6877-trunk.003.patch, YARN-6877-trunk.004.patch, YARN-6877-trunk.005.patch
>
>
> Currently, TFile log reader is used to read aggregated log in YARN. We need to add an
abstract layer, and pick up the correct log reader based on the configuration.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message