HADOOP-19970. Resolve one Jetty release and one servlet API on every module classpath - #8699
HADOOP-19970. Resolve one Jetty release and one servlet API on every module classpath#8699joseluisll wants to merge 5 commits into
Conversation
9aa1997 to
ce75ddb
Compare
|
💔 -1 overall
This message was automatically generated. |
ShuffleChannelHandler is a Netty handler. It imported org.eclipse.jetty.http. HttpHeader only to name the Connection and Keep-Alive headers, and the same file already imports Netty's HttpHeaderNames.CONTENT_TYPE. Use HttpHeaderNames and HttpHeaderValues throughout. That was the module's only Jetty reference in main sources, so the module no longer uses Jetty at all outside tests. Netty's constants are lowercase AsciiStrings, so the response now carries "connection: keep-alive" where it carried "Connection: Keep-Alive". Field names are case-insensitive per RFC 7230 section 3.2, connection options are case-insensitive per section 6.1, and the response already carried Netty's lowercase "content-length" from HttpUtil.setContentLength, so this makes the response more internally consistent rather than less. No shuffle client reads these headers by name; the fetcher uses HttpURLConnection, which is case-insensitive. Note for HADOOP-19970 (apache#8699): that PR adds a jetty-http dependency to this module's pom for exactly this import. With the import gone the declaration is no longer needed and should be dropped from that PR. Contains content generated by Claude. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ShuffleChannelHandler is a Netty handler. It imported org.eclipse.jetty.http. HttpHeader only to name the Connection and Keep-Alive headers, and the same file already imports Netty's HttpHeaderNames.CONTENT_TYPE. Use HttpHeaderNames and HttpHeaderValues throughout. That was the module's only Jetty reference in main sources, so the module no longer uses Jetty at all outside tests. Netty's constants are lowercase AsciiStrings, so the response now carries "connection: keep-alive" where it carried "Connection: Keep-Alive". Field names are case-insensitive per RFC 7230 section 3.2, connection options are case-insensitive per section 6.1, and the response already carried Netty's lowercase "content-length" from HttpUtil.setContentLength, so this makes the response more internally consistent rather than less. No shuffle client reads these headers by name; the fetcher uses HttpURLConnection, which is case-insensitive. Note for HADOOP-19970 (apache#8699): that PR adds a jetty-http dependency to this module's pom for exactly this import. With the import gone the declaration is no longer needed and should be dropped from that PR. Contains content generated by Claude. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This is the beginning of chaos. And in practice, during the migration, it's likely that we need both |
ShuffleChannelHandler is a Netty handler. It imported org.eclipse.jetty.http. HttpHeader only to name the Connection and Keep-Alive headers. That was the module's only Jetty reference in main sources, so with it gone the module does not use Jetty at all, tests included. The names are spelled out as constants rather than taken from Netty's HttpHeaderNames and HttpHeaderValues, whose constants are lowercase AsciiStrings: using those would have sent "connection: keep-alive" where this handler has always sent "Connection: Keep-Alive". Field names and connection options are both case-insensitive per RFC 7230, and Hadoop's own fetcher uses HttpURLConnection, but the shuffle response is read by other projects too and there is no reason to change a byte of it here. Note for HADOOP-19970 (apache#8699): that PR adds a jetty-http dependency to this module's pom for exactly this import. With the import gone the declaration is no longer needed and should be dropped from that PR. Contains content generated by Claude. Co-Authored-By: Claude <noreply@anthropic.com>
ShuffleChannelHandler is a Netty handler. It imported org.eclipse.jetty.http. HttpHeader only to name the Connection and Keep-Alive headers. That was the module's only Jetty reference in main sources, so with it gone the module does not use Jetty at all, tests included. The names are spelled out as constants rather than taken from Netty's HttpHeaderNames and HttpHeaderValues, whose constants are lowercase AsciiStrings: using those would have sent "connection: keep-alive" where this handler has always sent "Connection: Keep-Alive". Field names and connection options are both case-insensitive per RFC 7230, and Hadoop's own fetcher uses HttpURLConnection, but the shuffle response is read by other projects too and there is no reason to change a byte of it here. Note for HADOOP-19970 (apache#8699): that PR adds a jetty-http dependency to this module's pom for exactly this import. With the import gone the declaration is no longer needed and should be dropped from that PR. Contains content generated by Claude. Co-Authored-By: Claude <noreply@anthropic.com>
cff271a to
0427e2a
Compare
The jakarta.servlet groupId does not imply the jakarta.servlet package. Version 4.0.x was just the artifact rename under the Eclipse Foundation — the classes are still javax.servlet.http.HttpServlet. The actual package rename ("big bang") landed in 5.0.0. So if you see jakarta.servlet-api:4.0.4 on a classpath, your imports are still javax.*. |
|
With the three substasks HADOOP-19970, HADOOP-19971 and HADOOP-19972, we have migrated to jetty12-ee8 as an intermediate step, not breaking downstream consumers. I'm still testing the pr's results, it would be convenient to create a specific subtask for compatibility testing before releasing the intermediate step. Any idea on how to approach the compatibility testing is welcome. The breaking changes should be delivered in a future and additional release, giving time anf instructions for consumers to adapt to the namespace change, that is, using the jakarta instead of the javax classes. This changes also introduce jdk-17 as a requirement, they are no longer jdk-8 compatible due to jetty12. |
@pjfanning that's true. so in the process of namespace migration, we might mix in
but this might not be a real issue for hadoop project, since it stays near the root of the big data ecosystem dependency tree |
| else { | ||
| success = true; | ||
| Log.getLog().info("Job end notification to " + urlToNotify | ||
| LOG.info("Job end notification to " + urlToNotify |
There was a problem hiding this comment.
please also change it to {} placeholder style, and I think org.eclipse.jetty.util.log.Log => org.slf4j.Logger worth a dedicated PR and we can do a backport
|
💔 -1 overall
This message was automatically generated. |
077558a to
236e4bd
Compare
… module classpath
Several modules resolved more than one Jetty release, and more than one servlet
API, on a single classpath. Both combinations compile and then fail at run time,
on whichever code path reaches the wrong jar.
* Four modules used org.eclipse.jetty from their main sources while no pom in
their inheritance chain declared Jetty: hadoop-mapreduce-client-app,
hadoop-mapreduce-client-shuffle, hadoop-yarn-server-router and
hadoop-yarn-server-applicationhistoryservice. Each now declares the artifact
its main sources use, without a version, and jetty-http gains the
dependencyManagement entry it was missing. The Router declares jetty-webapp at
compile scope, since a test scope would override the compile-scoped copy it
inherits and take it off the Router's own runtime classpath.
* jersey-test-framework-provider-jetty carried jetty-continuation onto the test
classpath of around twenty modules at its own, older release. It is managed at
${jetty.version} rather than excluded, because Jersey needs the class:
Continuation is a final field on JettyHttpContainer's ResponseWriter,
initialised in the constructor.
* solr-core carried nine Jetty artifacts, and a servlet API, into
hadoop-yarn-applications-catalog-webapp's tests at a much older release. Those
tests reach Solr only through EmbeddedSolrServer, which runs no servlet
container, so none of it is needed. The exclusions are applied to
solr-test-framework as well, whose own path to solr-core would otherwise
reintroduce the same jars.
* hadoop-project excluded javax.servlet-api from jetty-server under the group
org.eclipse.jetty, which jetty-server has never used, so the exclusion matched
nothing and javax.servlet:javax.servlet-api stayed on the classpath beside the
managed jakarta.servlet:jakarta.servlet-api. Both publish the javax.servlet
packages, so which one a module compiled and ran against was decided by the
order of the jars, and 73 modules carried both. An exclusion naming the real
coordinates is added beside the existing one, and the same on
javax-websocket-server-impl, which reaches the API again through
websocket-servlet. hadoop-common now declares jakarta.servlet-api itself.
* hadoop-common declared jakarta.servlet.jsp-api, which reached some
eighty-five classpaths. Nothing in the tree uses JSP, so it and its managed
version are removed, together with the LICENSE-binary and NOTICE-binary
entries for the two artifacts that no longer ship.
One module keeps two servlet APIs, hadoop-yarn-server-timelineservice-hbase-tests,
where the second arrives with HBase's own test stack.
Co-Authored-By: Claude <noreply@anthropic.com>
jersey-test-framework-provider-jetty resolves to jersey-container-jetty-http,
which Jersey 2.46 builds against Jetty 9.4.55 and which needs jetty-continuation.
That artifact does not exist past Jetty 9, so managing it at ${jetty.version}
stops resolving once that is 12.0.x. Jersey 2.x has no Jetty 12 container: the
one in Jersey 3.1.x is jakarta.servlet and cannot be used here.
Switch to jersey-test-framework-provider-jdk-http. It is the same Jersey release
and speaks real HTTP, but it runs on the JDK's own com.sun.net.httpserver, so it
adds a single artifact and no transitive dependencies at all. It puts no Jetty
and no servlet API on any test classpath, so the jetty-continuation entry and
its shade exclusion go with it.
hadoop-client-minicluster relocates com/ and already keeps the JDK's own com/sun
packages out of that. Add com/sun/net/httpserver to the list, or the container's
references to it would be rewritten into the shaded namespace and fail to
resolve at run time.
LICENSE-binary gains jersey-container-jdk-http, which reaches
hadoop-client-minicluster at compile scope.
Co-Authored-By: Claude <noreply@anthropic.com>
JobEndNotifier logged through org.eclipse.jetty.util.log.Log, Jetty's own logging facade, which is the only reason hadoop-mapreduce-client-app needed a Jetty dependency at all. Use SLF4J, as the rest of the tree does, and the declaration added earlier in this PR goes with it. The module now names no Jetty type anywhere, in main sources or tests. Raised by pan3793 in review. Co-Authored-By: Claude <noreply@anthropic.com>
The tree took the Servlet 4.0 API from jakarta.servlet:jakarta.servlet-api. jetty-ee8 depends on org.eclipse.jetty.toolchain:jetty-servlet-api instead, so staying on the jakarta coordinate means moving off it again when the ee8 environment lands. Use jetty-servlet-api now. Both publish the same javax.servlet classes, so no source changes, and 4.0.9 is the version Jetty 12 pins for ee8. The coordinate is set once rather than twice. Exclusions naming the servlet API follow the coordinate where a Hadoop module brings it. Where a Jersey artifact declares it, they stay on jakarta.servlet, which is what those artifacts publish. jersey-test-framework-core is one of those: it is now excluded in dependencyManagement, or its copy of the API would sit on the test classpath beside jetty-servlet-api. Co-Authored-By: Claude <noreply@anthropic.com>
0bc0ff8 to
2c62353
Compare
hadoop-client-minicluster excludes org.eclipse.jetty:jetty-util from
hadoop-minicluster, hadoop-yarn-server-nodemanager and
hadoop-yarn-server-web-proxy, so the artifact reached the shaded jar only
through jersey-test-framework-provider-jetty, whose
jersey-container-jetty-http declares jetty-server, jetty-util and
jetty-continuation. Moving the test container to the JDK's HTTP server took
that path away, and every copy of jetty-util went with it.
jetty-server still arrives through hadoop-minicluster, and the shade plugin
rewrites the Jetty references in it, and in Hadoop's own classes, to the
relocated namespace. Starting a NameNode from the shaded minicluster then
failed:
java.lang.NoClassDefFoundError:
org/apache/hadoop/shaded/org/eclipse/jetty/util/Attributes
at org.apache.hadoop.hdfs.server.namenode.NameNode.startHttpServer
hadoop-client-runtime excludes jetty-util and jetty-server from its own jar
so that the two shaded artifacts do not ship the same Jetty classes, which
leaves the minicluster as the module that carries them. Declare jetty-util
there rather than relying on a test framework to drag it in.
Co-Authored-By: Claude <noreply@anthropic.com>
Description of PR
Every module in the reactor now resolves one Jetty release and one servlet API.
The one module still holding two servlet APIs is
hadoop-yarn-server-timelineservice-hbase-tests, where the second arrives with HBase's own test stack.What changed:
jersey-test-framework-provider-jettyto-grizzly2, taking Jetty off the test classpath of around twenty modules.jetty-continuationleaves the tree.solr-coreandsolr-test-frameworkno longer carry nine Jetty artifacts and a servlet API intohadoop-yarn-applications-catalog-webapp's tests.javax.servlet-apiexclusion onjetty-servernamed a groupjetty-serverhas never used, so it matched nothing. It now names the real coordinates, andhadoop-commondeclaresjakarta.servlet-apiitself.hadoop-mapreduce-client-app,hadoop-mapreduce-client-shuffle,hadoop-yarn-server-routerandhadoop-yarn-server-applicationhistoryservicedeclare the Jetty artifact their main sources use.jakarta.servlet.jsp-api, which reached some eighty-five classpaths, is gone — nothing in the tree uses JSP.LICENSE-binaryandNOTICE-binaryfollow the artifacts that changed.No source changes; poms and the licence files only.
How was this patch tested?
Against resolved dependency trees rather than pom scans.
mvn dependency:treeon an affected module shows one servlet API (jakarta.servlet:jakarta.servlet-api:4.0.4), one Jetty release (9.4.58), nojetty-continuation, and Grizzly at test scope only.TestResourceEstimatorService, which extendsJerseyTest, passes on the Grizzly container; thehadoop-resourceestimatorsuite is 47 tests, 0 failures.For code changes:
declared according to the connector-specific documentation? Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required.
LICENSE,LICENSE-binary,NOTICE-binaryfiles?Grizzly 2.4.4 is EPL 2.0 with GPL v2 and the Classpath Exception as a secondary licence, listed under its own heading in
LICENSE-binaryalong with the twojersey-container-grizzly2-*artifacts.AI Tooling
Contains content generated by Claude.
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html