slfan1989 commented on PR #7019: URL: https://github.com/apache/hadoop/pull/7019#issuecomment-2585703680
I have completed the review of all modules and we can say that the upgrade from Jersey 1.x to Jersey 2.x is essentially complete. We can now begin the PR review. There are only a few minor questions, but I believe they should not be blockers. I will summarize and explain them in detail below: > Background We are upgrading Hadoop to support JDK 17, and one of the biggest challenges is upgrading Jersey from 1.x to 2.x. There are many differences between Jersey 1.x and Jersey 2.x, and they are almost entirely unrelated, especially since Jersey 2.x has undergone a significant internal refactor and now includes its own dependency injection framework, which makes integration with Guice very difficult. I noticed that many community members had previously attempted to integrate Guice into Jersey 2.x, but with limited success. At the beginning of this PR, I also tried this approach, but it didn’t progress smoothly. After careful consideration, I decided to move away from Guice and directly use Jersey 2.x's own dependency injection framework. I initially tested this approach in the NodeManager module and confirmed its feasibility. I then applied this solution to other modules and validated that it works as expected. I have created a simple upgrade diagram, as shown below: <img width="920" alt="image" src="https://github.com/user-attachments/assets/d36673aa-543e-4f1a-82e2-d7aa36eb3afd"; /> > Upgrade process Step1. The way Jetty is injected has been adjusted. We now rely on Jersey 2's `ResourceConfig` for package scanning to complete dependency injection. We can refer to the following code: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java ``` /** * Add a Jersey resource package. * @param packageName The Java package name containing the Jersey resource. * @param pathSpec The path spec for the servlet * @param params properties and features for ResourceConfig */ public void addJerseyResourcePackage(final String packageName, final String pathSpec, Map<String, String> params) { LOG.info("addJerseyResourcePackage: packageName = {}, pathSpec = {}.", packageName, pathSpec); final ResourceConfig config = new ResourceConfig().packages(packageName); final ServletHolder sh = new ServletHolder(new ServletContainer(config)); for (Map.Entry<String, String> entry : params.entrySet()) { sh.setInitParameter(entry.getKey(), entry.getValue()); } webAppContext.addServlet(sh, pathSpec); } /** * Add a Jersey resource config. * @param config The Jersey ResourceConfig to be registered. * @param pathSpec The path spec for the servlet * @param params properties and features for ResourceConfig */ public void addJerseyResourceConfig(final ResourceConfig config, final String pathSpec, Map<String, String> params) { LOG.info("addJerseyResourceConfig: pathSpec = {}.", pathSpec); final ServletHolder sh = new ServletHolder(new ServletContainer(config)); if (params != null) { for (Map.Entry<String, String> entry : params.entrySet()) { sh.setInitParameter(entry.getKey(), entry.getValue()); } } webAppContext.addServlet(sh, pathSpec); } ``` Step2. Modify the way WebServices are injected in each module. We will take ResourceManager as an example. hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebServices.java - @Inject, @Singleton, and @Name are all from the standard javax annotations, not from Guice annotations. ``` @Inject public RMWebServices(final @javax.inject.Named("rm") ResourceManager rm, @javax.inject.Named("conf") Configuration conf) { // don't inject, always take appBaseRoot from RM. super(null); this.rm = rm; this.conf = conf; isCentralizedNodeLabelConfiguration = YarnConfiguration.isCentralizedNodeLabelConfiguration(conf); this.filterAppsByUser = conf.getBoolean( YarnConfiguration.FILTER_ENTITY_LIST_BY_USER, YarnConfiguration.DEFAULT_DISPLAY_APPS_FOR_LOGGED_IN_USER); ..... } } ``` hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebApp.java - When the ResourceManager starts, the instances of RM and Conf are directly injected into RMWebServices via Jersey2. ``` public ResourceConfig resourceConfig() { ResourceConfig config = new ResourceConfig(); config.packages("org.apache.hadoop.yarn.server.resourcemanager.webapp"); config.register(new JerseyBinder()); config.register(RMWebServices.class); config.register(GenericExceptionHandler.class); config.register(new JettisonFeature()).register(JAXBContextResolver.class); return config; } private class JerseyBinder extends AbstractBinder { @Override protected void configure() { bind(rm).to(ResourceManager.class).named("rm"); bind(rm.getConfig()).to(Configuration.class).named("conf"); } } ``` hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ResourceManager.java Line 1484-Line 1486 ``` try { RMWebApp rmWebApp = new RMWebApp(this); builder.withResourceConfig(rmWebApp.resourceConfig()); webApp = builder.start(rmWebApp, uiWebAppContext); } catch (WebAppException e) { webApp = e.getWebApp(); throw e; } ``` Through the steps outlined above, we have injected the required instances into the WebServices and the relevant servlets into Jetty. From my perspective, this approach is straightforward to understand and involves minimal modifications. Step3. Refactoring for unit testing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
