dsmiley commented on code in PR #2935:
URL: https://github.com/apache/solr/pull/2935#discussion_r1920955860
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -413,6 +430,19 @@ public String getQuorumHosts() {
return String.join(",", this.liveNodes);
}
+ public Set<String> getNodeNamesFromSolrUrls(List<String> urls) {
+ return urls.stream()
+ .map(
+ (url) -> {
+ try {
+ return getNodeNameFromSolrUrl(url);
+ } catch (MalformedURLException | URISyntaxException e) {
+ throw new RuntimeException("Failed to parse base Solr URL " +
url, e);
Review Comment:
IllegalArgumentException seems appropriate
##########
solr/solrj/src/java/org/apache/solr/common/util/Utils.java:
##########
@@ -786,6 +789,20 @@ public static String getBaseUrlForNodeName(
return urlScheme + "://" + hostAndPort + "/" + (isV2 ? "api" : "solr");
}
+ /**
+ * Construct base Solr URL to a Solr node name
+ *
+ * @param solrUrl Given a base Solr URL string (e.g.,
'https://app-node-1:8983/solr')
+ * @return URL that looks like {@code app-node-1:8983_solr}
+ * @throws MalformedURLException if the provided URL string is malformed
+ * @throws URISyntaxException if the provided URL string could not be parsed
as a URI reference.
+ */
+ public static String getNodeNameFromSolrUrl(String solrUrl)
+ throws MalformedURLException, URISyntaxException {
+ URL url = new URI(solrUrl).toURL();
+ return url.getAuthority() + url.getPath().replace('/', '_');
+ }
Review Comment:
Perfect. (never mind my redundant code review comment)
##########
solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java:
##########
@@ -530,14 +530,14 @@ public JettySolrRunner stopJettySolrRunner(int index)
throws Exception {
}
/**
- * Add a previously stopped node back to the cluster
+ * Add a previously stopped node back to the cluster and reuse its port
*
* @param jetty a {@link JettySolrRunner} previously returned by {@link
#stopJettySolrRunner(int)}
* @return the started node
* @throws Exception on error
*/
public JettySolrRunner startJettySolrRunner(JettySolrRunner jetty) throws
Exception {
- jetty.start(false);
+ jetty.start();
Review Comment:
That makes sense... I'll call it out in the commit log. I wonder if there's
any down-side to this.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -51,6 +55,7 @@ public abstract class BaseHttpClusterStateProvider implements
ClusterStateProvid
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private String urlScheme;
+ private Set<String> backupNodes;
Review Comment:
Also maybe leave as a list as it was provided. Why not use them in the
order provided.
##########
solr/solrj/src/java/org/apache/solr/common/util/Utils.java:
##########
@@ -786,6 +789,20 @@ public static String getBaseUrlForNodeName(
return urlScheme + "://" + hostAndPort + "/" + (isV2 ? "api" : "solr");
}
+ /**
+ * Construct base Solr URL to a Solr node name
+ *
+ * @param solrUrl Given a base Solr URL string (e.g.,
'https://app-node-1:8983/solr')
+ * @return URL that looks like {@code app-node-1:8983_solr}
+ * @throws MalformedURLException if the provided URL string is malformed
+ * @throws URISyntaxException if the provided URL string could not be parsed
as a URI reference.
+ */
Review Comment:
fantastic javadocs! I wish someone documented the existing reverse
direction method
--
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]
