[
https://issues.apache.org/jira/browse/SOLR-14680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17187964#comment-17187964
]
Andrzej Bialecki commented on SOLR-14680:
-----------------------------------------
On further reflection I think that this change needs to be either re-worked or
it should be reverted and committed again once it's done properly. In any case,
in its present form it's really not suitable for inclusion in branch_8x because
we can't guarantee that its final form will be compatible across future 8x
releases.
IMO here are some things that need to be changed in this API (and would have
been best done in the PR review before committing):
* the method naming in {{SolrCluster}} is awkward ... why not use the standard
getters, like {{getCollections()}}, {{getNodes()}} etc? The presence of the
"get" verb is actually informative, because it clearly indicates a read
operation.
* {{SimpleMap}} - the only reason this non-standard container exists in this
API is to be able to lazily cache & return the values. But it's a solution to a
problem that can be totally avoided if we define the API in a way that doesn't
require returning maps. Namely, as a pair of {{List<String>}} that returns just
entity names, and then {{getCollection}}, {{getNode}}, {{getConfig}} etc to
retrieve the entities. It's a simpler, more scalable, and more transparent API
than hiding the fetching & caching of values in a {{SimpleMap}}.
* {{SolrCluster.collections}} - this unnecessarily returns a map of
{{SolrCollection}}, in a similar way to the current {{ClusterState}} (which
also can lazily load DocCollection-s). But it would be sufficient and almost as
easy to use if it returned a list of names, with an additional accessor to get
the {{SolrCollection}}, optionally from an internal cache. BTW.
{{ConcurrentHashMap}} as a cache is totally non-scalable - imagine having
thousands of SolrCollection objects here...
* {{SolrNode.cores()}} actually returns replicas ... should be renamed to
{{getReplicas()}}. Again, what's wrong with standard getter names?
And finally, my main objection is that this API currently doesn't do anything,
it's totally disconnected and there's no proof (as in working usage) that it's
suitable or sufficient to be actually used for anything practical. IMHO it's
missing a few important methods related to state (as I explained earlier),
which would become apparent if it was used for anything practical.
> Provide simple interfaces to our concrete SolrCloud classes
> -----------------------------------------------------------
>
> Key: SOLR-14680
> URL: https://issues.apache.org/jira/browse/SOLR-14680
> Project: Solr
> Issue Type: Improvement
> Reporter: Noble Paul
> Assignee: Noble Paul
> Priority: Minor
> Labels: clean-api
> Time Spent: 10.5h
> Remaining Estimate: 0h
>
> All our current implementations of SolrCloud such as
> # ClusterState
> # DocCollection
> # Slice
> # Replica
> etc are concrete classes. Providing alternate implementations or wrappers is
> extremely difficult.
> SOLR-14613 is attempting to create such interfaces to make their sdk simpler
> The objective is not to have a comprehensive set of methods in these
> interfaces. We will start out with a subset of required interfaces. We
> guarantee is that signatures of methods in these interfaces will not be
> deleted/changed . But we may add more methods as and when it suits us
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
