punAhuja commented on PR #5172: URL: https://github.com/apache/solr/pull/5172#issuecomment-6261738565
> Thanks for the fix! The `numFoundExact` fix is right, and removing the `numFound--` on a failed push is a good cleanup. My concern is the new `numFound` value. > > In Solr, `numFound` is the number of matching documents, independent of rows/start/shards.rows, never the size of the page or candidate pool. Even when approximate (minExactCount, timeAllowed), it estimates the match count, as do grouping's per-group counts. `responseDocs.setNumFound(combinedShardDocs.size())` makes numFound depend on the size of the fusion pool. Each sub-query's queue holds at most perQueryQueueSize = max(start + rows, shards.rows), so the count follows how many docs were requested rather than how many matched. > > * rows=0 returns numFound: 0. The queue size is 0 and the shards return no docs, so the fused list is empty. This breaks count-only and facet-only requests, which previously got a reasonable lower bound. > * It can drop below a known match count. With two largely overlapping sub-queries of 6 hits each and rows=2, the fused list has 2–4 docs, so the response reports 2–4 even though we know at least 6 docs match. > * It changes while paging. As start + rows grows the pool, numFound grows too, so page counts built on numFound shift from page to page. > > `numFoundExact: false` doesn't fully cover this. Callers read an inexact count as "at least N", and in these cases N is too low. > > Suggestion: use the larger of the two counts we have: > > ``` > maxQueryNumFound = max(maxQueryNumFound, queryNumFound); // per query, summed across shards > .... > responseDocs.setNumFound(max(maxQueryNumFound, combinedShardDocs.size())); > responseDocs.setNumFoundExact(hitCountIsExact && !someQueryListTruncated); > ``` Thanks @ercsonusharma for the careful review. I have addressed your comments in my latest commit. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
