[
https://issues.apache.org/jira/browse/MAPREDUCE-7544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18118720#comment-18118720
]
ASF GitHub Bot commented on MAPREDUCE-7544:
-------------------------------------------
joseluisll opened a new pull request, #8751:
URL: https://github.com/apache/hadoop/pull/8751
### Description of PR
`JobEndNotifier` imports `org.eclipse.jetty.util.log.Log` and makes 14
`Log.getLog().info(...)` / `.warn(...)` calls. It is the only class under any
`src/main` in the whole Hadoop tree that still uses Jetty's logging facade.
This replaces it with a standard SLF4J `Logger` and converts every call site
from string concatenation to `{}` placeholder style, keeping the existing
trailing-exception argument form where an exception was already being logged
(e.g. `LOG.warn("Job end notification to {} failed", urlToNotify, ioe)`).
It also fixes a pre-existing missing space in one of the warnings, which
rendered as `couldn't parse configuredproxy's port`.
No pom change is needed: `hadoop-mapreduce-client/pom.xml`, the module's
parent, already declares `org.slf4j:slf4j-api`, so
`hadoop-mapreduce-client-app` inherits it. After this change the module's main
sources reference no `org.eclipse.jetty` type at all.
Split out of #8699 at @pan3793's request, and deliberately kept to this one
class so that it is straightforward to backport. Eight test classes elsewhere
(in hadoop-common, hadoop-yarn-client,
hadoop-yarn-server-applicationhistoryservice and
hadoop-yarn-server-resourcemanager) still use the same Jetty facade; those are
out of scope here.
### How was this patch tested?
Built and tested locally on Windows with Temurin JDK 21, using `-P
'!native-win'`:
- `mvnw -B -DskipTests -P '!native-win' install -pl
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app
-am` — BUILD SUCCESS.
- `mvnw -B -P '!native-win' test -Dtest=TestJobEndNotifier -pl
.../hadoop-mapreduce-client-app` — 7 tests run, 4 errors. The three tests that
exercise `JobEndNotifier` directly (`checkConfiguration`, `testNotifyRetries`,
`testCustomNotifierClass`) pass. The other four are `MRApp`-based and error
with `YarnRuntimeException: Error creating staging dir`, caused by
`FileNotFoundException: Hadoop bin directory does not exist` — i.e. missing
`winutils.exe`, a consequence of skipping the `native-win` profile. I ran the
same command against the unmodified file on trunk and got exactly the same four
errors with the same cause, so this is a pre-existing local environment
limitation and not a regression. Full test coverage is left to CI on Linux.
- `mvnw -B -P '!native-win' checkstyle:check` on the module: the file
reports only pre-existing violations on untouched code (the `protected` fields,
the leading-comma continuations, the `proxyConf` indentation, the `} else {`).
No violation falls on a changed line, and no changed line exceeds 80 characters.
### For code changes:
- [x] Does the title of this PR start with the corresponding JIRA issue id
(e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: Have the integration tests been executed and the
endpoint declared according to the connector-specific documentation? *Not
applicable.*
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)? *No new
dependencies; slf4j-api is already on the module classpath.*
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`,
`NOTICE-binary` files? *Not applicable.*
### AI Tooling
If an AI tool was used:
- [x] The PR includes the phrase "Contains content generated by Claude"
where <tool> is the name of the AI tool used.
- [x] My use of AI contributions follows the ASF legal policy
https://www.apache.org/legal/generative-tooling.html
Contains content generated by Claude.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
> JobEndNotifier should log through SLF4J
> ---------------------------------------
>
> Key: MAPREDUCE-7544
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-7544
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: mr-am
> Affects Versions: 3.6.0
> Reporter: Jose Luis López
> Priority: Minor
>
> {{JobEndNotifier}} logs through {{org.eclipse.jetty.util.log.Log}}, Jetty's
> own
> logging facade, rather than SLF4J as the rest of the tree does. It is the only
> class under any {{src/main}} in Hadoop that still does so.
> Two consequences:
> * {{Log.getLog()}} returns Jetty's root logger, so all fourteen messages are
> emitted under the logger name {{org.eclipse.jetty.util.log}} rather than
> {{org.apache.hadoop.mapreduce.v2.app.JobEndNotifier}}. Anyone configuring or
> grepping logging by Hadoop package name never sees them, and anyone who
> turns
> Jetty's logging down loses them silently. This may be related to
> MAPREDUCE-6899.
> * {{org.eclipse.jetty.util.log}} was deprecated in Jetty 9.4 and removed in
> Jetty 10, so this class will not compile against Jetty 10 or later. It is
> one
> of the things HADOOP-19912 has to clear out of the way.
> It is also the only reason {{hadoop-mapreduce-client-app}} references Jetty
> from
> its main sources at all; with this changed, the module names no Jetty type
> outside its tests.
> Proposed: switch to {{org.slf4j.Logger}} / {{LoggerFactory}} with a private
> static final LOG, and move the fourteen call sites to {} placeholder style
> rather than string concatenation.
> No pom change is needed: {{hadoop-mapreduce-client/pom.xml}}, the module's
> parent, already declares {{org.slf4j:slf4j-api}}.
> Out of scope: eight test classes still use the same Jetty facade, in
> hadoop-common (4), hadoop-yarn-client (2), hadoop-yarn-server-
> applicationhistoryservice (1) and hadoop-yarn-server-resourcemanager (1).
> Those
> are worth a follow-up but are kept out of here so this change stays small
> enough
> to backport.
> Raised by [~pan3793] in review of HADOOP-19970 (apache/hadoop PR #8699), where
> the change originally sat. Split out on his suggestion so it can be reviewed
> and
> backported on its own.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
