epugh commented on PR #2391: URL: https://github.com/apache/solr/pull/2391#issuecomment-3480335165
I asked Claude to summarize the last seven commits and he said: _Recent work on the `spike-zk-quorum` branch has focused on refining Solr's embedded ZooKeeper quorum mode implementation through targeted code improvements. The codebase has been streamlined by removing unnecessary variables and conditional logic in the `SolrZkServer` class, particularly eliminating redundant checks around the `zkServerEnabled` flag and renaming methods like `startZKSE` to more descriptive alternatives. Initialization has been optimized by removing the `/solr/initialized` ZooKeeper node, which was only needed for multi-threading protection that's unnecessary in the current implementation. The ZooKeeper initialization logic in `ZkContainer` has been restructured with clearer nesting and better separation between standalone and quorum modes, making the codebase more maintainable. These changes collectively enhance the embedded ZooKeeper quorum functionality while simplifying the code structure, resulting in a more robust and easier-to-understand implementation that properly manages ZooKeeper server resources throughout Solr's lifecycle._ I think my summary is: **I discovered that we have TWO ways of starting ZK, not including our independent `ZkTestServer` that runs a `ZkTestServer.ZKServerMain` during tests.** We now have: * For testing only, we have `org.apache.solr.cloud.ZkTestServer.ZKServerMain` that wraps `org.apache.zookeeper.server.ZooKeeperServer`. * `org.apache.solr.cloud.SolrZkServer` that wraps either a `org.apache.zookeeper.server.ZooKeeperServer` OR if there is more then one, then does a `org.apache.zookeeper.server.quorum.QuorumPeerMain` * `org.apache.solr.core.ZkContainer` that starts up `org.apache.zookeeper.server.embedded.ZooKeeperServerEmbedded` I am now wondering could we convert SolrZkServer to using a `ZooKeeperServerEmbedded` instead of a `ZooKeeperServer`? Secondly, now seeing the use of `QuorumPeerMain`, which currently kicks in if there are multiple ZooKeeper servers defined (presumably only in an external cluster set up), I wonder if we need to rename roles: `zookeeper_quorum` to `zookeeper`. If you have `zookeeper` then you fire up `ZooKeeperServerEmbedded`. And if not, then you default to `QuorumPeerMain`. Thirdly, why is `ZkContainer` in `o.a.s.core` and not `o.a.s.cloud`? I did take a stab at the `SolrZkServer` using the `ZooKeeperServerEmbedded`, updating the start/stop methods, but failed ;-(. Going to need to put this down for a bit. -- 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]
