Nothing Special   »   [go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

[WIP] Jetty12 + EE8 #2876

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[WIP] Jetty12 + EE8 #2876

wants to merge 1 commit into from
+1,066 −210

Conversation

iamsanjay
Copy link
Contributor
@iamsanjay iamsanjay commented Nov 19, 2024 •
edited
Loading

https://issues.apache.org/jira/browse/SOLR-17069

Currently, Jetty supports several environments:

  1. EE8 (Jetty10): Implements the Java EE8 specifications, supporting javax.servlet.
  2. EE9 (Jetty11): Implements Jakarta EE9 specifications, introducing the namespace migration from javax.* to jakarta.*.
  3. EE10 (Jetty12): Implements Jakarta EE10 specifications, including the Jakarta Servlet 6.0 specification, fully adopting the jakarta.* namespace.

Jetty 12 providing ongoing support for both older and upcoming EE specifications [from https://github.com/jetty/jetty.project/issues/10485]

In my opinion, Jetty12 + EE8 is the best next step. It retains compatibility with javax.servlet without forcing the removal of modules that haven’t transitioned to jakarta.* yet, while benefiting from all the new features introduced in Jetty12.

#1509

@risdenk
Copy link
Contributor
risdenk commented Nov 19, 2024

This should link to https://issues.apache.org/jira/browse/SOLR-17069 and potentially #1509

risdenk
risdenk reviewed Nov 19, 2024
View reviewed changes
gradle/libs.versions.toml
@@ -295,7 +296,7 @@ bouncycastle-bcprov = { module = "org.bouncycastle:bcprov-jdk18on", version.ref
carrot2-core = { module = "org.carrot2:carrot2-core", version.ref = "carrot2-core" }
carrotsearch-hppc = { module = "com.carrotsearch:hppc", version.ref = "carrotsearch-hppc" }
carrotsearch-randomizedtesting-runner = { module = "com.carrotsearch.randomizedtesting:randomizedtesting-runner", version.ref = "carrotsearch-randomizedtesting" }
# @keep transitive dependency for version alignment
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this changed

gradle/libs.versions.toml
@@ -305,7 +306,7 @@ commonsio-commonsio = { module = "commons-io:commons-io", version.ref = "commons
cybozulabs-langdetect = { module = "com.cybozu.labs:langdetect", version.ref = "cybozulabs-langdetect" }
dropwizard-metrics-core = { module = "io.dropwizard.metrics:metrics-core", version.ref = "dropwizard-metrics" }
dropwizard-metrics-graphite = { module = "io.dropwizard.metrics:metrics-graphite", version.ref = "dropwizard-metrics" }
dropwizard-metrics-jetty10 = { module = "io.dropwizard.metrics:metrics-jetty10", version.ref = "dropwizard-metrics" }
dropwizard-metrics-jetty10 = { module = "io.dropwizard.metrics:metrics-jetty12-ee10", version.ref = "dropwizard-metrics" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it matter that this is ee10 vs ee8 like the description?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the key should change from dropwizard-metrics-jetty10

solr/core/src/java/org/apache/solr/handler/admin/ShowFileRequestHandler.java
KNOWN_MIME_TYPES.add("text/xml");
KNOWN_MIME_TYPES.add("text/javascript");
KNOWN_MIME_TYPES.add("text/csv");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did csv/xhtml get added?

solr/core/src/test/org/apache/solr/cloud/OverseerStatusTest.java
@@ -24,6 +24,7 @@
import org.junit.BeforeClass;
import org.junit.Test;

// @LogLevel("org.eclipse.jetty=DEBUG")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left over from debugging?

solr/licenses/jetty-ee10-servlet-12.0.10.jar.sha1
@@ -0,0 +1 @@
a8d087bde78f231084901553bb6829663b4a22f1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ee10 servlet?

solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java
@@ -350,7 +351,7 @@ private void init(int port) {
connector.setIdleTimeout(THREAD_POOL_MAX_IDLE_TIME_MS);

server.setConnectors(new Connector[] {connector});
server.setSessionIdManager(new DefaultSessionIdManager(server, new Random()));
// server.setSessionIdManager(new DefaultSessionIdManager(server, new Random()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented out?

solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java
}

chain = injectJettyHandlers(chain);

if (config.enableV2) {
RewriteHandler rwh = new RewriteHandler();
rwh.setHandler(chain);
rwh.setRewriteRequestURI(true);
rwh.setRewritePathInfo(false);
// rwh.setRewriteRequestURI(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented out?

@iamsanjay
Copy link
Contributor Author

s3-repository test cases started to failing as HandlerWrapper class not available anymore which was part of Jetty10 and now with Jetty12, it's part of ee8.nested.
However, spring-boot jetty-starter doesn't have anything for ee8. They do have support for ee10 webapps, and adding those runs the test successfully but there are multiple library conflict is there. Working on it.

@risdenk
Copy link
Contributor
risdenk commented Nov 20, 2024

s3mock was a problem when I last looked at Jetty 11/12 due to it needing to stay on 2.x due to Java 17 required in 3.x. However, now that we are on JDK 17 on main, we should be able to upgrade to s3mock 3.x - https://github.com/adobe/S3Mock/releases/tag/3.0.0 (latest being https://github.com/adobe/S3Mock/releases/tag/3.11.0)

I wonder if that will address the Jetty incompatibilities since this upgraded Spring as well.

This upgrade of s3mock should be done separately from any Jetty upgrade if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@risdenk risdenk risdenk left review comments

Assignees
No one assigned
Labels
cat:cli cat:cloud cat:index cat:schema cat:search cat:security client:solrj dependencies Dependency upgrades jetty-server module:hadoop-auth module:jwt-auth test-framework tests tool:build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iamsanjay @risdenk