Repository navigation
Conversation
bin/solr commands funneling through CLIUtils.getSolrClient (and several that hand-built their own HttpJettySolrClient.Builder) inherited SolrHttpConstants' 60s connect / 600s idle defaults, which are sized for long-running SolrJ use cases like bulk indexing and replica recovery. A human running a CLI command against an unreachable node wants fast failure instead. Centralizes the 15s/30s timeouts DeleteTool already used (and CreateTool used to, before a recent refactor moved it onto the shared client) into CLIUtils.CLI_CONNECTION_TIMEOUT_SECONDS / CLI_IDLE_TIMEOUT_SECONDS, and applies them everywhere a CLI tool builds an HttpJettySolrClient. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HaP4sXv7KQBw9ZEqtvwug2
Contributor
Author
|
This was claude generated, but looking hrtorugh, it seem sto make sense. I don't know if we want to have a single place to configure hte client, feels like we have gone and back and forth on this.... |
…le-timeouts # Conflicts: # solr/core/src/java/org/apache/solr/cli/DeleteTool.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Spun out of review discussion on a
bin/solr createrefactor (SOLR-18320): CLI commands that construct aSolrClientviaCLIUtils.getSolrClient(or by hand-building anHttpJettySolrClient.Builder) currently inheritSolrHttpConstants' defaults — 60s connect / 600s idle. Those defaults are deliberately generous because they're shared by every SolrJ use case, including bulk indexing, shard-to-shard fan-out, and replica recovery, where a single request can legitimately take minutes.A human running a
bin/solrcommand at a terminal wants the opposite: fail fast against an unreachable or hung node.DeleteToolalready carried its own 15s/30s override for exactly this reason (andCreateToolused to, before a recent refactor accidentally dropped it by switching to the shared client builder).This PR centralizes that override:
CLIUtils.CLI_CONNECTION_TIMEOUT_SECONDS(15) andCLIUtils.CLI_IDLE_TIMEOUT_SECONDS(30).CLIUtils.getSolrClient's core builder and inCLIUtils.solrUrlFromConnection's builder, which covers every CLI tool that callsgetSolrClient(Healthcheck, SnapshotExport, Version, Assert, Export, Config, Create, Api, Delete, SnapshotList, Package, RunExample, Status, SnapshotCreate, SnapshotDelete).CLIUtils.getSolrClientand hand-build their ownHttpJettySolrClient.Builder:DeleteTool(replacing its hardcoded 15/30 with the shared constants),HealthcheckTool,ExportTool,PostLogsTool,StreamTool.Out of scope:
RunExampleTool's two internalwaitToSeeLiveNodes/example-bootstrapCloudSolrClientbuilds are left on the long defaults — those are local-example node-readiness polling loops with their own retry/backoff logic, not a user-facing connection attempt, so a short connect timeout there wouldn't offer the same benefit and risks interacting oddly with the poll loop.Test plan
./gradlew :solr:core:compileJava— compiles cleanly./gradlew :solr:core:test --tests DeleteToolTest --tests TestExportTool --tests StreamToolTest --tests CreateToolTest --tests HealthcheckToolTest --tests PostLogsToolTest— all pass (32 tests, 1 skipped)changelog/unreleased/🤖 Generated with Claude Code
https://claude.ai/code/session_01HaP4sXv7KQBw9ZEqtvwug2