Gus, what do you mean by "local management"? Anyway, I think I was misunderstood. And besides, admittedly you two aren't familiar with the classes in question. But here goes a clarification:
I'm concerned about internal SolrCloud code design/maintenance with respect to these abstractions -- some of which *are* still used. Maybe SolrCloudManager isn't an issue -- it's just a facade / holder to other stuff -- probably worth embracing further. I think the main concern here is DistribStateManager which is basically trying to abstract SolrZkClient. The question is... should we bother? To use it in some places and not others adds a degree of complexity / confusion and I question if it's serving any value. There is exactly one implementation (SolrZkClient based, of course). In Solr 8, there were four plus many anonymous subclasses. I could hypothetically imagine unit tests using some trivial in-memory like impl -- in Solr 8 that's "SimDistribStateManager" but we don't have tests that use such a thing. And besides, using embedded ZK is no big deal for a test; lots of our tests use ZK directly for this. The real value of DSM was to do fast simulations of scale. And perhaps to open a pathway to use something other than ZK... but I don't think that's realistic (a poor use of limited contributor time, not to mention ZK works pretty well). *If* we keep DistribStateManager, I think I would prefer that SolrZkClient *be* a DistribStateManager without having to wrap/delegate. What makes that non-trivial is the semantic differences in exceptions -- DSM maps some ZK specific exceptions (e.g. NoNodeException) into general ones (e.g. NoSuchElementException). Yet DSM actually fails to conceal ZK because it depends on ZK classes like Op and Watcher and even KeeperException! So that was an apparent failed attempt. Since DSM is not used *too* much, I could see simply replacing its method signatures with that of SolrZkClient. Or just remove DSM. This is my preference as it reduces complexity. ~ David Smiley Apache Lucene/Solr Search Developer http://www.linkedin.com/in/davidwsmiley On Mon, Jun 19, 2023 at 4:41 PM David Smiley <dsmi...@apache.org> wrote: > I noticed the SolrCloudManager concept added some time ago brought about > to abstract away SolrCloud in the context of doing simulated experiments on > auto-scaling. Essentially -- need to simulate SolrCloud and not actually > use a real SolrCloud. But that need and code went away in 9.0... > nonetheless SolrCloudManager and its friends (like DistributedStateManager) > are still around. I could imagine someone advocating for them > nonetheless. But the present state is very half-implemented as there is > code all over the place that assumes ZooKeeper (e.g. uses SolrZkClient or > ZkStateReader) instead of some of these abstractions. I think there is a > need to set a direction here -- do we embrace abstracting SolrCloud within > Solr or do we revert this stuff as needless indirection / concepts. > > I think there's lots of room to debate / review the particulars of > SolrCloudManager and friends if we do want to keep it. > DistributedQueueFactory isn't even used anymore. NodeStateProvider is only > for AttributeFactory; not very obvious. DistribStateManager is essentially > SolrZkClient but nonetheless still references ZK classes. > > ~ David Smiley > Apache Lucene/Solr Search Developer > http://www.linkedin.com/in/davidwsmiley >
