Set timeAllowed solr parameter to avoid solr wasting cycles on timed-out queries #11267
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR adds a timeAllowed parameter to Solr queries to prevent the server from wasting cycles on queries that have already timed out on the client side. The changes introduce a new parameter to control when the timeout is passed to Solr and set appropriate defaults for different use cases.
- Adds
timeAllowedparameter to Solr queries when client timeout is enabled - Introduces
_pass_time_allowedparameter to control timeout behavior - Updates timeout handling for trending data scripts and language count queries
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openlibrary/utils/solr.py | Core implementation of timeAllowed parameter and _pass_time_allowed control |
| openlibrary/plugins/worksearch/code.py | Updates execute_solr_query to support new timeout parameters |
| openlibrary/plugins/worksearch/languages.py | Disables timeAllowed for long-running language count query |
| scripts/solr_updater/trending_updater_hourly.py | Removes client timeout for trending data fetching |
| scripts/solr_updater/trending_updater_daily.py | Removes client timeout for daily trending scores |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Closes #11268
I was monitoring the solr logs, and noticed that some queries were running for 30+ seconds! Especially odd since in most places in OL we set a timeout of 10s for solr requests. Turns out solr won't abandon a query after the client closes the connection. (And this makes some sense, since it allows the query to effectively finish in the bg, so next time that query is made, caches should hopefully have been populated and it won't time out.)
But for our use cases we generally don't need that. And apparently solr has a url param for this!
timeAllowed={ms}. This lets us forward along the timeout effectively, and solr will stop processing after ms have passed and instead return a partial result.Technical
/updatedone by solr-updater ; those use httpx directly.language:eng(although note sort order might not wind up being correct). But for now just timeout.Testing
I patched this out, and it seems to have helped make us more resilient. I'm seeing solr perf more persistently in the sub 10ms zone (see graph). It seems like this might have increased our throughput, and is preventing large/slow queries from causing solr congestion.
Screenshot
Stakeholders