SOLR-10461: move setAliveCheckInterval from clients into Builder

solr/CHANGES.txt

@@ -93,6 +93,9 @@ Improvements
 * SOLR-10463: Introduce Builder setter for retryExpiryTime on cloud SolrClients.  Deprecated 
   direct setter setRetryExpiryTime on cloud SolrClients. (Eric Pugh)
 
+* SOLR-10461: Introduce Builder setter for aliveCheckInterval on load balanced SolrClients.  Deprecated 
+  direct setter setAliveCheckInterval on SolrClients. (Eric Pugh, David Smiley)
+
 Optimizations
 ---------------------
 

solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java

@@ -50,8 +50,8 @@
  * but this class may be used for updates because the server will forward them to the appropriate
  * leader.
  *
- * <p>It offers automatic failover when a server goes down and it detects when the server comes back
- * up.
+ * <p>It offers automatic failover when a server goes down, and it detects when the server comes
+ * back up.
  *
  * <p>Load balancing is done using a simple round-robin on the list of servers.
  *
@@ -69,11 +69,11 @@
  * </blockquote>
  *
  * This detects if a dead server comes alive automatically. The check is done in fixed intervals in
- * a dedicated thread. This interval can be set using {@link #setAliveCheckInterval} , the default
- * is set to one minute.
+ * a dedicated thread. This interval can be set using {@link
+ * LBHttp2SolrClient.Builder#setAliveCheckInterval(int)} , the default is set to one minute.
  *
  * <p><b>When to use this?</b><br>
- * This can be used as a software load balancer when you do not wish to setup an external load
+ * This can be used as a software load balancer when you do not wish to set up an external load
  * balancer. Alternatives to this code are to use a dedicated hardware load balancer or using Apache
  * httpd with mod_proxy_balancer as a load balancer. See <a
  * href="http://en.wikipedia.org/wiki/Load_balancing_(computing)">Load balancing on Wikipedia</a>
@@ -301,14 +301,33 @@ public void onFailure(Throwable oe) {
   public static class Builder {
     private final Http2SolrClient http2Client;
     private final String[] baseSolrUrls;
+    private int aliveCheckInterval;
 
     public Builder(Http2SolrClient http2Client, String... baseSolrUrls) {
       this.http2Client = http2Client;
       this.baseSolrUrls = baseSolrUrls;
     }
 
+    /**
+     * LBHttpSolrServer keeps pinging the dead servers at fixed interval to find if it is alive. Use
+     * this to set that interval
+     *
+     * @param aliveCheckInterval time in milliseconds
+     */
+    public LBHttp2SolrClient.Builder setAliveCheckInterval(int aliveCheckInterval) {
+      if (aliveCheckInterval <= 0) {
+        throw new IllegalArgumentException(
+            "Alive check interval must be " + "positive, specified value = " + aliveCheckInterval);
+      }
+      this.aliveCheckInterval = aliveCheckInterval;
+      return this;
+    }
+
     public LBHttp2SolrClient build() {
-      return new LBHttp2SolrClient(this.http2Client, Arrays.asList(this.baseSolrUrls));
+      LBHttp2SolrClient solrClient =
+          new LBHttp2SolrClient(this.http2Client, Arrays.asList(this.baseSolrUrls));
+      solrClient.aliveCheckInterval = this.aliveCheckInterval;
+      return solrClient;
     }
   }
 }

solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java

@@ -57,8 +57,8 @@
  * </blockquote>
  *
  * This detects if a dead server comes alive automatically. The check is done in fixed intervals in
- * a dedicated thread. This interval can be set using {@link #setAliveCheckInterval} , the default
- * is set to one minute.
+ * a dedicated thread. This interval can be set using {@link
+ * LBHttpSolrClient.Builder#setAliveCheckInterval(int)} , the default is set to one minute.
  *
  * <p><b>When to use this?</b><br>
  * This can be used as a software load balancer when you do not wish to setup an external load
@@ -92,6 +92,7 @@ protected LBHttpSolrClient(Builder builder) {
     this.connectionTimeout = builder.connectionTimeoutMillis;
     this.soTimeout = builder.socketTimeoutMillis;
     this.parser = builder.responseParser;
+    this.aliveCheckInterval = builder.aliveCheckInterval;
     for (String baseUrl : builder.baseSolrUrls) {
       urlToClient.put(baseUrl, makeSolrClient(baseUrl));
     }
@@ -181,6 +182,7 @@ public HttpClient getHttpClient() {
   public static class Builder extends SolrClientBuilder<Builder> {
     protected final List<String> baseSolrUrls;
     protected HttpSolrClient.Builder httpSolrClientBuilder;
+    private int aliveCheckInterval;
 
     public Builder() {
       this.baseSolrUrls = new ArrayList<>();
@@ -259,6 +261,21 @@ public Builder withBaseSolrUrls(String... solrUrls) {
       return this;
     }
 
+    /**
+     * LBHttpSolrServer keeps pinging the dead servers at fixed interval to find if it is alive. Use
+     * this to set that interval
+     *
+     * @param aliveCheckInterval time in milliseconds
+     */
+    public Builder setAliveCheckInterval(int aliveCheckInterval) {
+      if (aliveCheckInterval <= 0) {
+        throw new IllegalArgumentException(
+            "Alive check interval must be " + "positive, specified value = " + aliveCheckInterval);
+      }
+      this.aliveCheckInterval = aliveCheckInterval;
+      return this;
+    }
+
     /**
      * Provides a {@link HttpSolrClient.Builder} to be used for building the internally used
      * clients.

solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java

@@ -78,7 +78,7 @@ public abstract class LBSolrClient extends SolrClient {
 
   private volatile ScheduledExecutorService aliveCheckExecutor;
 
-  private int interval = CHECK_INTERVAL;
+  protected int aliveCheckInterval = CHECK_INTERVAL;
   private final AtomicInteger counter = new AtomicInteger(-1);
 
   private static final SolrQuery solrQuery = new SolrQuery("*:*");
@@ -462,14 +462,16 @@ protected Exception addZombie(String serverStr, Exception e) {
    * LBHttpSolrServer keeps pinging the dead servers at fixed interval to find if it is alive. Use
    * this to set that interval
    *
-   * @param interval time in milliseconds
+   * @param aliveCheckInterval time in milliseconds
+   * @deprecated use {@link LBHttpSolrClient.Builder#setAliveCheckInterval(int)} instead
    */
-  public void setAliveCheckInterval(int interval) {
-    if (interval <= 0) {
+  @Deprecated
+  public void setAliveCheckInterval(int aliveCheckInterval) {
+    if (aliveCheckInterval <= 0) {
       throw new IllegalArgumentException(
-          "Alive check interval must be " + "positive, specified value = " + interval);
+          "Alive check interval must be " + "positive, specified value = " + aliveCheckInterval);
     }
-    this.interval = interval;
+    this.aliveCheckInterval = aliveCheckInterval;
   }
 
   private void startAliveCheckExecutor() {
@@ -483,8 +485,8 @@ private void startAliveCheckExecutor() {
                   new SolrNamedThreadFactory("aliveCheckExecutor"));
           aliveCheckExecutor.scheduleAtFixedRate(
               getAliveCheckRunner(new WeakReference<>(this)),
-              this.interval,
-              this.interval,
+              this.aliveCheckInterval,
+              this.aliveCheckInterval,
               TimeUnit.MILLISECONDS);
         }
       }

solr/solrj/src/test/org/apache/solr/client/solrj/TestLBHttp2SolrClient.java

@@ -118,16 +118,16 @@ public void tearDown() throws Exception {
   }
 
   public void testSimple() throws Exception {
-    String[] s = new String[solr.length];
+    String[] solrUrls = new String[solr.length];
     for (int i = 0; i < solr.length; i++) {
-      s[i] = solr[i].getUrl();
+      solrUrls[i] = solr[i].getUrl();
     }
-    try (LBHttp2SolrClient client = getLBHttp2SolrClient(httpClient, s)) {
-      client.setAliveCheckInterval(500);
+    try (LBHttp2SolrClient client =
+        new LBHttp2SolrClient.Builder(httpClient, solrUrls).setAliveCheckInterval(500).build()) {
       SolrQuery solrQuery = new SolrQuery("*:*");
       Set<String> names = new HashSet<>();
       QueryResponse resp = null;
-      for (String ignored : s) {
+      for (String ignored : solrUrls) {
         resp = client.query(solrQuery);
         assertEquals(10, resp.getResults().getNumFound());
         names.add(resp.getResults().get(0).getFieldValue("name").toString());
@@ -138,7 +138,7 @@ public void testSimple() throws Exception {
       solr[1].jetty.stop();
       solr[1].jetty = null;
       names.clear();
-      for (String ignored : s) {
+      for (String ignored : solrUrls) {
         resp = client.query(solrQuery);
         assertEquals(10, resp.getResults().getNumFound());
         names.add(resp.getResults().get(0).getFieldValue("name").toString());
@@ -151,7 +151,7 @@ public void testSimple() throws Exception {
       // Wait for the alive check to complete
       Thread.sleep(1200);
       names.clear();
-      for (String ignored : s) {
+      for (String ignored : solrUrls) {
         resp = client.query(solrQuery);
         assertEquals(10, resp.getResults().getNumFound());
         names.add(resp.getResults().get(0).getFieldValue("name").toString());
@@ -165,9 +165,12 @@ private LBHttp2SolrClient getLBHttp2SolrClient(Http2SolrClient httpClient, Strin
   }
 
   public void testTwoServers() throws Exception {
+    String[] solrUrls = new String[2];
+    for (int i = 0; i < 2; i++) {
+      solrUrls[i] = solr[i].getUrl();
+    }
     try (LBHttp2SolrClient client =
-        getLBHttp2SolrClient(httpClient, solr[0].getUrl(), solr[1].getUrl())) {
-      client.setAliveCheckInterval(500);
+        new LBHttp2SolrClient.Builder(httpClient, solrUrls).setAliveCheckInterval(500).build()) {
       SolrQuery solrQuery = new SolrQuery("*:*");
       QueryResponse resp = null;
       solr[0].jetty.stop();
@@ -195,20 +198,20 @@ public void testTwoServers() throws Exception {
   }
 
   public void testReliability() throws Exception {
-    String[] s = new String[solr.length];
+    String[] solrUrls = new String[solr.length];
     for (int i = 0; i < solr.length; i++) {
-      s[i] = solr[i].getUrl();
+      solrUrls[i] = solr[i].getUrl();
     }
 
-    try (LBHttp2SolrClient client = getLBHttp2SolrClient(httpClient, s)) {
-      client.setAliveCheckInterval(500);
+    try (LBHttp2SolrClient client =
+        new LBHttp2SolrClient.Builder(httpClient, solrUrls).setAliveCheckInterval(500).build()) {
 
       // Kill a server and test again
       solr[1].jetty.stop();
       solr[1].jetty = null;
 
       // query the servers
-      for (String value : s) client.query(new SolrQuery("*:*"));
+      for (String ignored : solrUrls) client.query(new SolrQuery("*:*"));
 
       // Start the killed server once again
       solr[1].startJetty();

solr/solrj/src/test/org/apache/solr/client/solrj/TestLBHttpSolrClient.java

@@ -119,16 +119,20 @@ public void tearDown() throws Exception {
   }
 
   public void testSimple() throws Exception {
-    String[] s = new String[solr.length];
+    String[] solrUrls = new String[solr.length];
     for (int i = 0; i < solr.length; i++) {
-      s[i] = solr[i].getUrl();
+      solrUrls[i] = solr[i].getUrl();
     }
-    try (LBHttpSolrClient client = getLBHttpSolrClient(httpClient, s)) {
-      client.setAliveCheckInterval(500);
+    try (LBHttpSolrClient client =
+        new LBHttpSolrClient.Builder()
+            .withHttpClient(httpClient)
+            .withBaseSolrUrls(solrUrls)
+            .setAliveCheckInterval(500)
+            .build()) {
       SolrQuery solrQuery = new SolrQuery("*:*");
       Set<String> names = new HashSet<>();
       QueryResponse resp = null;
-      for (String value : s) {
+      for (String value : solrUrls) {
         resp = client.query(solrQuery);
         assertEquals(10, resp.getResults().getNumFound());
         names.add(resp.getResults().get(0).getFieldValue("name").toString());
@@ -139,7 +143,7 @@ public void testSimple() throws Exception {
       solr[1].jetty.stop();
       solr[1].jetty = null;
       names.clear();
-      for (String value : s) {
+      for (String value : solrUrls) {
         resp = client.query(solrQuery);
         assertEquals(10, resp.getResults().getNumFound());
         names.add(resp.getResults().get(0).getFieldValue("name").toString());
@@ -152,7 +156,7 @@ public void testSimple() throws Exception {
       // Wait for the alive check to complete
       Thread.sleep(1200);
       names.clear();
-      for (String value : s) {
+      for (String value : solrUrls) {
         resp = client.query(solrQuery);
         assertEquals(10, resp.getResults().getNumFound());
         names.add(resp.getResults().get(0).getFieldValue("name").toString());
@@ -162,9 +166,16 @@ public void testSimple() throws Exception {
   }
 
   public void testTwoServers() throws Exception {
+    String[] solrUrls = new String[2];
+    for (int i = 0; i < 2; i++) {
+      solrUrls[i] = solr[i].getUrl();
+    }
     try (LBHttpSolrClient client =
-        getLBHttpSolrClient(httpClient, solr[0].getUrl(), solr[1].getUrl())) {
-      client.setAliveCheckInterval(500);
+        new LBHttpSolrClient.Builder()
+            .withHttpClient(httpClient)
+            .withBaseSolrUrls(solrUrls)
+            .setAliveCheckInterval(500)
+            .build()) {
       SolrQuery solrQuery = new SolrQuery("*:*");
       QueryResponse resp = null;
       solr[0].jetty.stop();
@@ -192,30 +203,31 @@ public void testTwoServers() throws Exception {
   }
 
   public void testReliability() throws Exception {
-    String[] s = new String[solr.length];
+    String[] solrUrls = new String[solr.length];
     for (int i = 0; i < solr.length; i++) {
-      s[i] = solr[i].getUrl();
+      solrUrls[i] = solr[i].getUrl();
     }
 
-    CloseableHttpClient myHttpClient = HttpClientUtil.createClient(null);
-    try {
-      try (LBHttpSolrClient client = getLBHttpSolrClient(myHttpClient, 500, 500, s)) {
-        client.setAliveCheckInterval(500);
+    try (LBHttpSolrClient client =
+        new LBHttpSolrClient.Builder()
+            .withHttpClient(httpClient)
+            .withBaseSolrUrls(solrUrls)
+            .withConnectionTimeout(500)
+            .withSocketTimeout(500)
+            .setAliveCheckInterval(500)
+            .build()) {
 
-        // Kill a server and test again
-        solr[1].jetty.stop();
-        solr[1].jetty = null;
+      // Kill a server and test again
+      solr[1].jetty.stop();
+      solr[1].jetty = null;
 
-        // query the servers
-        for (String value : s) client.query(new SolrQuery("*:*"));
+      // query the servers
+      for (String value : solrUrls) client.query(new SolrQuery("*:*"));
 
-        // Start the killed server once again
-        solr[1].startJetty();
-        // Wait for the alive check to complete
-        waitForServer(30, client, 3, solr[1].name);
-      }
-    } finally {
-      HttpClientUtil.close(myHttpClient);
+      // Start the killed server once again
+      solr[1].startJetty();
+      // Wait for the alive check to complete
+      waitForServer(30, client, 3, solr[1].name);
     }
   }