linslee75 commented on PR #5194: URL: https://github.com/apache/solr/pull/5194#issuecomment-6356219831
> 🤖 _AI text below_ 🤖 _(reviewed by Nick Shanin)_ > > The "Behavior change" section says new fields are queryable after the next commit. That is incomplete. A facet on a new field before a commit fails with `undefined field` on every replica. The [diff against this PR](https://github.com/apache/solr/compare/linslee75:SOLR-18563-additive-schema-api-no-reload...nick-boss-tech:solr:solr-18563-searcher-reopen) adds a test that shows this. On upstream main, the same facet passes on the replica that receives the request. The other replica fails. > > Options: > > 1. **Reopen the searcher on every replica after an additive change.** Queries see the new field right after the command returns. Cost: one searcher open per replica for each schema request. On an empty two-replica collection, add-field takes about 0.5 ms longer. On a 20,000-document index, it takes about 2 ms longer. The reload on main takes about 97 ms on that index. > 2. **Keep the change and document the failure.** The Behavior change section and the changelog would say that facets and other queries on the field can fail until a commit. Cost: a visible change from today on every replica. > 3. **Keep the reload in SolrCloud and skip it only in standalone.** Cost: a long merge still blocks indexing in SolrCloud, which is the problem this PR fixes. > > **Recommendation: option 1.** The diff makes three changes: > > * `SchemaManager.java`: the searcher opens after an additive change in SolrCloud too, not only in standalone. The `else if (appliedWithoutReload)` becomes `if (appliedWithoutReload)`. > * `ZkIndexSchemaReader.java`: the reader keeps the core. After it sets a new schema from ZooKeeper, it opens a searcher: > > ```java > try { > solrCore.getSearcher(true, false, null); > } catch (SolrException e) { > if (!solrCore.isClosed()) { > throw e; > } > } > ``` > > * `TestManagedSchemaAPI.java`: the new test `testFacetOnNewFieldBeforeCommitOnEveryReplica`. > > The `try` block covers a reload that closes the old core while its schema watcher is still running. [SOLR-13795](https://issues.apache.org/jira/browse/SOLR-13795) fixed the same symptom in SolrCloud by reloading the core. This change removes that reload and opens the searcher without it. On the diff, `TestManagedSchemaAPI` passes 7 of 7 and `TestBulkSchemaAPI` passes 19 of 19. `TestSchemaChangeDuringMerge` passed on an earlier head. > > **Bottom line: take option 1.** It keeps queries working right after the command, and it keeps the fix that stops long merges from blocking indexing. Thanks, Nick. My intent with this PR was to make additive Schema API changes follow the existing Schemaless behavior: publish the updated schema without reloading the core or immediately reopening searchers on every replica. That avoids blocking indexing behind a long-running merge. With this behavior, a query or facet using a newly added field may fail on a replica until its searcher picks up the updated schema. I agree the Behavior change section and changelog should describe this clearly. Whether additive Schema API changes should instead reopen searchers on every replica, so the new field is immediately queryable everywhere, is a broader visibility-model decision. @dsmiley @epugh , could you weigh in on which behavior Solr should adopt? For this PR, I propose following the existing Schemaless behavior. -- 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]
