[
https://issues.apache.org/jira/browse/YARN-6125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15861864#comment-15861864
]
Daniel Templeton commented on YARN-6125:
----------------------------------------
Looking good, [~andras.piros]. A few more comments:
* On the log and exception messages:
** "Diagnostic messages truncated, showing last (%d) chars out of (%d) in
total:" would be better without the parentheses, and "in total" is redundant.
Should "chars" be "bytes?"
** Instead of "<property> (<value>) should be positive," I'd prefer "The value
of <property> should be a positive integer: <value>."
** "<value> should be a positive integer. Exception message: <message>," I'd
prefer that same message: "The value of <property> should be a positive
integer: <value>." The exception message on a NFE is seldom useful.
** With, "There was a problem appending a diagnostics message to an RM app
attempt. Here is the message: <message>," I'd leave out the last sentence and
reword a little, "There was an issue updating the diagnostic message for RM app
attempt <id>: <message>" But, see my next point...
* In {{appendToDiagnosticsSafely()}}, while {{IndexOutOfBoundsException}} is
theoretically possible, it can't happen in practice. I'm not sure why the
{{IllegalStateException}} is there. Is it safe to drop this method completely
in favor of just calling {{append()}}?
* In {{BoundedAppender}}, I think it would be simpler to always just prepend
the header to the string directly instead of adding it in the {{toString()}}.
* It would be good to have some javadocs to explain why the {{BoundedAppender}}
is there and how it's intended to function.
* Any reason not to name {{currentLength()}} just {length()}}?
* The docs talk about bytes, but the messages and code deal in characters. Not
always the same thing. It's easier to talk about bytes, but easier to code for
characters. Let's pick one or the other.
> The application attempt's diagnostic message should have a maximum size
> -----------------------------------------------------------------------
>
> Key: YARN-6125
> URL: https://issues.apache.org/jira/browse/YARN-6125
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: resourcemanager
> Affects Versions: 2.7.0
> Reporter: Daniel Templeton
> Assignee: Andras Piros
> Priority: Critical
> Fix For: 3.0.0-alpha3
>
> Attachments: YARN-6125.000.patch, YARN-6125.001.patch,
> YARN-6125.002.patch, YARN-6125.003.patch, YARN-6125.004.patch,
> YARN-6125.005.patch, YARN-6125.006.patch
>
>
> We've found through experience that the diagnostic message can grow
> unbounded. I've seen attempts that have diagnostic messages over 1MB. Since
> the message is stored in the state store, it's a bad idea to allow the
> message to grow unbounded. Instead, there should be a property that sets a
> maximum size on the message.
> I suspect that some of the ZK state store issues we've seen in the past were
> due to the size of the diagnostic messages and not to the size of the
> classpath, as is the current prevailing opinion.
> An open question is how best to prune the message once it grows too large.
> Should we
> # truncate the tail,
> # truncate the head,
> # truncate the middle,
> # add another property to make the behavior selectable, or
> # none of the above?
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
