Skip to content

Commit

Permalink
SOLR-17066: Switch ConcurrentSolrClients away from core URLs (#2254)
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 all usage of both "concurrent update" clients
away from core URLs.
  • Loading branch information
gerlowskija committed Feb 14, 2024
1 parent 7054fda commit 6afbc20
Show file tree
Hide file tree
Showing 15 changed files with 42 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,13 @@ static class Builder extends ConcurrentUpdateHttp2SolrClient.Builder {
protected SolrCmdDistributor.Req req;
protected List<SolrError> errors;

/**
* @param baseSolrUrl the base URL of a Solr node. Should <em>not</em> contain a collection or
* core name
* @param client the client to use in making requests
* @param req the command distributor request object for this client
* @param errors a collector for any errors
*/
public Builder(
String baseSolrUrl,
Http2SolrClient client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ private SolrClient buildClient(CloseableHttpClient httpClient, URL url) {
switch (random().nextInt(3)) {
case 0:
// currently, only testing with 1 thread
return new ConcurrentUpdateSolrClient.Builder(url.toString() + "/" + COLLECTION)
return new ConcurrentUpdateSolrClient.Builder(url.toString())
.withDefaultCollection(COLLECTION)
.withHttpClient(httpClient)
.withQueueSize(6)
.withThreadCount(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -812,8 +812,8 @@ public void testConcurrentIndexing() throws Exception {
final int numDocs = atLeast(50);
final JettySolrRunner nodeToUpdate = cluster.getRandomJetty(random());
try (ConcurrentUpdateSolrClient indexClient =
new ConcurrentUpdateSolrClient.Builder(
nodeToUpdate.getProxyBaseUrl() + "/" + collectionName)
new ConcurrentUpdateSolrClient.Builder(nodeToUpdate.getProxyBaseUrl().toString())
.withDefaultCollection(collectionName)
.withQueueSize(10)
.withThreadCount(2)
.build()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ protected HttpSolrClient makeSolrClient(String server) {
.withResponseParser(parser)
.withConnectionTimeout(connectionTimeoutMillis, TimeUnit.MILLISECONDS)
.withSocketTimeout(soTimeoutMillis, TimeUnit.MILLISECONDS);

// Note that this may override the value extracted from the base URL..should we log a warning
// here?
if (defaultCollection != null) {
clientBuilder.withDefaultCollection(defaultCollection);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class SolrExampleStreamingBinaryHttp2Test extends SolrExampleStreamingHtt

@Override
public SolrClient createNewSolrClient() {
String url = getCoreUrl();
String url = getBaseUrl();
// smaller queue size hits locks more often
Http2SolrClient solrClient =
new Http2SolrClient.Builder()
Expand All @@ -46,6 +46,7 @@ public SolrClient createNewSolrClient() {
.build();
ConcurrentUpdateHttp2SolrClient concurrentClient =
new ErrorTrackingConcurrentUpdateSolrClient.Builder(url, solrClient)
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(2)
.withThreadCount(5)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public class SolrExampleStreamingBinaryTest extends SolrExampleStreamingTest {
public SolrClient createNewSolrClient() {

SolrClient client =
new ErrorTrackingConcurrentUpdateSolrClient.Builder(getCoreUrl())
new ErrorTrackingConcurrentUpdateSolrClient.Builder(getBaseUrl())
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(2)
.withThreadCount(5)
.withResponseParser(new BinaryResponseParser())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public static void beforeTest() throws Exception {

@Override
public SolrClient createNewSolrClient() {
String url = getCoreUrl();
String url = getBaseUrl();
// smaller queue size hits locks more often
Http2SolrClient solrClient =
new Http2SolrClient.Builder()
Expand All @@ -54,6 +54,7 @@ public SolrClient createNewSolrClient() {
.build();
ConcurrentUpdateHttp2SolrClient concurrentClient =
new ErrorTrackingConcurrentUpdateSolrClient.Builder(url, solrClient)
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(2)
.withThreadCount(5)
.build();
Expand All @@ -63,10 +64,11 @@ public SolrClient createNewSolrClient() {
public void testWaitOptions() throws Exception {
// SOLR-3903
final List<Throwable> failures = new ArrayList<>();
final String serverUrl = getCoreUrl();
final String serverUrl = getBaseUrl();
try (Http2SolrClient http2Client = new Http2SolrClient.Builder().build();
ConcurrentUpdateHttp2SolrClient concurrentClient =
new FailureRecordingConcurrentUpdateSolrClient.Builder(serverUrl, http2Client)
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(2)
.withThreadCount(2)
.build()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public static void beforeTest() throws Exception {
@Override
public SolrClient createNewSolrClient() {
// smaller queue size hits locks more often
return new ErrorTrackingConcurrentUpdateSolrClient.Builder(getCoreUrl())
return new ErrorTrackingConcurrentUpdateSolrClient.Builder(getBaseUrl())
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(2)
.withThreadCount(5)
.withResponseParser(new XMLResponseParser())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public void testDeleteByIdReportsInvalidIdLists() throws Exception {

try (Http2SolrClient http2Client = new Http2SolrClient.Builder().build();
SolrClient client =
new ConcurrentUpdateHttp2SolrClient.Builder(
getBaseUrl() + "/" + ANY_COLLECTION, http2Client)
new ConcurrentUpdateHttp2SolrClient.Builder(getBaseUrl(), http2Client)
.withDefaultCollection(ANY_COLLECTION)
.withQueueSize(ANY_QUEUE_SIZE)
.withThreadCount(ANY_MAX_NUM_THREADS)
.build()) {
Expand Down Expand Up @@ -78,8 +78,8 @@ public void testDeleteByIdReportsInvalidIdLists() throws Exception {

try (Http2SolrClient http2Client = new Http2SolrClient.Builder().build();
SolrClient client =
new ConcurrentUpdateHttp2SolrClient.Builder(
getBaseUrl() + "/" + ANY_COLLECTION, http2Client)
new ConcurrentUpdateHttp2SolrClient.Builder(getBaseUrl(), http2Client)
.withDefaultCollection(ANY_COLLECTION)
.withQueueSize(ANY_QUEUE_SIZE)
.withThreadCount(ANY_MAX_NUM_THREADS)
.build()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ public void testCollectionParameters() throws IOException, SolrServerException {

try (Http2SolrClient http2Client = new Http2SolrClient.Builder().build();
ConcurrentUpdateHttp2SolrClient concurrentClient =
new ConcurrentUpdateHttp2SolrClient.Builder(getCoreUrl(), http2Client)
new ConcurrentUpdateHttp2SolrClient.Builder(getBaseUrl(), http2Client)
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(cussQueueSize)
.withThreadCount(cussThreadCount)
.build()) {
Expand Down Expand Up @@ -205,7 +206,8 @@ public void testConcurrentCollectionUpdate() throws Exception {

try (Http2SolrClient http2Client = new Http2SolrClient.Builder().build();
ConcurrentUpdateHttp2SolrClient concurrentClient =
new ConcurrentUpdateHttp2SolrClient.Builder(getCoreUrl(), http2Client)
new ConcurrentUpdateHttp2SolrClient.Builder(getBaseUrl(), http2Client)
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(cussQueueSize)
.withThreadCount(cussThreadCount)
.build()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public static void beforeTest() throws Exception {
@Test
public void testDeleteByIdReportsInvalidIdLists() throws Exception {
try (SolrClient client =
new ConcurrentUpdateSolrClient.Builder(getBaseUrl() + "/" + ANY_COLLECTION)
new ConcurrentUpdateSolrClient.Builder(getBaseUrl())
.withDefaultCollection(ANY_COLLECTION)
.withQueueSize(ANY_QUEUE_SIZE)
.withThreadCount(ANY_MAX_NUM_THREADS)
.build()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
/** Unit tests for {@link Builder}. */
public class ConcurrentUpdateSolrClientBuilderTest extends SolrTestCase {

private static final String ANY_BASE_URL = "http://localhost:8983/solr";

@Test(expected = IllegalArgumentException.class)
public void testRejectsMissingBaseSolrUrl() {
new Builder(null).build();
Expand All @@ -39,7 +41,7 @@ public void testRejectsMissingBaseSolrUrl() {
@Test
@SuppressWarnings({"try"})
public void testMissingQueueSize() {
try (ConcurrentUpdateSolrClient client = new Builder("someurl").build()) {
try (ConcurrentUpdateSolrClient client = new Builder(ANY_BASE_URL).build()) {
// Do nothing as we just need to test that the only mandatory parameter for building the
// client is the baseSolrUrl
}
Expand Down Expand Up @@ -76,7 +78,7 @@ public void testSocketTimeoutOnCommit() throws IOException, SolrServerException
@Test
public void testDefaultCollectionPassedFromBuilderToClient() throws IOException {
try (SolrClient createdClient =
new ConcurrentUpdateSolrClient.Builder("someurl")
new ConcurrentUpdateSolrClient.Builder(ANY_BASE_URL)
.withDefaultCollection("aCollection")
.build()) {
assertEquals("aCollection", createdClient.getDefaultCollection());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ public void testCollectionParameters() throws IOException, SolrServerException {
}

try (ConcurrentUpdateSolrClient concurrentClient =
(new ConcurrentUpdateSolrClient.Builder(getCoreUrl()))
(new ConcurrentUpdateSolrClient.Builder(getBaseUrl()))
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(cussQueueSize)
.withThreadCount(cussThreadCount)
.build()) {
Expand Down Expand Up @@ -283,7 +284,8 @@ public void testConcurrentCollectionUpdate() throws Exception {
}

try (ConcurrentUpdateSolrClient concurrentClient =
(new ConcurrentUpdateSolrClient.Builder(getCoreUrl()))
(new ConcurrentUpdateSolrClient.Builder(getBaseUrl()))
.withDefaultCollection(DEFAULT_TEST_CORENAME)
.withQueueSize(cussQueueSize)
.withThreadCount(cussThreadCount)
.build()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class HttpSolrClientConPoolTest extends SolrJettyTestBase {
public static void beforeTest() throws Exception {
createAndStartJetty(legacyExampleCollection1SolrHome());
fooUrl = getBaseUrl();

secondJetty.startSolr(Path.of(legacyExampleCollection1SolrHome()));
barUrl = secondJetty.getBaseUrl();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public FullThrottleStoppableIndexingThread(
cusc =
new ErrorLoggingConcurrentUpdateSolrClient.Builder(
((HttpSolrClient) clients.get(0)).getBaseURL())
.withDefaultCollection(clients.get(0).getDefaultCollection())
.withHttpClient(httpClient)
.withQueueSize(8)
.withThreadCount(2)
Expand Down Expand Up @@ -127,6 +128,7 @@ private void changeUrlOnError(Exception e) {
cusc =
new ErrorLoggingConcurrentUpdateSolrClient.Builder(
((HttpSolrClient) clients.get(clientIndex)).getBaseURL())
.withDefaultCollection(clients.get(clientIndex).getDefaultCollection())
.withHttpClient(httpClient)
.withQueueSize(30)
.withThreadCount(3)
Expand Down

0 comments on commit 6afbc20

Please sign in to comment.