sigram commented on code in PR #2666: URL: https://github.com/apache/solr/pull/2666#discussion_r1731349309
########## solr/CHANGES.txt: ########## @@ -109,8 +109,11 @@ New Features Improvements --------------------- +* SOLR-17158: Users for whom partial results are uninteresting may set partialResultsAllowed=false. This allows Solr to reduce time spent processing partial results and omit them from the response. (Gus Heck, Andrzej Bialeki, hossman) Review Comment: Typo - my last name is Bialecki :) or actually BiaĆecki where non-ascii is allowed, but I've been using both forms. ########## solr/core/src/java/org/apache/solr/response/SolrQueryResponse.java: ########## @@ -139,30 +142,63 @@ public ReturnFields getReturnFields() { /** * If {@link #getResponseHeader()} is available, set {@link #RESPONSE_HEADER_PARTIAL_RESULTS_KEY} - * flag to true. + * attribute to true or "omitted" as required. */ - public void setPartialResults() { + public void setPartialResults(SolrQueryRequest req) { Review Comment: Should this be `static` now? ########## solr/core/src/java/org/apache/solr/response/SolrQueryResponse.java: ########## @@ -61,7 +64,7 @@ */ public class SolrQueryResponse { public static final String NAME = "response"; - public static final String RESPONSE_HEADER_PARTIAL_RESULTS_KEY = CommonParams.PARTIAL_RESULTS; Review Comment: Originally I did this on purpose to avoid getting these two strings out of sync. ########## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc: ########## @@ -71,6 +71,13 @@ Due to changes in Lucene 9, that isn't possible any more. === Configuration In solrconfig.xml, the `numVersionBuckets` and `versionBucketLockTimeoutMs` settings are now obsolete and ignored; a warning will be logged if specified. +=== Partial Results +When solr is used for reporting, legal discovery or other use cases that cannot accept partial results, yet it still needs to be protected from long-running queries, users may now opt out of receiving partial results. +This feature should eliminate some wasted processing and bandwidth, as well as simplifying client side logic, and reducing the chance of inappropriate responses to the user. +This can be enabled either via a request parameter (`allowPartialResults`), environment variable or system property. +The partialResults attribute in the request header will now have three possible states: `"true"`, `"omitted"` or absent, with the new `"omitted"` value indicating the difference between a timed out request with results omitted due to query limits and results that are merely empty because the query didn't match any docs. Review Comment: Somehow this sentence is very confusing to me ... maybe this: the new "omitted" value indicating that query returned partial results, which due to the setting were not allowed and they were then discarded? or something more English-like ;) ########## solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc: ########## @@ -71,6 +71,13 @@ Due to changes in Lucene 9, that isn't possible any more. === Configuration In solrconfig.xml, the `numVersionBuckets` and `versionBucketLockTimeoutMs` settings are now obsolete and ignored; a warning will be logged if specified. +=== Partial Results +When solr is used for reporting, legal discovery or other use cases that cannot accept partial results, yet it still needs to be protected from long-running queries, users may now opt out of receiving partial results. Review Comment: long-running queries -> queries exceeding limits ? ########## solr/core/src/java/org/apache/solr/response/SolrQueryResponse.java: ########## @@ -139,30 +142,63 @@ public ReturnFields getReturnFields() { /** * If {@link #getResponseHeader()} is available, set {@link #RESPONSE_HEADER_PARTIAL_RESULTS_KEY} - * flag to true. + * attribute to true or "omitted" as required. */ - public void setPartialResults() { + public void setPartialResults(SolrQueryRequest req) { NamedList<Object> header = getResponseHeader(); - if (header != null - && header.get(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY) == null) { - header.add(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, Boolean.TRUE); + if (header != null) { + if (header.get(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY) == null) { + Object value = partialResultsStatus(shouldDiscardPartials(req.getParams())); + header.add(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, value); + } } } + public static Object partialResultsStatus(boolean discarding) { + return discarding ? "omitted" : Boolean.TRUE; Review Comment: Should this be a constant somewhere? -- 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 to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org