[
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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
