dsmiley commented on PR #3507: URL: https://github.com/apache/solr/pull/3507#issuecomment-3222118251
> Not sure I understand the reason for this refactoring Please read the PR description again for the reason. I was impressed to read CoPilot AI's [PR description](https://github.com/dsmiley/solr/pull/2#issue-3349659618) (given my minimal prompt) that it inferred the reason eloquently: separation of concerns. > which seems to introduce initialization order issues (coreContainer.getZkController() not yet instantiated when DistributedCollectionCommandContext is constructed). Initialization order will always be a challenge. Anybody working on SolrCloud startup stuff must struggle / deal with it; I can only assume you did as well when you worked on this code originally. I added comments to communicate things to avoid possible problems going forward. Maybe such comments suggest that this change is bad but I prefer such comments. A couple of these components were extracting stuff out of CoreContainer (via getZkController) during construction when _they didn't need to_. I'd argue that this challenge might explain why CoreContainer had this SolrCloud specific matter in the first place when it's a better fit inside ZkController. > There are 18 occurrences of if (isZooKeeperAware()) in CoreContainer of this PR, Hmm. I only see one, and it's a removal. > so if the goal is to remove SolrCloud concerns from CoreContainer, I'd suggest to first decide the strategy then apply it to the various components (DistributedCollectionCommandRunner would be one of those). I would love to hear what such a strategy/vision might look like; maybe you've thought about it. But I don't think the absence of such should stop incremental progress of separation of concerns. For example, the very minor thing in this PR that refactors out a `ZkController.waitForPendingTasksToComplete` is a clear step in this direction. -- 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
