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 4 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>
| <groupId>com.fasterxml.jackson.core</groupId> | ||
| <artifactId>jackson-databind</artifactId> | ||
| </dependency> | ||
| <!-- JobEndNotifier uses org.eclipse.jetty.util.log.Log --> |
There was a problem hiding this comment.
by design, or just an incorrect reference? in general, we should always use SFL4J
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>
… 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 put jetty-continuation on the test classpath of around twenty modules at its own, older Jetty release. Managing that artifact pins the version but leaves Jersey's Jetty 9 container there. Switch to jersey-test-framework-provider-grizzly2 instead. It is the same Jersey release, it speaks real HTTP, and it puts no Jetty on a test classpath at all, so the jetty-continuation entry and its shade exclusion go with it. It brings no second servlet API: grizzly-http-servlet declares javax.servlet-api at provided scope, which is not transitive, and jersey-container-grizzly2-servlet declares the jakarta.servlet-api coordinate this tree already manages. LICENSE-binary gains the four Grizzly artifacts and the two jersey-container-grizzly2 ones, which reach hadoop-client-minicluster at compile scope. 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 |
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>
| <dependency> | ||
| <groupId>org.glassfish.jersey.test-framework.providers</groupId> | ||
| <artifactId>jersey-test-framework-provider-jetty</artifactId> | ||
| <artifactId>jersey-test-framework-provider-grizzly2</artifactId> |
There was a problem hiding this comment.
I'm not familiar with this part, why should we move from ...provider-jetty to ...provider-grizzly2? will the latter pull in other dependencies? I checked that spark uses ...provider-simple which sounds lighter, is it sufficient for hadoop use case?
| 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
Grizzly's servlet container declares jakarta.servlet-api at compile scope, where the Jetty test container it replaced took the servlet API from Jetty. hadoop-client-minicluster shades the javax.servlet package and so does hadoop-client-runtime, which already ships those classes, so the new edge put them in both jars and BanDuplicateClasses in hadoop-client-check-test-invariants rejected the build. Exclude it, as the module already does for the servlet API arriving through the Hadoop modules it pulls back in. Co-Authored-By: Claude <noreply@anthropic.com>
|
💔 -1 overall
This message was automatically generated. |
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