Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Jetty 12 #30698

Closed
wilkinsona opened this issue Jun 20, 2023 · 9 comments
Closed

Upgrade to Jetty 12 #30698

wilkinsona opened this issue Jun 20, 2023 · 9 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: dependency-upgrade A dependency upgrade
Milestone

Comments

@wilkinsona
Copy link
Member

Affects: 6.1.0-M1

this.byteBufferPool = new MappedByteBufferPool(2048,
this.executor instanceof ThreadPool.SizedThreadPool sizedThreadPool ?
sizedThreadPool.getMaxThreads() / 2 :
ProcessorUtils.availableProcessors() * 2);

This code fails with Jetty 12.0.0.beta2 as MappedByteBufferPool does not exist:

java.lang.NoClassDefFoundError: org/eclipse/jetty/io/MappedByteBufferPool
	at org.springframework.http.client.reactive.JettyResourceFactory.afterPropertiesSet(JettyResourceFactory.java:133)
	at org.springframework.boot.web.embedded.jetty.JettyReactiveWebServerFactoryTests.useServerResources(JettyReactiveWebServerFactoryTests.java:118)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:95)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:91)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:60)
	at org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:98)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:40)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:529)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:756)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:452)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
Caused by: java.lang.ClassNotFoundException: org.eclipse.jetty.io.MappedByteBufferPool
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	... 72 more

It looks like it was removed in beta0 as part of jetty/jetty.project@ded18f5.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 20, 2023
@jhoeller
Copy link
Contributor

While our server parts seem to work with Jetty 12.0.0.beta2 as-is, I'm afraid we haven't tried the reactive web client at all yet.

Investigating it for 6.0.11. I guess we should narrow the title of this issue to the client part if no wider incompatibilities come up.

@jhoeller
Copy link
Contributor

From a quick glance, it seems the client incompatibility is much wider than MappedByteBufferPool. Jetty 12 seems to have relocated the entire client request/response API, with their own latest jetty-reactive-httpclient not being compatible with it.

So not a 6.0.x candidate, and even hard to address in 6.1 once a corresponding jetty-reactive-httpclient becomes available since we'll still have the Jetty 11 vs 12 compatibility problem.

I wonder whether we should do the same as with JettyWebSocketClient and deprecate our Jetty-based client options in their entirety, never upgrading them to Jetty 12. Or wholesale upgrade them to Jetty 12, leaving Jetty 11 compatibility behind.

@jhoeller
Copy link
Contributor

jhoeller commented Jun 20, 2023

It looks like we'll have to upgrade our reactive Jetty client as well as the new non-reactive Jetty client in 6.1 to Jetty 12, assuming that Jetty 12 goes GA in time for November, and assuming that the jetty-reactive-httpclient project will release a Jetty 12 compatible version.

For the time being in 6.0.x as well as 6.1 milestones, we only support Jetty 12 next to Jetty 11 for the server runtime but not for any of our client options which remain bound to Jetty 11 only. For 6.1 GA, we may upgrade the client options to Jetty 12 only but still retain a choice of Jetty 11 and 12 for server deployment, or ideally upgrade to Jetty 12 only for WebFlux server as well.

@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: dependency-upgrade A dependency upgrade and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 20, 2023
@jhoeller jhoeller added this to the 6.1.x milestone Jun 20, 2023
@jhoeller jhoeller changed the title Compatibility with Jetty 12.0.0.beta2 Upgrade Jetty HTTP client options to Jetty 12 Jun 20, 2023
@jhoeller jhoeller changed the title Upgrade Jetty HTTP client options to Jetty 12 Upgrade Jetty HTTP client adapters to Jetty 12 Jun 20, 2023
@wilkinsona
Copy link
Member Author

For Boot, this isn't just a client-side problem. We use JettyResourceFactory on the server side when creating the ServerConnector:

ServerConnector connector;
if (resourceFactory != null) {
	connector = new ServerConnector(server, resourceFactory.getExecutor(), resourceFactory.getScheduler(),
			resourceFactory.getByteBufferPool(), this.acceptors, this.selectors,
			connectionFactories.toArray(new ConnectionFactory[0]));
}
else {
	connector = new ServerConnector(server, this.acceptors, this.selectors,
			connectionFactories.toArray(new ConnectionFactory[0]));
}
connector.setHost(address.getHostString());
connector.setPort(address.getPort());

@jhoeller
Copy link
Contributor

JettyResourceFactory lives in http.client.reactive, so I suppose Boot is trying to reuse Jetty resource between client and server there. We can try to partially address this in 6.1 M2 but it looks like we can only do a full Jetty 12 client upgrade later once jetty-reactive-httpclient is available for it.

@jhoeller
Copy link
Contributor

jhoeller commented Jun 20, 2023

So I wonder whether the existing JettyResourceFactory is worth patching on its own for 6.1 M2 if its corresponding client does not work with Jetty 12 yet? @wilkinsona I suppose you could just disable that resourceFactory test code path for Jetty 12 based server tests for the time being...

@jhoeller
Copy link
Contributor

It looks like jetty-reactive-httpclient will get a Jetty 12 based release soon after Jetty 12 GA in late July: jetty-project/jetty-reactive-httpclient#244 (comment)

@wilkinsona we meant to skip August but if it is useful for Boot, we can arrange for a 6.1 M3 in mid August to pick up Jetty 12. Otherwise it would happen for our 6.1 RC1 in September.

@jhoeller jhoeller modified the milestones: 6.1.x, 6.1.0-M3 Jun 21, 2023
@wilkinsona
Copy link
Member Author

I have Boot's server-side Jetty support largely working with Jetty 12. There's one (I hope) remaining problem with loading servlet context resources from META-INF/resources of nested jars. In my branch I have removed the auto-configuration for Jetty-based clients for the time being. Assuming that fixing the static resource problem doesn't prove too tricky, I plan to merge the Jetty 12 upgrade in time for Spring Boot 3.2.0-M1 next month. We'll then reinstate the Jetty client support in M2 (August) or M3 (September). Either of those dates is fine from my perspective so no need for a Framework M3 in August just on Boot's account. Thank you for the offer though.

@wilkinsona
Copy link
Member Author

On the Boot side, my branch now has everything, other than client support, working with Jetty 12.

I plan to merge the Jetty 12 upgrade in time for Spring Boot 3.2.0-M1 next month. We'll then reinstate the Jetty client support in M2 (August) or M3 (September)

Given the recent addition of support for JettyClientHttpRequestFactory in Boot, I've reconsidered this plan. I now think we should merge the Jetty 12 upgrade in time for 3.2.0-M2 in August alongside the changes planned here for Framework M3. That will give us a complete Jetty 12 story in a single milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: dependency-upgrade A dependency upgrade
Projects
None yet
Development

No branches or pull requests

4 participants