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-17541: LBSolrClient implementations should agree on 'getClient()' semantics #2899

Merged
merged 35 commits into from
Dec 23, 2024
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5261cd3
Initial development
jdyer1 Dec 4, 2024
bc8c888
fix bugs
jdyer1 Dec 5, 2024
a249169
javadoc modifications
jdyer1 Dec 5, 2024
1f8a307
tidy / remove nocommit
jdyer1 Dec 5, 2024
90bf9e2
remove obsolete test
jdyer1 Dec 6, 2024
51676f5
Fix bug: (1) Only generate a client per base url, not base url+core.…
jdyer1 Dec 6, 2024
db76064
tidy
jdyer1 Dec 6, 2024
605bdc7
Only hold the internal builder
jdyer1 Dec 6, 2024
508af0d
remove override base url from jdk client
jdyer1 Dec 6, 2024
39177b4
fix test
jdyer1 Dec 9, 2024
35651b8
Merge branch 'main' into feature/SOLR-17541
jdyer1 Dec 9, 2024
19577dc
add missing @Override
jdyer1 Dec 9, 2024
ca59536
implement code review suggestions
jdyer1 Dec 10, 2024
67c8b8f
fix getClient/buildClient
dsmiley Dec 12, 2024
7b90862
more
dsmiley Dec 12, 2024
1539d97
PKI Authentication Plugin to add the listener factory to subsequently…
jdyer1 Dec 13, 2024
8f17279
@lucene.experimental annotations
jdyer1 Dec 13, 2024
effedb9
tidy
jdyer1 Dec 13, 2024
29a3445
remove system.out.println
jdyer1 Dec 13, 2024
50470cd
httpClientBuilder > httpSolrClientBuilder
jdyer1 Dec 16, 2024
f423bd2
API change to Http2SolrClient.Builder:
jdyer1 Dec 16, 2024
6176a99
Remove unnecessary null check
jdyer1 Dec 16, 2024
783f815
tidy
jdyer1 Dec 16, 2024
e8c998d
small change to doc comment
jdyer1 Dec 16, 2024
e035a0c
fix typo
jdyer1 Dec 17, 2024
99282aa
Better variable names
jdyer1 Dec 17, 2024
45b9e10
remove silly duplicate method
jdyer1 Dec 18, 2024
7df3c3b
variable rename
jdyer1 Dec 19, 2024
90bd2ec
Merge branch 'main' into feature/SOLR-17541
jdyer1 Dec 19, 2024
f984d12
TODO on HttpClientBuilderPlugin
jdyer1 Dec 19, 2024
28aec3b
CHANGES.txt
jdyer1 Dec 19, 2024
30dcdb3
tidy
jdyer1 Dec 19, 2024
730aad0
CHANGES.txt deprecation notice for Solr 9.9
jdyer1 Dec 19, 2024
071fb04
CHANGES.txt: mention rename LBHttp2SolrClient.Builder#withListenerFac…
jdyer1 Dec 19, 2024
05eb1ea
Merge branch 'main' into feature/SOLR-17541
jdyer1 Dec 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Better variable names
  • Loading branch information
jdyer1 committed Dec 17, 2024
commit 99282aa62e69bd9bb11c9c1b2fa8de754f8427d3
4 changes: 2 additions & 2 deletions solr/core/src/java/org/apache/solr/cloud/ZkController.java
Original file line number Diff line number Diff line change
@@ -868,14 +868,14 @@ public SolrCloudManager getSolrCloudManager() {
if (cloudManager != null) {
return cloudManager;
}
var httpClientBuilder =
var httpSolrClientBuilder =
new Http2SolrClient.Builder()
.withHttpClient(cc.getDefaultHttpSolrClient())
.withIdleTimeout(30000, TimeUnit.MILLISECONDS)
.withConnectionTimeout(15000, TimeUnit.MILLISECONDS);
cloudSolrClient =
new CloudHttp2SolrClient.Builder(new ZkClientClusterStateProvider(zkStateReader))
.withInternalClientBuilder(httpClientBuilder)
.withInternalClientBuilder(httpSolrClientBuilder)
.build();
cloudManager = new SolrClientCloudManager(cloudSolrClient, cc.getObjectCache());
cloudManager.getClusterStateProvider().connect();
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT @iamsanjay ?

Original file line number Diff line number Diff line change
@@ -36,23 +36,24 @@ final class HttpSolrClientProvider implements AutoCloseable {

private final Http2SolrClient httpSolrClient;

private final Http2SolrClient.Builder httpClientBuilder;
private final Http2SolrClient.Builder httpSolrClientBuilder;

private final InstrumentedHttpListenerFactory trackHttpSolrMetrics;

HttpSolrClientProvider(UpdateShardHandlerConfig cfg, SolrMetricsContext parentContext) {
trackHttpSolrMetrics = new InstrumentedHttpListenerFactory(getNameStrategy(cfg));
initializeMetrics(parentContext);

this.httpClientBuilder = new Http2SolrClient.Builder().addListenerFactory(trackHttpSolrMetrics);
this.httpSolrClientBuilder =
new Http2SolrClient.Builder().addListenerFactory(trackHttpSolrMetrics);

if (cfg != null) {
httpClientBuilder
httpSolrClientBuilder
.withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS)
.withIdleTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS)
.withMaxConnectionsPerHost(cfg.getMaxUpdateConnectionsPerHost());
}
httpSolrClient = httpClientBuilder.build();
httpSolrClient = httpSolrClientBuilder.build();
}

private InstrumentedHttpListenerFactory.NameStrategy getNameStrategy(
@@ -76,7 +77,7 @@ Http2SolrClient getSolrClient() {
}

void setSecurityBuilder(HttpClientBuilderPlugin builder) {
builder.setup(httpClientBuilder, httpSolrClient);
builder.setup(httpSolrClientBuilder, httpSolrClient);
}

@Override
Original file line number Diff line number Diff line change
@@ -53,8 +53,8 @@ public class CloudHttp2SolrClient extends CloudSolrClient {
*/
protected CloudHttp2SolrClient(Builder builder) {
super(builder.shardLeadersOnly, builder.parallelUpdates, builder.directUpdatesToLeadersOnly);
var httpClientBuilder = createOrGetHttpClientBuilder(builder);
this.myClient = httpClientBuilder.build();
var httpSolrClientBuilder = createOrGetHttpClientBuilder(builder);
this.myClient = httpSolrClientBuilder.build();
this.stateProvider = createClusterStateProvider(builder);
this.retryExpiryTimeNano = builder.retryExpiryTimeNano;
this.defaultCollection = builder.defaultCollection;
@@ -72,7 +72,7 @@ protected CloudHttp2SolrClient(Builder builder) {
// locks.
this.locks = objectList(builder.parallelCacheRefreshesLocks);

this.lbClient = new LBHttp2SolrClient.Builder<>(httpClientBuilder).build();
this.lbClient = new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder).build();
}

private Http2SolrClient.Builder createOrGetHttpClientBuilder(Builder builder) {
Original file line number Diff line number Diff line change
@@ -52,11 +52,11 @@ public void testLBHttp2SolrClientWithTheseParamNamesInTheUrl() {
Set<String> urlParamNames = new HashSet<>(2);
urlParamNames.add("param1");

var httpClientBuilder =
var httpSolrClientBuilder =
new Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames);
var endpoint = new LBSolrClient.Endpoint(url);
try (var testClient =
new LBHttp2SolrClient.Builder<Http2SolrClient.Builder>(httpClientBuilder, endpoint)
new LBHttp2SolrClient.Builder<Http2SolrClient.Builder>(httpSolrClientBuilder, endpoint)
.build()) {

assertArrayEquals(
@@ -76,11 +76,11 @@ public void testSynchronous() throws Exception {
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);

var httpClientBuilder =
var httpSolrClientBuilder =
new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);

try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {

String lastEndpoint = null;
for (int i = 0; i < 10; i++) {
@@ -104,11 +104,11 @@ public void testSynchronousWithFalures() throws Exception {
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);

var httpClientBuilder =
var httpSolrClientBuilder =
new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);

try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {

setEndpointToFail(testClient, ep1);
setEndpointToSucceed(testClient, ep2);
@@ -154,11 +154,11 @@ public void testAsyncWithFailures() {
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);

var httpClientBuilder =
var httpSolrClientBuilder =
new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);

try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {

for (int j = 0; j < 2; j++) {
// first time Endpoint One will return error code 500.
@@ -226,11 +226,11 @@ public void testAsync() {
LBSolrClient.Endpoint ep2 = new LBSolrClient.Endpoint("http://endpoint.two");
List<LBSolrClient.Endpoint> endpointList = List.of(ep1, ep2);

var httpClientBuilder =
var httpSolrClientBuilder =
new MockHttpSolrClientBuilder().withConnectionTimeout(10, TimeUnit.SECONDS);

try (LBHttp2SolrClient<MockHttpSolrClientBuilder> testClient =
new LBHttp2SolrClient.Builder<>(httpClientBuilder, ep1, ep2).build()) {
new LBHttp2SolrClient.Builder<>(httpSolrClientBuilder, ep1, ep2).build()) {

int limit = 10; // For simplicity use an even limit
List<CompletableFuture<LBSolrClient.Rsp>> responses = new ArrayList<>();
Loading