nick-boss-tech opened a new pull request, #5131:
URL: https://github.com/apache/solr/pull/5131

   🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
   
   https://issues.apache.org/jira/browse/SOLR-5394
   
   ## What happens today
   
   **With facet.method=fcs and no local params, the segment tasks for a field 
all start at once.**
   
   `parseParams` starts the per-field thread count at -1 before this change 
([SimpleFacets.java#L183](https://github.com/nick-boss-tech/solr/blob/b6b2b8f10e9827e3b86e44649fb7966ce646c185/solr/core/src/java/org/apache/solr/request/SimpleFacets.java#L183)).
 A value of 0 or less means no limit in `PerSegmentSingleValuedFaceting` 
([L128](https://github.com/nick-boss-tech/solr/blob/b6b2b8f10e9827e3b86e44649fb7966ce646c185/solr/core/src/java/org/apache/solr/request/PerSegmentSingleValuedFaceting.java#L128)),
 so each segment gets a task on the facet executor. The ticket reports about 46 
facet executor threads from two fields. The global `facet.threads` parameter 
does not reach this path 
([SimpleFacets.java#L885](https://github.com/nick-boss-tech/solr/blob/b6b2b8f10e9827e3b86e44649fb7966ce646c185/solr/core/src/java/org/apache/solr/request/SimpleFacets.java#L885)).
 The links in this paragraph point at the base commit, before this change.
   
   ## What this change does
   
   **The default per-field thread count changes from -1 (no limit) to 1.**
   
   `parseParams` now starts at 1 
([SimpleFacets.java#L183](https://github.com/nick-boss-tech/solr/blob/967445622f9294e26e425ec494d37e5ca13d4fe4/solr/core/src/java/org/apache/solr/request/SimpleFacets.java#L183)).
 A `threads` local param still overrides it 
([L208-L211](https://github.com/nick-boss-tech/solr/blob/967445622f9294e26e425ec494d37e5ca13d4fe4/solr/core/src/java/org/apache/solr/request/SimpleFacets.java#L208-L211)).
 With the default, segments run one at a time on the facet executor. 
`threads=0` still runs on the calling thread, and `threads=N` still runs N at a 
time. This is a behavior change for `facet.method=fcs` requests that set no 
`threads` local param: they now run one segment at a time.
   
   ## Proof
   
   **The new test fails on the base code, and the class has no failures at this 
head.**
   
   `SimpleFacetsTest.testFacetThreadsDefaultIsSingleThread` is new. It checks 
the parsed thread count for a plain field, a keyed field, `threads=2`, and 
`threads=0`. On the base code (b6b2b8f10e98) it fails with `expected:<1> but 
was:<-1>`. At head 967445622f9294e26e425ec494d37e5ca13d4fe4, verified 
2026-10-07, `SimpleFacetsTest` runs 49 tests with 0 failures, 0 errors, and 1 
skipped.
   
   ## A choice to check
   
   **The fcs default is 1 in this change. The other route makes `facet.threads` 
control the fcs path.**
   
   - Implemented: segments still run on pool threads, one at a time. 
`facet.threads=0` does not switch fcs to the calling thread.
   - Alternative: read `facet.threads` on the fcs path, so `facet.threads=0` 
runs segments on the calling thread. The cost is that `facet.threads` would 
then set parallelism between segments as well as between fields.
   
   The reporter expected `facet.threads=0` to stop the extra threads. A 2014 
comment on the ticket proposed a default of 1. Was a default of 1 the right 
call, or should `facet.threads` control this path?
   
   ## Limits
   
   **The fcs path still does not read `facet.threads`.**
   
   - The reporter's exact case (`facet.threads=0` with `facet.method=fcs`) 
still uses the facet executor, one segment at a time.
   - Only the fcs branch reads the per-field thread count. Other methods are 
unchanged.
   - No timing comparison is included. Multi-segment indexes may be slower on 
this path.
   - If maintainers want `facet.threads` to control this path, that is a 
separate change. I can open a follow-up ticket and PR for it on request.
   
   Changelog: 
[`changelog/unreleased/SOLR-5394-fcs-default-threads.yml`](https://github.com/nick-boss-tech/solr/blob/967445622f9294e26e425ec494d37e5ca13d4fe4/changelog/unreleased/SOLR-5394-fcs-default-threads.yml)
   
   ### AI assistance
   
   AI agents assisted with research, implementation, review, and drafting. Nick 
Shanin directed the work and takes responsibility for this contribution.
   


-- 
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]

Reply via email to