[
https://issues.apache.org/jira/browse/YARN-3505?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14534850#comment-14534850
]
Junping Du commented on YARN-3505:
----------------------------------
Thanks Xuan for updating the patch! The latest patch looks much closer.
bq. Why we need to differentiate diagnosticMessage with failureMessages in
LogAggregationReport? If we don't want to cache successful report, we can
differentiate from LogAggregationStatus. Isn't it?
It seems this previous comment haven't been responsed so far. Just think it
again, I think our intention is trying to differentiate the normal
diagnosticMessage and errorMessages, so it should be fine to track
LogAggregationDiagnosticsForNMs and LogAggregationFailureMessagesForNMs
separatedly in RMAppImpl. However, I think we don't need this differentiation
in LogAggregationReport as in case of FAILED case, diagnosticMessage and
failureMessages store the same message which is duplicated. Isn't it?
Other minor comments:
In RMAppImpl.java,
aggregateLogReport() method sounds too long and need some refactor work here.
Can we abstract some methods like: updateLogAggregationStatus(),
updateLogAggregationDiagnosticMessages(),
updateLogAggregationFailureMessages(), etc.? In addition, what would happen if
(this.logAggregationSucceed + this.logAggregationFailed) !=
this.logAggregationStatus.size()?
{code}
private static int MAX_LOG_AGGREGATION_DIAGNOSTICS_IN_MEMORY = 10;
{code}
Shall we make it configurable? May be as private first (not putting on
yarn-default.xml)?
In AppBlock.java,
{code}
+ protected LogAggregationStatus getLogAggregationStatus() {
+ return null;
+ }
{code}
Can we add a comment to said it has to be overriden in subclass?
{code}
+ th(_TH, "Last 10 Diagnostis Messages").
{code}
Typo, "Diagnostis" => "Diagnostic"
> Node's Log Aggregation Report with SUCCEED should not cached in RMApps
> ----------------------------------------------------------------------
>
> Key: YARN-3505
> URL: https://issues.apache.org/jira/browse/YARN-3505
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: log-aggregation
> Affects Versions: 2.8.0
> Reporter: Junping Du
> Assignee: Xuan Gong
> Priority: Critical
> Attachments: YARN-3505.1.patch, YARN-3505.2.patch,
> YARN-3505.2.rebase.patch
>
>
> Per discussions in YARN-1402, we shouldn't cache all node's log aggregation
> reports in RMApps for always, especially for those finished with SUCCEED.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
