SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2

solr/CHANGES.txt

@@ -97,6 +97,9 @@ Improvements
 
 * SOLR-17145: The INSTALLSHARDDATA API now includes a 'requestid' field when run asynchronously (Jason Gerlowski)
 
+* SOLR-17159: bin/solr post now has proper unit testing.  Users can specify a --dry-run option to 
+  simulate posting documents without sending them to Solr. (Eric Pugh)
+
 * SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2
 
 Optimizations

solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java

@@ -47,9 +47,7 @@
 import org.apache.solr.common.cloud.ZooKeeperException;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.UpdateParams;
-import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.SolrNamedThreadFactory;
 import org.apache.solr.common.util.URLUtil;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.CoreDescriptor;
@@ -178,7 +176,6 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) {
     this.recoveringAfterStartup = recoveringAfterStartup;
   }
 
-  /** Builds a new HttpSolrClient for use in recovery. Caller must close */
   private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) {
     // workaround for SOLR-13605: get the configured timeouts & set them directly
     // (even though getRecoveryOnlyHttpClient() already has them set)
@@ -915,15 +912,11 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa
     int readTimeout =
         conflictWaitMs
             + Integer.parseInt(System.getProperty("prepRecoveryReadTimeoutExtraWait", "8000"));
-    var recoveryExec =
-        ExecutorUtil.newMDCAwareFixedThreadPool(
-            1, new SolrNamedThreadFactory("sendPrepRecoveryCmd"));
     try (Http2SolrClient client =
         recoverySolrClientBuilder(
                 leaderBaseUrl,
                 null) // leader core omitted since client only used for 'admin' request
             .withIdleTimeout(readTimeout, TimeUnit.MILLISECONDS)
-            .withExecutor(recoveryExec)
             .build()) {
       log.info("Sending prep recovery command to [{}]; [{}]", leaderBaseUrl, prepCmd);
       MDC.put("HttpSolrClient.url", baseUrl);
@@ -935,7 +928,7 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa
                 @Override
                 public void onSuccess(NamedList<Object> entries) {
                   log.info(
-                      "Prep recovery command successfully send to [{}]; [{}]",
+                      "Prep recovery command successfully sent to [{}]; [{}]",
                       leaderBaseUrl,
                       prepCmd);
                 }
@@ -946,7 +939,6 @@ public void onFailure(Throwable throwable) {
                 }
               });
     } finally {
-      recoveryExec.shutdown();
       MDC.remove("HttpSolrClient.url");
     }
   }

solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java

@@ -75,7 +75,6 @@ public class UpdateShardHandler implements SolrInfoBean {
 
   private final Http2SolrClient updateOnlyClient;
 
-  // private final CloseableHttpClient recoveryOnlyClient;
   private final Http2SolrClient recoveryOnlyClient;
 
   private final CloseableHttpClient defaultClient;
@@ -86,9 +85,7 @@ public class UpdateShardHandler implements SolrInfoBean {
 
   private final InstrumentedHttpRequestExecutor httpRequestExecutor;
 
-  private final InstrumentedHttpListenerFactory updateHttpListenerFactory;
-
-  private final InstrumentedHttpListenerFactory recoverHttpListenerFactory;
+  private final InstrumentedHttpListenerFactory trackHttpSolrMetrics;
 
   private SolrMetricsContext solrMetricsContext;
 
@@ -123,8 +120,7 @@ public UpdateShardHandler(UpdateShardHandlerConfig cfg) {
     log.debug("Created default UpdateShardHandler HTTP client with params: {}", clientParams);
 
     httpRequestExecutor = new InstrumentedHttpRequestExecutor(getMetricNameStrategy(cfg));
-    updateHttpListenerFactory = new InstrumentedHttpListenerFactory(getNameStrategy(cfg));
-    recoverHttpListenerFactory = new InstrumentedHttpListenerFactory(getNameStrategy(cfg));
+    trackHttpSolrMetrics = new InstrumentedHttpListenerFactory(getNameStrategy(cfg));
 
     defaultClient =
         HttpClientUtil.createClient(
@@ -149,10 +145,10 @@ public UpdateShardHandler(UpdateShardHandlerConfig cfg) {
 
     updateOnlyClientBuilder.withTheseParamNamesInTheUrl(urlParamNames);
     updateOnlyClient = updateOnlyClientBuilder.build();
-    updateOnlyClient.addListenerFactory(updateHttpListenerFactory);
+    updateOnlyClient.addListenerFactory(trackHttpSolrMetrics);
 
     recoveryOnlyClient = recoveryOnlyClientBuilder.build();
-    recoveryOnlyClient.addListenerFactory(recoverHttpListenerFactory);
+    recoveryOnlyClient.addListenerFactory(trackHttpSolrMetrics);
 
     ThreadFactory recoveryThreadFactory = new SolrNamedThreadFactory("recoveryExecutor");
     if (cfg != null && cfg.getMaxRecoveryThreads() > 0) {
@@ -216,7 +212,7 @@ public String getName() {
   public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
     solrMetricsContext = parentContext.getChildContext(this);
     String expandedScope = SolrMetricManager.mkName(scope, getCategory().name());
-    updateHttpListenerFactory.initializeMetrics(solrMetricsContext, expandedScope);
+    trackHttpSolrMetrics.initializeMetrics(solrMetricsContext, expandedScope);
     defaultConnectionManager.initializeMetrics(solrMetricsContext, expandedScope);
     updateExecutor =
         MetricUtils.instrumentedExecutorService(

solr/core/src/test/org/apache/solr/cloud/RecoveryStrategyStressTest.java

@@ -16,6 +16,7 @@
  */
 package org.apache.solr.cloud;
 
+import com.carrotsearch.randomizedtesting.annotations.Nightly;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
@@ -36,6 +37,7 @@
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+@Nightly
 public class RecoveryStrategyStressTest extends SolrCloudTestCase {
 
   @BeforeClass
@@ -50,12 +52,10 @@ public void stressTestRecovery() throws Exception {
         .process(cluster.getSolrClient());
     waitForState(
         "Expected a collection with one shard and two replicas", collection, clusterShape(1, 4));
-
-    SolrClient solrClient =
-        cluster.basicSolrClientBuilder().withDefaultCollection(collection).build();
     final var scheduledExecutorService =
         Executors.newScheduledThreadPool(1, new SolrNamedThreadFactory("stressTestRecovery"));
-    try (solrClient) {
+    try (SolrClient solrClient =
+        cluster.basicSolrClientBuilder().withDefaultCollection(collection).build()) {
       final StoppableIndexingThread indexThread =
           new StoppableIndexingThread(null, solrClient, "1", true, 10, 1, true);
 

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

@@ -176,6 +176,16 @@ protected Http2SolrClient(String serverBaseUrl, Builder builder) {
 
     this.idleTimeoutMillis = builder.idleTimeoutMillis;
 
+    executor = builder.executor;
+    if (executor == null) {
+      BlockingArrayQueue<Runnable> queue = new BlockingArrayQueue<>(256, 256);
+      this.executor =
+          new ExecutorUtil.MDCAwareThreadPoolExecutor(
+              32, 256, 60, TimeUnit.SECONDS, queue, new SolrNamedThreadFactory("h2sc"));
+      shutdownExecutor = true;
+    } else {
+      shutdownExecutor = false;
+    }
     if (builder.httpClient != null) {
       this.httpClient = builder.httpClient;
       this.closeClient = false;
@@ -226,17 +236,6 @@ ProtocolHandlers getProtocolHandlers() {
   private HttpClient createHttpClient(Builder builder) {
     HttpClient httpClient;
 
-    executor = builder.executor;
-    if (executor == null) {
-      BlockingArrayQueue<Runnable> queue = new BlockingArrayQueue<>(256, 256);
-      this.executor =
-          new ExecutorUtil.MDCAwareThreadPoolExecutor(
-              32, 256, 60, TimeUnit.SECONDS, queue, new SolrNamedThreadFactory("h2sc"));
-      shutdownExecutor = true;
-    } else {
-      shutdownExecutor = false;
-    }
-
     SslContextFactory.Client sslContextFactory;
     if (builder.sslConfig == null) {
       sslContextFactory = getDefaultSslContextFactory();
@@ -1162,6 +1161,10 @@ private static CookieStore getDefaultCookieStore() {
      */
     public Builder withHttpClient(Http2SolrClient http2SolrClient) {
       this.httpClient = http2SolrClient.httpClient;
+
+      if (this.executor == null) {
+        this.executor = http2SolrClient.executor;
+      }
       if (this.basicAuthAuthorizationStr == null) {
         this.basicAuthAuthorizationStr = http2SolrClient.basicAuthAuthorizationStr;
       }