murblanc commented on pull request #1845: URL: https://github.com/apache/lucene-solr/pull/1845#issuecomment-690736516
Thanks @thelabdude for your long and useful comment. Let me try to give my take on this. By saying writing plugins should be easy in point 1, I meant the boilerplate code should not get in the way and force more code lines than really necessary. It's the ability to write for example things such as `for (Replica replica : shard.replicas())`, `Collections.shuffle(nodes, new Random())` and `replica.getShard().getState()`. Such compact statements do not require any explanation and are natural for any Java programmer. My thinking is that once the interfaces in `org.apache.solr.cluster.placement` are understood, given an understanding of the corresponding _concepts_ in Solr, writing a placement plugin is relatively accessible. If you look at `SamplePluginAffinityReplicaPlacement`, most of the complexity is not Solr related but for implementing the _business logic_ of the placement decisions (sorting, filtering etc). For point 2 I didn't want the API to get in the way of efficiency. The current implementation of the API is definitely not optimized (no multithreading etc) but this can be changed without impact on the API or on existing plugins. I believe we reached a good place. I too prefer how the attribute fetching looks now (@noblepaul's contribution) than what I initially proposed. Point 3 is very important. Any internal Solr interface is relatively easy to change, we have the code using it and adapt it as the interface is modified. Once we start handing these interfaces to external code (external to the lucene-solr github repo really), then changing (or not changing) them is a lot more complex and painful. My assumption here is that placement code might be implemented by outside users to suit their specific needs, and that code might not be contributed back to the project (as opposed to the plugin I wrote and that will be the default one and a possible starting point for custom ones). Therefore, we want to be able to maintain these interfaces unchanged even if internal implementation changes. Of course if internal _concepts_ change then the interfaces will likely have to change. For example if the notion of shard leader goes away (imagine...) then of course that part of the API (be it defined on the `Replica` or on the `Shard`) doesn't really make sense anymore. Take as example the ongoing discussions about configuration. The plugin writer should not have to change the code based on how and where we decide placement plugin configuration should live. Last, the cluster abstractions for the placement plugins do not necessarily represent the existing cluster! In the initial (current) proposal they do (see `SimpleClusterAbstractionsImpl`), but soon we'll want to provide a forward view of how the cluster will (likely) look after known past assignment decisions are applied (these things are quite slow to happen due to the structure of `Overseer`). We had a similar separate mechanism in Autoscaling in 8x (the notion of `Session`), here we can keep the plugin writer completely agnostic of that fact, and placement decisions will simply become better (esp. under high load) once we change the internal implementation. This is to say the interfaces should really focus on the _concepts_ at play and not the current internal implementation of these concepts, as the implementations of these interfaces will drift away from their internal counterparts quite soon. If we want to recreate a simulation environment, focusing on concepts rather than implementation simplifies things a lot as well. I guess everybody agrees on the last points made though. All this being said, it would be better to unify cluster abstractions (and possibly other abstractions) that are to be used by external code and have a single set of abstractions (interfaces). External uses of such interfaces include placement code (this PR), event processing (see [SOLR-14749](https://issues.apache.org/jira/browse/SOLR-14749)) and possibly other external code that needs to interact with the cluster. The interfaces defined here were used to write the plugin, and were changed in the process to simplify the plugin code. I believe if we make them evolve to adapt to event processing we'll have a pretty good coverage of potential uses. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
