[
https://issues.apache.org/jira/browse/YARN-5382?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15394678#comment-15394678
]
Jason Lowe commented on YARN-5382:
----------------------------------
Thanks for the update, Vrushali!
I should have said this earlier: normally we review patches against trunk
first, since that's where the change must go before it goes anywhere else. We
can only put this into 2.7 once it's also in trunk, branch-2, and branch-2.8,
otherwise we risk releasing a fix/feature that disappears in later releases.
createSuccessLog should build on other versions rather than replicate the code.
For example, now that we have an overload that takes the IP address as a
string, the one that doesn't take an IP address should get the IP address as a
string and call the other version.
Nit: ClientRMService#forceKillApplication is calling Server.getRemoteAddress
three times which is wasteful, we should just cache this in a local.
Would RMAppKillByClientEvent be a more meaningful name than RMAppKillLogEvent?
Should we be passing around an InetAddress in the event and audit log overloads
instead of a String for improved type safety? Not a must-fix, just wondering
if it would be clearer.
I'm not sure we should log empty strings for values that aren't provided. It
would probably be better to log "null" which is what it will already do for
user names, for example, if we just let the null pass through. Not sure if
audit log parsers will properly parse if there isn't some kind of corresponding
token listed for a key.
Existing audit log code will not log a key for an IP address if it can't obtain
it. This code will log an IP key with no value. May be a reason to pass the
InetAddress through and let the audit logger decide whether to add the key if
the value is non-null and it can obtain the address.
In the tests, why do we need a fooUser and a testUGI? I think we only need one
of these. fooUser is created and then only used to get information to create
testUGI, and I don't see why we can't just create testUGI directly. Also that
UGI could be a static final "constant" in the test class rather than having
each test method replicate the code.
> RM does not audit log kill request for active applications
> ----------------------------------------------------------
>
> Key: YARN-5382
> URL: https://issues.apache.org/jira/browse/YARN-5382
> Project: Hadoop YARN
> Issue Type: Bug
> Components: resourcemanager
> Affects Versions: 2.7.2
> Reporter: Jason Lowe
> Assignee: Vrushali C
> Attachments: YARN-5382-branch-2.7.01.patch,
> YARN-5382-branch-2.7.02.patch, YARN-5382-branch-2.7.03.patch,
> YARN-5382-branch-2.7.04.patch, YARN-5382-branch-2.7.05.patch
>
>
> ClientRMService will audit a kill request but only if it either fails to
> issue the kill or if the kill is sent to an already finished application. It
> does not create a log entry when the application is active which is arguably
> the most important case to audit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
