SOLR-15955: Update Jetty dependency to 10 #585

markrmiller · 2022-02-01T20:21:57Z

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

Summary:

Upgrades to Jetty 10.0.12
dropwizard metrics 4.2.12 for dropwizard-metrics9 -> dropwizard-metrics10
log4j 2.19.0 and slf4j 2.0.3
for s3mock specifically upgrade spring-boot 2.5.14 and spring 5.3.23 to handle Jetty 10

solr/modules/s3-repository/build.gradle

solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java

solr/modules/hdfs/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java

solr/modules/s3-repository/build.gradle

.github/workflows/manual.yml

gradle/ant-compat/force-versions.gradle

solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java

solr/modules/s3-repository/src/test/org/apache/solr/s3/S3ReadWriteTest.java

solr/core/src/java/org/apache/solr/request/json/RequestUtil.java

solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java

solr/solrj/src/java/org/apache/solr/client/solrj/util/SolrBasicAuthentication.java

...t-framework/src/java/org/apache/solr/cloud/api/collections/AbstractBackupRepositoryTest.java

markrmiller · 2022-02-19T07:00:24Z

Thanks for taking a look. I have a n update I'll push I'll push tomorrow.

gradle/testing/randomization/policies/solr-tests.policy

solr/modules/hdfs/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java

markrmiller · 2022-02-21T06:34:32Z

A couple of those are extra lines that came in when I merged to a clean repo (test policy and s3 build.gradle I believe).

I'll get the follow up pushed tomorrow - I rebased yesterday and after resolving new conflicts, tests were flakier (a handful failed first run) and one test failed consistently (managed storage test I believe). So I held back to look into that a bit, but in the in end, it seems I see the same behavior on a clean checkout, so should not be related this work.

markrmiller · 2022-03-04T01:09:38Z

That was a bit more effort than expected, but updated jetty from 10.0.7 to 10.0.8. That removes the need for the security policy read permission for "/".

Rebased to latest main, let's see if these build checks pass, did a couple test runs without issue.

solr/modules/hadoop-auth/src/test/org/apache/solr/client/solrj/impl/Krb5HttpClientUtils.java

solr/modules/s3-repository/build.gradle

versions.lock

gradle/solr/force-versions.gradle

solr/core/build.gradle

madrob · 2022-03-08T20:20:39Z

solr/core/src/java/org/apache/solr/request/json/RequestUtil.java

+        // if BinaryResponseParser.BINARY_CONTENT_TYPE, let the following fail below - we may have adjusted the content without updating the content type
+        // problem in this case happens in a few tests, one seems to happen with kerberos and remote node query (HttpSolrCall's request proxy)


It took a bit of digging, but I finally figured it out. When Jetty receives a request with no body content, it creates a placeholder zero-length application/octet-stream content object. Then when it gets a redirect (like an explicit redirect handler, or some authentication plugins cause), it copies that body to the new request. When the new request gets to our search handlers, we see the body and save it as a stream. So I think that we could check for that in SolrRequestParsers.parseParamsAndFillStreams - the combination of a GET request with a zero length octet-stream body seems like a safe instance to drop the stream. Maybe we could drop all the stream bodies for all get requests, I'm not sure. This could be a bug in Jetty, but I haven't figured out a simple repro case for them yet.

Good find, and that does jive with what was happening when I found the fail I saw here pop up on the mailing list in the past.

So I think the diagnosis makes sense, and the change sounds reasonable high level, but I am concerned that it requires that you can inspect the stream or count on the content-size header to properly describe the stream. You cannot generally do that though. The size is not required, usually not given in Solr, and you can't reasonable inspect a stream in a webapp/servlet unless you buffer it, which is also problematic. Given we are talking about a steram on a GET request, maybe somehow you can get away with this, but it seems pretty fragile?

RawRequestParser.parseParamsAndFillStreams will proactively save the stream if there is a content length >= 0 - that should probably be strictly > 0. Because then in the else block, if it's not a GET we try to buffer the first byte - if it exists, save the stream again and if not then we don't need it. I think we need to fix RawRequestParser with this PR to accommodate the change that Jetty made - it's likely a regression but I couldn't find an existing bug report or an easy reproducer for them.

Hmmm, it shouldn't save the steam content I hope, that would prevent actual streaming. But in either case, that does remind me: I think there is a tiny little buffer that is used for content type auto detect in RequestParsers...we would know if a stream is empty or not there.

solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java

markrmiller · 2022-03-15T19:27:44Z

I don't think we need this - tried running tests locally without it and using S3_MOCK_RULE to build the client directly and things seemed fine.

I actually took things out one by one from this class on the last update, and I left this because the tests passed as I removed up to this point and then failed when I removed this. But I tried again, and it seems to have passed, so perhaps some stale result issue I saw. Looks like it can come out if we go by your result and my latest.

markrmiller · 2022-03-22T01:17:10Z

Okay, I had missed in the patch you passed me how you proposed dealing with the toolchain dep issue - just using that instead of servlet-api - that seems fine to me.

I spent a bit of time on the 0 byte inputstream issue, but in the end, I think your approach is likely fine to start with - given that it works in the case that we currently know causes an issue, and if it stopped working for that case, tests would fail, I've come to think this solution is fine until we find it not to be.

This cleanup push that just went up includes a couple required changes to jetty config that I had forgotten are required and introduce the biggest upgrade issues a user would face - the existing config will not work. There is a fix required in the main jetty xml config where the rewrite handler is referenced by id instead of refid - that fails on jetty 10. And there is gzip config that references methods that are now removed and for the most part without replacement. They have to be changed.

risdenk · 2022-10-24T03:14:02Z

setDeflaterPoolCapacity addressed in 22cfe1d

This reverts commit 7d799d8.

solr/core/src/java/org/apache/solr/logging/LogWatcher.java

solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java

Co-authored-by: David Smiley <dsmiley@apache.org>

HoustonPutman

One question, one formatting thing.

Otherwise this looks good to go.

Btw we need to upgrade, as we are seeing jetty/jetty.project#8558 in our failing tests, and need an upgrade path when Jetty 10.0.13 is released.

solr/core/src/java/org/apache/solr/request/json/RequestUtil.java

solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java

risdenk · 2022-11-03T21:27:50Z

Btw we need to upgrade, as we are seeing jetty/jetty.project#8558 in our failing tests, and need an upgrade path when Jetty 10.0.13 is released.

yup this also affects Jetty 9.4.x from what I gather from SOLR-16099

This reverts commit 3ad2030.

risdenk · 2022-11-04T14:07:09Z

@markrmiller do you have any concerns on the current state of this PR? I'm hoping to merge it soon.

risdenk · 2022-11-04T14:33:58Z

I haven't looked at these yet or see if they reproduce.

./gradlew check -Pvalidation.errorprone=true -Ptests.filter=@Nightly
...
ERROR: The following test(s) have failed:
  - org.apache.solr.hdfs.cloud.SharedFileSystemAutoReplicaFailoverTest.test (:solr:modules:hdfs)
    Test output: /Users/risdenk/repos/apache/solr/solr/modules/hdfs/build/test-results/test/outputs/OUTPUT-org.apache.solr.hdfs.cloud.SharedFileSystemAutoReplicaFailoverTest.txt
    Reproduce with: gradlew :solr:modules:hdfs:test --tests "org.apache.solr.hdfs.cloud.SharedFileSystemAutoReplicaFailoverTest.test" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=8633D060D495F1DF -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitAfterFailedSplit2 (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitAfterFailedSplit2" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=8633D060D495F1DF -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitWithChaosMonkey (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitWithChaosMonkey" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=8633D060D495F1DF -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=8633D060D495F1DF -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=8633D060D495F1DF -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

reran again and got this:

./gradlew check -Pvalidation.errorprone=true -Ptests.filter=@Nightly
...
ERROR: The following test(s) have failed:
  - org.apache.solr.hdfs.cloud.SharedFileSystemAutoReplicaFailoverTest.test (:solr:modules:hdfs)
    Test output: /Users/risdenk/repos/apache/solr/solr/modules/hdfs/build/test-results/test/outputs/OUTPUT-org.apache.solr.hdfs.cloud.SharedFileSystemAutoReplicaFailoverTest.txt
    Reproduce with: gradlew :solr:modules:hdfs:test --tests "org.apache.solr.hdfs.cloud.SharedFileSystemAutoReplicaFailoverTest.test" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=16377EC258C8C266 -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitWithChaosMonkey (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitWithChaosMonkey" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=16377EC258C8C266 -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitAfterFailedSplit2 (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.testSplitAfterFailedSplit2" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=16377EC258C8C266 -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=16377EC258C8C266 -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-1

  - org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod (:solr:core)
    Test output: /Users/risdenk/repos/apache/solr/solr/core/build/test-results/test/outputs/OUTPUT-org.apache.solr.cloud.api.collections.ShardSplitTest.txt
    Reproduce with: gradlew :solr:core:test --tests "org.apache.solr.cloud.api.collections.ShardSplitTest.classMethod" -Ptests.jvms=5 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=16377EC258C8C266 -Ptests.filter=@Nightly -Ptests.file.encoding=ISO-8859-

working on checking on main to see if this is a problem there too.

risdenk · 2022-11-04T15:58:33Z

The above failures also happen on main branch so at least it is nothing new added from this PR for nightly tests.

* Upgrades to Jetty 10.0.12 * Upgrades dropwizard metrics 4.2.12 for `dropwizard-metrics9` -> `dropwizard-metrics10` * Upgrades log4j 2.19.0 and slf4j 2.0.3 for Jetty 10 * For s3mock specifically, upgrade spring-boot 2.5.14 and spring 5.3.23 to handle Jetty 10 Co-authored-by: Kevin Risden <krisden@apache.org> Co-authored-by: David Smiley <dsmiley@apache.org>

markrmiller force-pushed the SOLR-15955 branch from 406d43f to 9dc830d Compare February 9, 2022 21:26

sonatype-lift bot reviewed Feb 9, 2022

View reviewed changes

solr/modules/s3-repository/build.gradle Outdated Show resolved Hide resolved

markrmiller force-pushed the SOLR-15955 branch 2 times, most recently from ba3c027 to 4a9e65c Compare February 9, 2022 23:18

madrob reviewed Feb 11, 2022

View reviewed changes

solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java Outdated Show resolved Hide resolved

solr/modules/hdfs/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java Outdated Show resolved Hide resolved

solr/modules/s3-repository/build.gradle Outdated Show resolved Hide resolved

madrob reviewed Feb 11, 2022

View reviewed changes

.github/workflows/manual.yml Outdated Show resolved Hide resolved

markrmiller force-pushed the SOLR-15955 branch 3 times, most recently from 490bfe6 to 5a8744e Compare February 14, 2022 23:20

madrob reviewed Feb 16, 2022

View reviewed changes

madrob requested changes Feb 20, 2022

View reviewed changes

gradle/testing/randomization/policies/solr-tests.policy Outdated Show resolved Hide resolved

gradle/testing/randomization/policies/solr-tests.policy Outdated Show resolved Hide resolved

solr/modules/hdfs/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java Outdated Show resolved Hide resolved

markrmiller force-pushed the SOLR-15955 branch 2 times, most recently from 26c26c8 to 69a9803 Compare February 22, 2022 01:41

madrob mentioned this pull request Feb 28, 2022

Upgrade to Jetty 9.4.45.v20220203 #699

Closed

markrmiller force-pushed the SOLR-15955 branch from 69a9803 to 5257851 Compare March 4, 2022 01:07

madrob reviewed Mar 4, 2022

View reviewed changes

markrmiller force-pushed the SOLR-15955 branch from 5257851 to 166bd8d Compare March 4, 2022 03:28

markrmiller marked this pull request as ready for review March 4, 2022 03:45

markrmiller force-pushed the SOLR-15955 branch 2 times, most recently from 03986cd to 5863b10 Compare March 4, 2022 05:54

madrob reviewed Mar 8, 2022

View reviewed changes

markrmiller force-pushed the SOLR-15955 branch from 5863b10 to 17bcd97 Compare March 22, 2022 01:10

markrmiller added 3 commits May 3, 2022 13:33

SOLR-15955: Update Jetty to version 10.

ae1cbc3

SOLR-15955: Updates.

d3c855e

SOLR-15955: Use Jetty 10.0.8 and remove test policy / read permission.

0313f30

risdenk added 3 commits October 23, 2022 23:29

Revert "Address isMultipart comment"

abb36d4

This reverts commit 7d799d8.

Address isMultipart comment

73dd4f9

Really address deprecated setDeflaterPoolCapacity

247df45

risdenk requested review from dsmiley, risdenk and elyograg October 24, 2022 04:03

risdenk approved these changes Oct 24, 2022

View reviewed changes

dsmiley approved these changes Oct 24, 2022

View reviewed changes

solr/core/src/java/org/apache/solr/logging/LogWatcher.java Outdated Show resolved Hide resolved

solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java Outdated Show resolved Hide resolved

risdenk and others added 6 commits October 24, 2022 10:36

Update solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java

7063376

Co-authored-by: David Smiley <dsmiley@apache.org>

Update solr/core/src/java/org/apache/solr/logging/LogWatcher.java

ba5dff5

Co-authored-by: David Smiley <dsmiley@apache.org>

Merge branch 'main' into SOLR-15955

34f966b

Merge branch 'main' into SOLR-15955

c5f571f

Merge branch 'main' into SOLR-15955

cc91255

Merge branch 'main' into SOLR-15955

d4e1a52

HoustonPutman approved these changes Nov 3, 2022

View reviewed changes

solr/core/src/java/org/apache/solr/request/json/RequestUtil.java Outdated Show resolved Hide resolved

solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java Show resolved Hide resolved

risdenk added 3 commits November 3, 2022 17:13

Merge branch 'main' into SOLR-15955

053209d

Fix extra parentheses in RequestUtil

13cf430

Revert setAuthenticationStore change

3ad2030

Revert "Revert setAuthenticationStore change"

a080a8c

This reverts commit 3ad2030.

Merge branch 'main' into SOLR-15955

88b7330

risdenk merged commit 96ddf2c into apache:main Nov 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SOLR-15955: Update Jetty dependency to 10 #585

SOLR-15955: Update Jetty dependency to 10 #585

markrmiller commented Feb 1, 2022 •

edited by risdenk

Loading

markrmiller commented Feb 19, 2022

markrmiller commented Feb 21, 2022

markrmiller commented Mar 4, 2022

madrob Mar 8, 2022

markrmiller Mar 15, 2022

madrob Mar 15, 2022

markrmiller Mar 16, 2022

markrmiller commented Mar 15, 2022

markrmiller commented Mar 22, 2022

risdenk commented Oct 24, 2022

HoustonPutman left a comment

risdenk commented Nov 3, 2022

risdenk commented Nov 4, 2022

risdenk commented Nov 4, 2022 •

edited

Loading

risdenk commented Nov 4, 2022

		// if BinaryResponseParser.BINARY_CONTENT_TYPE, let the following fail below - we may have adjusted the content without updating the content type
		// problem in this case happens in a few tests, one seems to happen with kerberos and remote node query (HttpSolrCall's request proxy)

SOLR-15955: Update Jetty dependency to 10 #585

SOLR-15955: Update Jetty dependency to 10 #585

Conversation

markrmiller commented Feb 1, 2022 • edited by risdenk Loading

markrmiller commented Feb 19, 2022

markrmiller commented Feb 21, 2022

markrmiller commented Mar 4, 2022

madrob Mar 8, 2022

Choose a reason for hiding this comment

markrmiller Mar 15, 2022

Choose a reason for hiding this comment

madrob Mar 15, 2022

Choose a reason for hiding this comment

markrmiller Mar 16, 2022

Choose a reason for hiding this comment

markrmiller commented Mar 15, 2022

markrmiller commented Mar 22, 2022

risdenk commented Oct 24, 2022

HoustonPutman left a comment

Choose a reason for hiding this comment

risdenk commented Nov 3, 2022

risdenk commented Nov 4, 2022

risdenk commented Nov 4, 2022 • edited Loading

risdenk commented Nov 4, 2022

markrmiller commented Feb 1, 2022 •

edited by risdenk

Loading

risdenk commented Nov 4, 2022 •

edited

Loading