Restore the explicit closes on solr clients in test to prevent resource starvation

[epugh]

This is a follow up to the PR #3947 where the explicit closing of SolrClient's in DistributedDebugComponentTest failed.

This now passes:

./gradlew :solr:core:test --tests "org.apache.solr.handler.component.DistributedDebugComponentTest" -Ptests.haltonfailure=false "-Ptests.jvmargs=-XX:+UseCompressedOops -XX:+UseParallelGC" -Ptests.multiplier=8

[dsmiley]

It's impossilbe that this fixes the problem. I still see the problem with this change.

[epugh]

Any more details? I was disappointed by this fix, but it did make the Gradle test command pass.

[janhoy]

The test failures I see from DistributedDebugComponentTest in Develocity, e.g. https://develocity.apache.org/s/wtusdzkwyoxow/tests/task/:solr:core:test/details/org.apache.solr.handler.component.DistributedDebugComponentTest/testSimpleSearch?top-execution=1 are all due to SSL being active, and the test stripping https:// prefix in the shard request https://github.com/apache/solr/blob/main/solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java#L72-L73 causing shard handler to fall back to default http:// prefix and failing. No idea why this would be different after switching to TestRule though. Leaving the https:// in the &shards param seems to fix the issue though.

[epugh]

I can try and make the change that you are suggesting @janhoy and see what happens. I swear my change was sorting it out!

[epugh]

Oh, I wonder if somethign about how I was running it locally meant it never did the ssl path?

[hossman]

@RandomizeSSL default odds (when not explicitly configured on the annotation) are increased based on tests.multiplier and tests.nightly

[dsmiley]

Closing, as the PR is invalid (explicit closes definitely is of no use).

[epugh]

@dsmiley I know you closed this as not working, but I did a bit more poking, and I pushed up a version that DOES require the SSL and then the test pass. I'm guessing that the the url made by the test rule is somewhat different than the url that was generated in the old way...

I made a video of using the annotation to force SSL and saw the bug: https://share.descript.com/view/D7IYXkdqdF3

I think you have a plan, or if you want I can clean up this PR and merge it for the fix.

[dsmiley]

I think changing this test isn't the way to go. What its doing isn't wrong. Doing so ignores a deeper root cause that likely affects other tests. We need to (somehow) start Solr with "urlScheme" configured on the HttpShardHandlerFactory in solr.xml, which the vast majority of tests use via their use of a test solr.xml that has it with var substitution. We could do so by having each test prep/configure/copy that solr.xml test fixture into place, which is what was happening before via SolrJettyTestBase, now removed. That would be a partial revert to this PR, in a sense -- we can certainly continue with the rule. But that's unsatisfying! Ideally this just gets set automatically if the circumstances are right. I'll post a PR today.

[dsmiley]

See #4008 for my PR