[ 
https://issues.apache.org/jira/browse/HADOOP-19970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18137912#comment-18137912
 ] 

ASF GitHub Bot commented on HADOOP-19970:
-----------------------------------------

joseluisll commented on PR #8699:
URL: https://github.com/apache/hadoop/pull/8699#issuecomment-6627599916

   @pan3793 thanks for your review. I agree that `jetty-servlet-api` is the 
wrong choice for downstream projects. Between your two options I'd go with 
`javax.servlet:javax.servlet-api` (4.0.1). The risk I see with the 
`jakarta.servlet` coordinate is that if a downstream project pulls servlet API 
5.x or 6.x, Maven resolves a single `jakarta.servlet-api` version, Hadoop's 
`javax.servlet` classes disappear, and the failure only shows at runtime.
   
   I checked the root POMs of the main downstream projects to see whether that 
risk is real:
   
   | Project (branch) | Hadoop | Jetty | Servlet API in the root POM |
   |---|---|---|---|
   | Spark (master) | 3.5.0 | 12.1.13 | `jakarta.servlet-api` **6.0.0** and 
`javax.servlet-api` 4.0.1, both in `dependencyManagement` |
   | Spark (branch-4.0) | 3.4.1 | 11.0.24 | `jakarta.servlet-api` **5.0.0** and 
`javax.servlet-api` 4.0.1, both managed |
   | Spark (branch-3.5) | 3.3.4 | 9.4.58 | `jakarta.servlet-api` 4.0.3 |
   | Kyuubi (master) | 3.4.3 | 9.4.57 | `jakarta.servlet-api` **5.0.0** 
(**6.0.0** in two profiles) and `javax.servlet-api` 4.0.1, both managed |
   | Hive (master, branch-4.0) | — | 9.4.x | `javax.servlet-api` 3.1.0 |
   | HBase (master) | 3.4.3 | — | `javax.servlet-api` 4.0.1 |
   | HBase (branch-2.6) | 3.4.3 | — | `javax.servlet-api` 3.1.0 |
   | Phoenix (master) | 3.3.6 | 9.4.58 | `javax.servlet-api` 3.1.0 |
   | Ranger (master) | 3.4.2 | 9.4.56 | `javax.servlet-api` 4.0.1 |
   | Livy (master) | 3.3.4 | 9.4.58 | `javax.servlet-api` 3.1.0 |
   | Tez (master) | 3.5.0 | — | `jakarta.servlet-api` 4.0.4 |
   | Ozone (master) | 3.4.3 | 12.1.14 | `jetty-servlet-api` 4.0.9; excludes 
`javax.servlet-api` from the Hadoop artifacts |
   | Oozie (master) | 2.8.5 | 9.4.44 | `javax.servlet:servlet-api` 2.5 |
   
   Hudi, Drill, Flink and Zeppelin don't declare a servlet API in their root 
POM.
   
   What this shows:
   
   1. **`javax.servlet:javax.servlet-api` is what most of the ecosystem 
declares**: Hive, HBase, Phoenix, Ranger and Livy, plus Spark and Kyuubi, which 
manage it next to `jakarta.servlet-api`. If Hadoop ships it, Maven mediates 
with all of them, and every one of them gets the `javax.servlet` classes Hadoop 
needs, whichever version wins.
   
   2. **The projects already on the `jakarta.servlet` namespace pin 
`jakarta.servlet-api` 5.x or 6.x in `dependencyManagement`** (Spark 4.0 and 
master, Kyuubi). A managed version overrides every transitive one. If Hadoop's 
only source of `javax.servlet` were `jakarta.servlet-api` 4.0.4, then in a 
module of those projects that uses the non-shaded Hadoop artifacts, Maven would 
silently replace it with 5.0.0 or 6.0.0, with no `javax.servlet.*` classes 
left. Today they're saved because Hadoop also brings `javax.servlet-api` (3.1.0 
through `jetty-server`), and Spark and Kyuubi manage it to 4.0.1. That is the 
mixed-namespace classpath from your first option, and it already happens in 
practice. Your earlier point, that the `javax` classes must come from 
`javax.servlet:javax.servlet-api`, is exactly what keeps those builds working. 
The shaded clients aren't affected, but not everyone uses them.
   
   3. **`jakarta.servlet-api` 4.x in the `javax` namespace is the minority**: 
Spark 3.5 (4.0.3) and Tez (4.0.4, which follows Hadoop's current managed 
version). For them, a switch to `javax.servlet-api` gives two copies of the 
same classes until they exclude one, the same cost the release note already 
describes. That's one maintained branch that is moving off this coordinate, 
plus Tez, which tracks Hadoop.
   
   4. **Ozone is the only project here that uses `jetty-servlet-api`.** It 
already runs on Jetty 12 ee8 and excludes `javax.servlet-api` from the Hadoop 
artifacts, so it is the one place where the current PR fits as it is. With 
`javax.servlet-api` 4.0.1, Ozone keeps its existing exclusion and adds 
`jetty-servlet-api` to it, or drops its own.
   
   So I'll switch the PR to `javax.servlet:javax.servlet-api` 4.0.1:
   
   - Manage `javax.servlet-api` 4.0.1 in `hadoop-project`. It replaces the 
3.1.0 that `jetty-server` brings, and the YARN, HDFS and KMS modules that 
declare `jakarta.servlet-api` 4.0.4 today move to it, so each classpath has one 
servlet API jar.
   - Drop `jakarta.servlet-api` 4.x from Hadoop entirely. When HADOOP-19395 
moves Hadoop to the `jakarta.servlet` namespace, Hadoop takes 
`jakarta.servlet-api` 6.x, so the coordinate changes exactly when the namespace 
does, and the two can coexist on a classpath during the migration.
   - In HADOOP-19972, exclude `jetty-servlet-api` from `jetty-ee8-nested`, the 
only Jetty 12 ee8 artifact that depends on it.
   - Keep `TestJettyClasspath`, checking for `javax.servlet-api`, and update 
the release note: direct users of `hadoop-common` move from 3.1.0 to 4.0.1 
(same coordinate, mediated by Maven), and projects that excluded or declared 
`jakarta.servlet-api` 4.x need the `javax` coordinate instead.




> HADOOP - Move the servlet API to jetty-servlet-api 4.0.9
> --------------------------------------------------------
>
>                 Key: HADOOP-19970
>                 URL: https://issues.apache.org/jira/browse/HADOOP-19970
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: build, hadoop-common
>            Reporter: Jose Luis López
>            Assignee: Jose Luis López
>            Priority: Minor
>              Labels: pull-request-available
>
> *Scope* (rescoped: the other changes from PR #8699 moved to HADOOP-20012 to 
> HADOOP-20016 and MAPREDUCE-7544):
> * hadoop-project/pom.xml: add the jetty-servlet-api.version property (4.0.9), 
> and replace the managed jakarta.servlet:jakarta.servlet-api 4.0.4 with 
> org.eclipse.jetty.toolchain:jetty-servlet-api, with a comment on why and on 
> what downstream projects need to do.
> * hadoop-common/pom.xml: declare 
> org.eclipse.jetty.toolchain:jetty-servlet-api at compile scope. HttpServer2 
> and its filters compile against javax.servlet, which today arrives only as 
> javax.servlet-api 3.1.0 through jetty-server.
> * hadoop-project/pom.xml and hadoop-tools/hadoop-sls/pom.xml: the exclusion 
> on jetty-server names org.eclipse.jetty:javax.servlet-api, a coordinate that 
> matches nothing. Replace it with javax.servlet:javax.servlet-api.
> * hadoop-project/pom.xml: on javax-websocket-server-impl, also exclude 
> javax.servlet:javax.servlet-api (through websocket-servlet).
> * Switch the coordinate in the 17 module poms that declare the servlet API, 
> and in the 3 exclusions of it on Hadoop artifacts in 
> hadoop-client-minicluster.
> * jersey-test-framework-core declares jakarta.servlet-api at compile scope. 
> Exclude it on the managed entry only: managed exclusions also cover the 
> Jersey test provider and hadoop-client-minicluster's own declaration of core.
> * LICENSE-binary: replace jakarta.servlet:jakarta.servlet-api:4.0.4 with 
> org.eclipse.jetty.toolchain:jetty-servlet-api:4.0.9. LICENSE-binary and 
> NOTICE-binary: drop javax.servlet:javax.servlet-api 3.1.0.
> * Tests: AbstractJettyClasspathTest in the hadoop-common test jar, with an 
> empty TestJettyClasspath subclass in hadoop-common, 
> hadoop-mapreduce-client-shuffle, hadoop-yarn-common, the 
> ApplicationHistoryServer, the NodeManager and the Router. They check that one 
> jar, jetty-servlet-api, provides javax.servlet, and that all Jetty jars come 
> from one release. Jars are recognised by their contents and manifests, not by 
> repository path.
> *Why*: javax.servlet-api 3.1.0 and jakarta.servlet-api 4.0.4 both publish the 
> javax.servlet packages, and which one a module compiles and runs against 
> depends on jar order. jetty-servlet-api 4.0.9 is the artifact and version 
> Jetty 12's ee8 environment manages. Switching now leaves one servlet API jar 
> and takes the coordinate change out of the Jetty 12 upgrade.
> *Impact on Jetty 9*: none. 4.0 is a superset of 3.1, and Jetty 9.4 runs on 
> Servlet 4.0. jetty-servlet-api and jakarta.servlet-api 4.0.4 ship the same 
> javax.servlet 4.0 classes, and jetty-servlet-api also carries the web.xml 
> schemas. The project's web.xml files declare versions 2.4, 2.5 and 3.0, which 
> Servlet 4.0 accepts.
> *Downstream*:
> * hadoop-client-runtime: still relocates one servlet API under 
> org.apache.hadoop.shaded.javax.servlet, now taken from jetty-servlet-api 
> 4.0.9 instead of jakarta.servlet-api 4.0.4. No public API change.
> * hadoop-client-api: carries no servlet classes. hadoop-client-minicluster: 
> unchanged, it already excludes the servlet API.
> * Non-shaded artifacts: the coordinate changes from 
> jakarta.servlet:jakarta.servlet-api to 
> org.eclipse.jetty.toolchain:jetty-servlet-api, so downstream exclusions of 
> jakarta.servlet-api need the new coordinate. Direct consumers of 
> hadoop-common get jetty-servlet-api 4.0.9 instead of javax.servlet-api 3.1.0.
> * A project that also declares javax.servlet-api or jakarta.servlet-api 4.x 
> gets two jars with the same javax.servlet classes (Maven cannot mediate 
> between different coordinates) and should exclude one of them. See the 
> release note.
> *Depends on*: HADOOP-20014. Until it is merged, 
> jersey-test-framework-provider-jetty still brings jetty-continuation 9.4.55 
> onto the test classpath of hadoop-mapreduce-client-shuffle, 
> hadoop-yarn-common, the ApplicationHistoryServer and the NodeManager, so 
> their TestJettyClasspath.testSingleJettyRelease fails. The patch itself 
> applies on trunk; with HADOOP-20013, only an adjacent line in NOTICE-binary 
> overlaps.
> *CI*: touches hadoop-project, so a full build.
> *PR*: https://github.com/apache/hadoop/pull/8699



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to