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.

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

SOLR-13350, SOLR-17298: multi-threaded search: don't (yet) support query limits #2590

Closed
wants to merge 1 commit into from

Conversation

cpoerschke
Copy link
Contributor

Code change only here. Separately we still need to document that the multiThreaded=true parameter is not yet supported for queries with query limits amongst other things.

https://issues.apache.org/jira/browse/SOLR-13350
https://issues.apache.org/jira/browse/SOLR-17298

@cpoerschke
Copy link
Contributor Author

cpoerschke commented Jul 24, 2024

https://github.com/apache/solr/actions/runs/10079922542/job/27868413918?pr=2590 CI failure does sound related, will investigate.

> Task :solr:core:wipeTaskTemp
ERROR: The following test(s) have failed:
  - org.apache.solr.ltr.TestLTRQParserPlugin.ltr_expensiveFeatureRescoringAndPartialResultsNotTolerated_shouldRaiseException (:solr:modules:ltr)
    Test history: https://ge.apache.org/scans/tests?search.rootProjectNames=solr-root&tests.container=org.apache.solr.ltr.TestLTRQParserPlugin&tests.test=ltr_expensiveFeatureRescoringAndPartialResultsNotTolerated_shouldRaiseException http://fucit.org/solr-jenkins-reports/history-trend-of-recent-failures.html#series/org.apache.solr.ltr.TestLTRQParserPlugin.ltr_expensiveFeatureRescoringAndPartialResultsNotTolerated_shouldRaiseException
    Test output: /tmp/src/solr/solr/modules/ltr/build/test-results/test/outputs/OUTPUT-org.apache.solr.ltr.TestLTRQParserPlugin.txt
    Reproduce with: ./gradlew :solr:modules:ltr:test --tests "org.apache.solr.ltr.TestLTRQParserPlugin.ltr_expensiveFeatureRescoringAndPartialResultsNotTolerated_shouldRaiseException" -Ptests.jvms=96 "-Ptests.jvmargs=-XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 -XX:ReservedCodeCacheSize=120m" -Ptests.seed=9817284D4195A5AA -Ptests.timeoutSuite=600000! -Ptests.file.encoding=ISO-8859-1
2> 13276 ERROR (TEST-TestLTRQParserPlugin.ltr_expensiveFeatureRescoringAndPartialResultsNotTolerated_shouldRaiseException-seed#[9817284D4195A5AA]) [n: c: s: r: x: t:] o.a.s.u.RestTestBase query failed JSON validation. error: Path not found: /error/msg
  2>  expected: /error/msg=='org.apache.solr.search.QueryLimitsExceededException: Limits exceeded! (Learning To Rank rescoring - The full reranking didn\'t complete. If partial results are tolerated the reranking got reverted and all documents preserved their original score and ranking.): Query limits: [TimeAllowedLimit:LIMIT EXCEEDED]'
  2>  response: {
  2>   "responseHeader":{
  2>     "partialResults":true,
  2>     "status":0,
  2>     "QTime":406,
  2>     "params":{
  2>       "fv":"true",
  2>       "q":"_query_:{!edismax qf='id' v='8^=10 9^=5 7^=3 6^=1'}",
  2>       "indent":"on",
  2>       "fl":"id,score",
  2>       "timeAllowed":"300",
  2>       "rows":"4",
  2>       "wt":"json",
  2>       "partialResults":"false",
  2>       "rq":"{!ltr model=slowModel reRankDocs=3}"
  2>     }
  2>   },
  2>   "response":{
  2>     "numFound":0,
  2>     "start":0,
  2>     "maxScore":0.0,
  2>     "numFoundExact":true,
  2>     "docs":[ ]
  2>   }
  2> }

@cpoerschke
Copy link
Contributor Author

cpoerschke commented Jul 25, 2024

Unable to reproduce the TestLTRQParserPlugin.ltr_expensiveFeatureRescoringAndPartialResultsNotTolerated_shouldRaiseException issue locally. Nothing obvious from inspecting the CI output. Mailing list archive shows that occasionally this has happened before too: https://lists.apache.org/[email protected]:lte=1y:%22org.apache.solr.ltr.TestLTRQParserPlugin.ltr_expensiveFeatureRescoringAndPartialResultsNotTolerated_shouldRaiseException%22

@cpoerschke cpoerschke marked this pull request as ready for review July 25, 2024 10:13
@sigram
Copy link
Contributor

sigram commented Jul 25, 2024

I wonder whether we should add some information to the response header when multiThreaded=true is explicitly set in the params but then it's internally disabled due to the other conditions (limits, post filters, etc..) because it's not obvious anymore if MT was actually used as requested.

@dsmiley
Copy link
Contributor

dsmiley commented Jul 25, 2024

First started failing April 8th. SOLR-13350 landed May 6th.

I agree that we need clear diagnostics on when segments are actually searched in parallel or not.

@cpoerschke
Copy link
Contributor Author

closing in favour of #2595 adding support for query limits;

separately #2596 to add documentation and responseHeader.multiThreadedUsed diagnostics;

@cpoerschke cpoerschke closed this Jul 26, 2024
@gus-asf
Copy link
Contributor

gus-asf commented Jul 26, 2024

First started failing April 8th. SOLR-13350 landed May 6th.

I agree that we need clear diagnostics on when segments are actually searched in parallel or not.

There actually are two log messages that indicate this but they were cryptic I updated them in #2595 here: https://github.com/apache/solr/pull/2595/files#diff-99978700f1c69d6a5f6c2190f89c98cfe20c441161db5a183ec002e15cb1be28R1924

@cpoerschke
Copy link
Contributor Author

... they were cryptic I updated them in #2595 ...

I wonder if log level diagnostics might be enough, perhaps the diagnostic could be debug generally and info when the multiThreadedUsed contradicts multiThreaded in the request. A responseHeader element seems a bit messy in the code base -- see #2596 -- and also it seems impossible when the request was served from the cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants