github-actions[bot] commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-2638303410
This PR is now closed due to 60 days of inactivity after being marked as
stale. Re-opening this PR is still possible, in which case it will be marked
as active again.
--
This
github-actions[bot] closed pull request #2221: SOLR-17131: Optimize rows=0
since score/sort isn't necessary
URL: https://github.com/apache/solr/pull/2221
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go
github-actions[bot] commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-2525348954
This PR has had no activity for 60 days and is now labeled as stale. Any
new activity will remove the stale label. To attract more reviewers, please
tag people who might be fam
risdenk opened a new pull request, #2221:
URL: https://github.com/apache/solr/pull/2221
https://issues.apache.org/jira/browse/SOLR-17131
# Description
If a query has `rows=0` Solr does not need to sort the results or even
compute the score. This means the filterCache can be mor
gus-asf commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-2071000102
So code that does something with that maxScore value could hit
null/undefined? That's sort of iffy for back compatibility...
On Mon, Apr 22, 2024 at 4:51 PM David Smiley ***@***.***>
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-2070929291
I think the change is "safe" from a test stability standpoint.
There is one change to be mentioned in CHANGES.txt and probably in release
notes. If rows=0 then maxScore will no longer
gus-asf commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-2070903192
I guess the question is more a matter of how safe you think this change
is... We all know that there are some flaky tests. The test doesn't sound
related (being admin/recovery related no
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-2070806945
Shall I just merge this for 9.6? I think it's embarrassing we've had this
perf issue for so long without it being noticed. match-all-docs is so common,
like in ping queries for one.
dsmiley commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1574045448
##
solr/benchmark/src/java/org/apache/solr/bench/MiniClusterState.java:
##
Review Comment:
CC @tflobbe ; you added the setUseHttp1 call 1 year ago.
--
This is an
dsmiley commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1571706033
##
solr/benchmark/src/java/org/apache/solr/bench/MiniClusterState.java:
##
Review Comment:
The field `useHttp1` is initialized with `Boolean.getBoolean("solr.http1")`
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-2016351884
There is great work here; looking forward to it making 9.6 :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
UR
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1919850528
I pushed 80f11d5 which has a benchmark for checking a bunch of this. I
haven't compared against main yet, but on my machine:
```
Benchmark (flScore) (matchAllDocs)
risdenk commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1471790378
##
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##
@@ -1682,7 +1684,6 @@ private void getDocListC(QueryResult qr, QueryCommand
cmd) throws IOExceptio
risdenk commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1471411181
##
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##
@@ -1818,7 +1820,7 @@ private void getDocListNC(QueryResult qr, QueryCommand
cmd) throws IOExcepti
risdenk commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1471409812
##
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##
@@ -169,6 +160,20 @@ public void prepare(ResponseBuilder rb) throws IOException
{
t
risdenk commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1471406109
##
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##
@@ -147,15 +147,6 @@ public void prepare(ResponseBuilder rb) throws IOException
{
}
dsmiley commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1471266067
##
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##
@@ -1682,7 +1684,6 @@ private void getDocListC(QueryResult qr, QueryCommand
cmd) throws IOExceptio
risdenk commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1471253926
##
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##
@@ -1682,7 +1684,6 @@ private void getDocListC(QueryResult qr, QueryCommand
cmd) throws IOExceptio
risdenk commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1471252293
##
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##
@@ -169,6 +160,20 @@ public void prepare(ResponseBuilder rb) throws IOException
{
t
dsmiley commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1470537795
##
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##
@@ -169,6 +160,20 @@ public void prepare(ResponseBuilder rb) throws IOException
{
t
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1915662767
Updated this based on talking w/ @dsmiley. the `useFilterCache` part I was
confused on that it always applies to `q` so my change didn't make sense. I
pushed a change that made it clear what
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1915447012
@uschindler See
[getDocListNC](https://github.com/apache/solr/blob/326afddeddaa2092086e6f6b27f8bc35bf6d79cf/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java#L1812)
which alre
uschindler commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1915232387
Hi,
I already responded on Slack, but I now found this PR and wanted to say some
additional things:
There is also a separate collector and on top of that a separate method in
`I
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1915205945
Let's use GH links to ensure we understand each other. You are referring to
[this](https://github.com/apache/solr/blob/e53bf109ba2d35e1aff6f9a5c428fff5eff63f7b/solr/core/src/java/org/apache/
risdenk commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1469808152
##
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##
@@ -1818,7 +1820,7 @@ private void getDocListNC(QueryResult qr, QueryCommand
cmd) throws IOExcepti
dsmiley commented on code in PR #2221:
URL: https://github.com/apache/solr/pull/2221#discussion_r1469804423
##
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##
@@ -1818,7 +1820,7 @@ private void getDocListNC(QueryResult qr, QueryCommand
cmd) throws IOExcepti
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1914972100
> You claim that your approach to this uses the filter cache (for q)
With my change, for the case of `rows=0`, the filtercache docset will be
used to compute the result using q.
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1914960070
You claim that your approach to this uses the filter cache (for `q`). Yet
it does not because it calls `getDocSetNC`. I feel compelled to -1 this
because that's what `useFilterCache` means
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1914939631
I addressed the queryResultWindowSize with 326afdd when rows=0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
U
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1914900925
> is that queryResultWindowSize should be ignored when rows=0 to allow us
to run this query as fast as possible, even if this means not caching
queryResultWindowSize docs -- because we'd ra
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1914878120
I made a slight change to improve the variable name I added - c6618d6
> The method you are attempting to modify calls getDocListAndSetNC when
useFilterCache=false, which is well optimi
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1914809682
> (Me) The other code path is actually already optimized for rows==0 and no
scores
The method you are attempting to modify calls `getDocListAndSetNC` when
`useFilterCache=false`, whic
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1914730907
> The other code path is actually already optimized for rows==0 and no scores
Which code path are you talking about?
> I think the solution is to not save the results into queryR
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1913695210
I also think a `queryResultWindowSize` of 1 (which is the default if it
isn't specified in solrconfig and isn't documented AFAICT) should not bump up a
request for no rows to a window of a p
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1913487230
I see what's happening. The `queryResultCache`'s `queryResultWindowSize` is
forcing the actual number of documents to be raised from 0 to that window
(which is 1 in this test config), thus
risdenk commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1913296690
> Is there a unit test you can recommend that I might run in a debugger
looking at key spots, that would show that a score is being computed wastefully?
`TestMainQueryCaching#testConst
dsmiley commented on PR #2221:
URL: https://github.com/apache/solr/pull/2221#issuecomment-1912952744
> does not need to sort the results or even compute the score
Is there a unit test you can recommend that I might run in a debugger
looking at key spots, that would show that a score i
37 matches
Mail list logo
