mlbiscoc commented on code in PR #2935: URL: https://github.com/apache/solr/pull/2935#discussion_r1910698804
########## solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java: ########## @@ -413,6 +418,30 @@ public String getQuorumHosts() { return String.join(",", this.liveNodes); } + /** Live nodes should always have the latest set of live nodes but never remove initial set */ + private void setLiveNodes(Set<String> nodes) { + Set<String> liveNodes = new HashSet<>(nodes); + liveNodes.addAll(this.initialNodes); + this.liveNodes = Set.copyOf(liveNodes); + } + + public Set<String> getNodeNamesFromSolrUrls(List<String> urls) + throws URISyntaxException, MalformedURLException { + Set<String> set = new HashSet<>(); + for (String url : urls) { + String nodeNameFromSolrUrl = getNodeNameFromSolrUrl(url); + set.add(nodeNameFromSolrUrl); + } + return Collections.unmodifiableSet(set); + } + + /** URL to cluster state node name (http://127.0.0.1:12345/solr to 127.0.0.1:12345_solr) */ + public String getNodeNameFromSolrUrl(String solrUrl) + throws MalformedURLException, URISyntaxException { + URL url = new URI(solrUrl).toURL(); + return url.getAuthority() + url.getPath().replace('/', '_'); + } + Review Comment: Not sure if these methods belong here but it is required to convert the initial set of String urls into cluster state node names ########## solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java: ########## @@ -61,6 +67,7 @@ public abstract class BaseHttpClusterStateProvider implements ClusterStateProvid private int cacheTimeout = EnvUtils.getPropertyAsInteger("solr.solrj.cache.timeout.sec", 5); public void init(List<String> solrUrls) throws Exception { + this.initialNodes = getNodeNamesFromSolrUrls(solrUrls); Review Comment: > I think the idea of this JIRA issue is that we'll take the Solr URLs as configured and use this is the initial / backup liveNodes. I think this is a very simple idea to to document/understand/implement. That makes sense. This leaves me with a few questions then. Shouldn't this take a list of `URL` or `URI` java object to verify actual non malformed URLs instead of a list of URLs? I created the functions below to convert these Strings into cluster state nodeNames for `liveNodes` -- 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