Skip to content

Commit

Permalink
SOLR-17066: Switch HttpSolrClient away from coreURLs, pt 2 (#2231)
Browse files Browse the repository at this point in the history
Providing a core URL as a SolrClient's "base URL" prevents it from
communicating with other cores or making core-agnostic API requests
(e.g. node healthcheck, list cores, etc.)

This commit migrates usage in solr-core tests, solrj, solrj-streaming,
hadoop-auth, and hdfs to use the `withDefaultCollection` builder
method in lieu of using core-based URLs directly.
  • Loading branch information
gerlowskija authored Feb 1, 2024
1 parent 61cf8f8 commit 5c64979
Show file tree
Hide file tree
Showing 26 changed files with 200 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ private SolrClient buildClient(CloseableHttpClient httpClient, URL url) {
.withThreadCount(1)
.build();
case 1:
return new HttpSolrClient.Builder(url.toString() + "/" + COLLECTION)
return new HttpSolrClient.Builder(url.toString())
.withDefaultCollection(COLLECTION)
.withHttpClient(httpClient)
.build();
case 2:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ public void testLeaderFailsOver() throws Exception {
// Check that we can continue indexing after this
updateResponse = new UpdateRequest().add("id", "2").commit(cluster.getSolrClient(), collection);
assertEquals(0, updateResponse.getStatus());
try (SolrClient followerClient = new HttpSolrClient.Builder(oldLeader.getCoreUrl()).build()) {
try (SolrClient followerClient =
new HttpSolrClient.Builder(oldLeader.getBaseUrl())
.withDefaultCollection(oldLeader.getCoreName())
.build()) {
QueryResponse queryResponse = new QueryRequest(new SolrQuery("*:*")).process(followerClient);
assertEquals(
queryResponse.getResults().toString(), 2, queryResponse.getResults().getNumFound());
Expand All @@ -128,9 +131,9 @@ private Replica corruptLeader(String collection) throws IOException, SolrServerE
Replica oldLeader = dc.getLeader("shard1");
log.info("Will crash leader : {}", oldLeader);

try (SolrClient solrClient =
new HttpSolrClient.Builder(dc.getLeader("shard1").getCoreUrl()).build()) {
new UpdateRequest().add("id", "99").commit(solrClient, null);
final Replica leaderReplica = dc.getLeader("shard1");
try (SolrClient solrClient = new HttpSolrClient.Builder(leaderReplica.getBaseUrl()).build()) {
new UpdateRequest().add("id", "99").commit(solrClient, leaderReplica.getCoreName());
fail("Should have injected tragedy");
} catch (RemoteSolrException e) {
// solrClient.add would throw RemoteSolrException with code 500
Expand Down
20 changes: 13 additions & 7 deletions solr/core/src/test/org/apache/solr/cloud/RouteFieldTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.URLUtil;
import org.junit.BeforeClass;
import org.junit.Test;
import org.slf4j.Logger;
Expand Down Expand Up @@ -154,13 +155,8 @@ private void compareShardDocs(String urlId, String urlRoute)
params.add(CommonParams.SORT, "sorter asc");
params.add(CommonParams.ROWS, "1000");

SolrClient solrClient = new HttpSolrClient.Builder(urlId).build();
SolrDocumentList docsId = (SolrDocumentList) solrClient.request(request).get("response");
solrClient.close();

solrClient = new HttpSolrClient.Builder(urlRoute).build();
SolrDocumentList docsRoute = (SolrDocumentList) solrClient.request(request).get("response");
solrClient.close();
final var docsId = getDocsMatching(urlId, request);
final var docsRoute = getDocsMatching(urlRoute, request);

assertEquals(
"We should have the exact same number of docs on each shard",
Expand All @@ -175,4 +171,14 @@ private void compareShardDocs(String urlId, String urlRoute)
idRoute - 1_500_000);
}
}

private SolrDocumentList getDocsMatching(String coreUrl, QueryRequest request)
throws IOException, SolrServerException {
final var baseUrl = URLUtil.extractBaseUrl(coreUrl);
final var coreName = URLUtil.extractCoreFromCoreUrl(coreUrl);
try (SolrClient solrClient =
new HttpSolrClient.Builder(baseUrl).withDefaultCollection(coreName).build()) {
return (SolrDocumentList) solrClient.request(request).get("response");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ private static int assertConsistentReplicas(CloudSolrClient cloudClient, Slice s
int count = 0;
for (Replica replica : shard.getReplicas()) {
SolrClient client =
new Builder(replica.getCoreUrl())
new Builder(replica.getBaseUrl())
.withDefaultCollection(replica.getCoreName())
.withHttpClient(((CloudLegacySolrClient) cloudClient).getHttpClient())
.build();
QueryResponse response = client.query(new SolrQuery("q", "*:*", "distrib", "false"));
Expand Down
15 changes: 12 additions & 3 deletions solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java
Original file line number Diff line number Diff line change
Expand Up @@ -464,15 +464,21 @@ public void testRealTimeGet()
List<String> ids = new ArrayList<>(slice.getReplicas().size());
for (Replica rAdd : slice.getReplicas()) {
try (SolrClient client =
new HttpSolrClient.Builder(rAdd.getCoreUrl()).withHttpClient(httpClient).build()) {
new HttpSolrClient.Builder(rAdd.getBaseUrl())
.withDefaultCollection(rAdd.getCoreName())
.withHttpClient(httpClient)
.build()) {
client.add(new SolrInputDocument("id", String.valueOf(id), "foo_s", "bar"));
}
SolrDocument docCloudClient =
cluster.getSolrClient().getById(collectionName, String.valueOf(id));
assertEquals("bar", docCloudClient.getFieldValue("foo_s"));
for (Replica rGet : slice.getReplicas()) {
try (SolrClient client =
new HttpSolrClient.Builder(rGet.getCoreUrl()).withHttpClient(httpClient).build()) {
new HttpSolrClient.Builder(rGet.getBaseUrl())
.withDefaultCollection(rGet.getCoreName())
.withHttpClient(httpClient)
.build()) {
SolrDocument doc = client.getById(String.valueOf(id));
assertEquals("bar", doc.getFieldValue("foo_s"));
}
Expand All @@ -483,7 +489,10 @@ public void testRealTimeGet()
SolrDocumentList previousAllIdsResult = null;
for (Replica rAdd : slice.getReplicas()) {
try (SolrClient client =
new HttpSolrClient.Builder(rAdd.getCoreUrl()).withHttpClient(httpClient).build()) {
new HttpSolrClient.Builder(rAdd.getBaseUrl())
.withDefaultCollection(rAdd.getCoreName())
.withHttpClient(httpClient)
.build()) {
SolrDocumentList allIdsResult = client.getById(ids);
if (previousAllIdsResult != null) {
assertTrue(compareSolrDocumentList(previousAllIdsResult, allIdsResult));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ public String getWriterType() {
/** Helper to convert from wt string parameter to actual SolrClient. */
private static SolrClient getSolrClient(final String jettyBaseUrl, final String wt) {
HttpSolrClient.Builder builder =
new HttpSolrClient.Builder(jettyBaseUrl + "/" + COLLECTION_NAME + "/");
new HttpSolrClient.Builder(jettyBaseUrl).withDefaultCollection(COLLECTION_NAME);
switch (wt) {
case "xml":
builder.withResponseParser(RAW_XML_RESPONSE_PARSER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ private void testRequestTracking() throws Exception {
String baseUrl = replicas.iterator().next().getBaseUrl();
if (!baseUrl.endsWith("/")) baseUrl += "/";
try (SolrClient client =
new HttpSolrClient.Builder(baseUrl + "a1x2")
new HttpSolrClient.Builder(baseUrl)
.withDefaultCollection("a1x2")
.withConnectionTimeout(2000, TimeUnit.MILLISECONDS)
.withSocketTimeout(5000, TimeUnit.MILLISECONDS)
.build()) {
Expand Down Expand Up @@ -221,11 +222,10 @@ private void testQueryAgainstDownReplica() throws Exception {
// Query against the node which hosts the down replica

String baseUrl = notLeader.getBaseUrl();
if (!baseUrl.endsWith("/")) baseUrl += "/";
String path = baseUrl + "football";
log.info("Firing queries against path={}", path);
log.info("Firing queries against path={} and collection=football", baseUrl);
try (SolrClient client =
new HttpSolrClient.Builder(path)
new HttpSolrClient.Builder(baseUrl)
.withDefaultCollection("football")
.withConnectionTimeout(2000, TimeUnit.MILLISECONDS)
.withSocketTimeout(5000, TimeUnit.MILLISECONDS)
.build()) {
Expand Down
15 changes: 12 additions & 3 deletions solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,10 @@ public void testRealTimeGet()
List<String> ids = new ArrayList<>(slice.getReplicas().size());
for (Replica rAdd : slice.getReplicas()) {
try (SolrClient client =
new HttpSolrClient.Builder(rAdd.getCoreUrl()).withHttpClient(httpClient).build()) {
new HttpSolrClient.Builder(rAdd.getBaseUrl())
.withDefaultCollection(rAdd.getCoreName())
.withHttpClient(httpClient)
.build()) {
client.add(new SolrInputDocument("id", String.valueOf(id), "foo_s", "bar"));
}
SolrDocument docCloudClient =
Expand All @@ -415,7 +418,10 @@ public void testRealTimeGet()
assertEquals("bar", docCloudClient.getFieldValue("foo_s"));
for (Replica rGet : slice.getReplicas()) {
try (SolrClient client =
new HttpSolrClient.Builder(rGet.getCoreUrl()).withHttpClient(httpClient).build()) {
new HttpSolrClient.Builder(rGet.getBaseUrl())
.withDefaultCollection(rGet.getCoreName())
.withHttpClient(httpClient)
.build()) {
SolrDocument doc = client.getById(String.valueOf(id));
assertEquals("bar", doc.getFieldValue("foo_s"));
}
Expand All @@ -426,7 +432,10 @@ public void testRealTimeGet()
SolrDocumentList previousAllIdsResult = null;
for (Replica rAdd : slice.getReplicas()) {
try (SolrClient client =
new HttpSolrClient.Builder(rAdd.getCoreUrl()).withHttpClient(httpClient).build()) {
new HttpSolrClient.Builder(rAdd.getBaseUrl())
.withDefaultCollection(rAdd.getCoreName())
.withHttpClient(httpClient)
.build()) {
SolrDocumentList allIdsResult = client.getById(ids);
if (previousAllIdsResult != null) {
assertTrue(compareSolrDocumentList(previousAllIdsResult, allIdsResult));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ private int assertConsistentReplicas(Slice shard) throws SolrServerException, IO
int count = 0;
for (Replica replica : shard.getReplicas()) {
var client =
new HttpSolrClient.Builder(replica.getCoreUrl())
new HttpSolrClient.Builder(replica.getBaseUrl())
.withDefaultCollection(replica.getCoreName())
.withHttpClient(((CloudLegacySolrClient) cloudClient).getHttpClient())
.build();
QueryResponse response = client.query(new SolrQuery("q", "*:*", "distrib", "false"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,20 @@ public static JettySolrRunner createAndStartJetty(SolrInstance instance) throws
return jetty;
}

/**
* @param baseUrl the root URL for a Solr node
*/
public static SolrClient createNewSolrClient(String baseUrl) {
return createNewSolrClient(baseUrl, null);
}

/**
* @param baseUrl the root URL for a Solr node
* @param collectionOrCore an optional default collection/core for the created client
*/
public static SolrClient createNewSolrClient(String baseUrl, String collectionOrCore) {
return new HttpSolrClient.Builder(baseUrl)
.withDefaultCollection(collectionOrCore)
.withConnectionTimeout(15000, TimeUnit.MILLISECONDS)
.withSocketTimeout(90000, TimeUnit.MILLISECONDS)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void setUp() throws Exception {
leaderJetty = ReplicationTestHelper.createAndStartJetty(leader);
leaderClient =
ReplicationTestHelper.createNewSolrClient(
buildUrl(leaderJetty.getLocalPort()) + "/" + DEFAULT_TEST_CORENAME);
buildUrl(leaderJetty.getLocalPort()), DEFAULT_TEST_CORENAME);
leaderClientHealthCheck =
ReplicationTestHelper.createNewSolrClient(buildUrl(leaderJetty.getLocalPort()));

Expand All @@ -71,7 +71,7 @@ public void setUp() throws Exception {
followerJetty = createAndStartJetty(follower);
followerClient =
ReplicationTestHelper.createNewSolrClient(
buildUrl(followerJetty.getLocalPort()) + "/" + DEFAULT_TEST_CORENAME);
buildUrl(followerJetty.getLocalPort()), DEFAULT_TEST_CORENAME);
followerClientHealthCheck =
ReplicationTestHelper.createNewSolrClient(buildUrl(followerJetty.getLocalPort()));

Expand Down
Loading

0 comments on commit 5c64979

Please sign in to comment.