patsonluk opened a new pull request, #2607: URL: https://github.com/apache/solr/pull/2607
https://issues.apache.org/jira/browse/SOLR-17386 # Description It was reported that calling /admin/cores?action=RELOAD&core=<synthetic core on coordinator node> gives 500 # Cause This is caused by the change introduced in https://github.com/apache/solr/pull/2438, which core from coordinator node is now of class `SyntheticSolrCore`, which is a child class of `SolrCore`, (instead of being `SolrCore`) and the dummy coordinator collection no longer registers against the zk. There are 2 main issues arise because of this change: 1. On a `reload` call of the coordinator core, since the implementation class `SyntheticSolrCore` does not override `reload`, it will run the `SolrCore` implementation which create a new `SolrCore` (instead of `SyntheticSorlCore`) as a replacement. 2. In `CoreContainer#reload`, it will attempt to load the `DocCollection` from ZK (via cluster state) of the core's collection, and perform various logic (index writer, replication) base on the result. However, for synthetic solr core, such collection does not really exist. # Solution Corresponding to the causes: 1. Overload the `reload` method in `SyntheticSolrCore` , it is a bit hard to just call `super.reload` as it will require quite a bit of refactoring. Instead, we will just use a simplified (ignore all the write/indexing handling) version of `reload`. 2. Introduce a `SolrCore#isSynthetic` method, which return false for all cores except from `SyntheticSolrCore`. Then in `CoreContainer#reload` make a check on such method before getting the `DocCollection` from ZK. For `SyntheticSolrCore`, it will get null `DocCollection` which is already being handled in existing `CoreContainer` logic # Concerns I have concerns on the scope of change in this PR, in particular we will need to modify existing core classes `SolrCore` and `CoreContainer`. Both need to be somehow aware of the `Synthetic` core, which is really not ideal. Another option is to refactor the code to break things down and then override the corresponding parts in `SyntheticSolrCore`, this could be quite a bit of refactoring and might even change more of the existing core classes (but then they will not be "synthetic core" aware anymore!) Any thoughts please? @dsmiley @noblepaul 🙏🏼 # Tests Added `TestCoordinatorRole#testCoreReload` to perform config update then followed by a core reload. Ensure that core reload returns success response code and query updates according to the config update. # Checklist Please review the following and check all that apply: - [x] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability. - [x] I have created a Jira issue and added the issue ID to my pull request title. - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended) - [x] I have developed this patch against the `main` branch. - [x] I have run `./gradlew check`. - [x] I have added tests for my changes. - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide) -- 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]
