Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17066: Switch ConcurrentSolrClients away from core URLs #2254

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigh... if only a builder new what a url was and we didn't .tostring everywhere. ;-) #beatdeadhorse

.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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nicer!


@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