SOLR-17945: fix flaky test CloudHttp2SolrClientTest#testHttpCspPerf

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

@@ -56,6 +56,8 @@
 public abstract class BaseHttpClusterStateProvider implements ClusterStateProvider {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
+  protected static final String SYS_PROP_CACHE_TIMEOUT_SECONDS = "solr.solrj.cache.timeout.sec";
+
   private String urlScheme;
   private List<URL> configuredNodes;
   volatile Set<String> liveNodes; // initially null then never null
@@ -65,7 +67,7 @@ public abstract class BaseHttpClusterStateProvider implements ClusterStateProvid
   long aliasesTimestamp = 0;
 
   // the liveNodes and aliases cache will be invalidated after 5 secs
-  private int cacheTimeout = EnvUtils.getPropertyAsInteger("solr.solrj.cache.timeout.sec", 5);
+  private int cacheTimeout = EnvUtils.getPropertyAsInteger(SYS_PROP_CACHE_TIMEOUT_SECONDS, 5);
 
   volatile boolean liveNodeReloadingScheduled = false;
   private final ScheduledExecutorService liveNodeReloadingService =

solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java

@@ -16,6 +16,7 @@
  */
 package org.apache.solr.client.solrj.impl;
 
+import static org.apache.solr.client.solrj.impl.BaseHttpClusterStateProvider.SYS_PROP_CACHE_TIMEOUT_SECONDS;
 import static org.apache.solr.client.solrj.impl.CloudSolrClient.RouteResponse;
 
 import java.io.IOException;
@@ -256,47 +257,58 @@ public void testAliasHandling() throws Exception {
   @Test
   @LogLevel("org.apache.solr.servlet.HttpSolrCall=DEBUG")
   public void testHttpCspPerf() throws Exception {
+    // This ensures CH2SC is caching cluster status by counting the number of logged calls to the
+    // admin endpoint. Too many calls to CLUSTERSTATUS might mean insufficient caching and
+    // peformance regressions!
+    try {
+      // BaseHttpClusterStateProvider has a background job that pre-fetches data from CLUSTERSTATUS
+      // on timed intervals.  This can pollute this test, so we set the interval very high to
+      // prevent it from running.
+      System.setProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS, "" + Integer.MAX_VALUE);
 
-    String collectionName = "HTTPCSPTEST";
-    CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1)
-        .process(cluster.getSolrClient());
-    cluster.waitForActiveCollection(collectionName, 2, 2);
+      String collectionName = "HTTPCSPTEST";
+      CollectionAdminRequest.createCollection(collectionName, "conf", 2, 1)
+          .process(cluster.getSolrClient());
+      cluster.waitForActiveCollection(collectionName, 2, 2);
 
-    try (LogListener adminLogs = LogListener.info(HttpSolrCall.class).substring("[admin]");
-        CloudSolrClient solrClient = createHttpCSPBasedCloudSolrClient(); ) {
-      solrClient.getClusterStateProvider().getLiveNodes(); // talks to Solr
+      try (LogListener adminLogs = LogListener.info(HttpSolrCall.class).substring("[admin]");
+          CloudSolrClient solrClient = createHttpCSPBasedCloudSolrClient(); ) {
+        solrClient.getClusterStateProvider().getLiveNodes(); // talks to Solr
 
-      assertEquals(1, adminLogs.getCount());
-      assertTrue(
-          adminLogs
-              .pollMessage()
-              .contains(
-                  "path=/admin/collections params={prs=true&liveNodes=true&action"
-                      + "=CLUSTERSTATUS&includeAll=false"));
-
-      SolrInputDocument doc = new SolrInputDocument("id", "1", "title_s", "my doc");
-      solrClient.add(collectionName, doc);
-
-      // getCount seems to return a cumulative count, but add() results in only 1 additional admin
-      // request to fetch CLUSTERSTATUS for the collection
-      assertEquals(2, adminLogs.getCount());
-      assertTrue(
-          adminLogs
-              .pollMessage()
-              .contains(
-                  "path=/admin/collections "
-                      + "params={prs=true&action=CLUSTERSTATUS&includeAll=false"));
+        assertEquals(1, adminLogs.getCount());
+        assertTrue(
+            adminLogs
+                .pollMessage()
+                .contains(
+                    "path=/admin/collections params={prs=true&liveNodes=true&action"
+                        + "=CLUSTERSTATUS&includeAll=false"));
 
-      solrClient.commit(collectionName);
-      // No additional admin requests sent
-      assertEquals(2, adminLogs.getCount());
+        SolrInputDocument doc = new SolrInputDocument("id", "1", "title_s", "my doc");
+        solrClient.add(collectionName, doc);
 
-      for (int i = 0; i < 3; i++) {
-        assertEquals(
-            1, solrClient.query(collectionName, params("q", "*:*")).getResults().getNumFound());
+        // getCount seems to return a cumulative count, but add() results in only 1 additional admin
+        // request to fetch CLUSTERSTATUS for the collection
+        assertEquals(2, adminLogs.getCount());
+        assertTrue(
+            adminLogs
+                .pollMessage()
+                .contains(
+                    "path=/admin/collections "
+                        + "params={prs=true&action=CLUSTERSTATUS&includeAll=false"));
+
+        solrClient.commit(collectionName);
         // No additional admin requests sent
         assertEquals(2, adminLogs.getCount());
+
+        for (int i = 0; i < 3; i++) {
+          assertEquals(
+              1, solrClient.query(collectionName, params("q", "*:*")).getResults().getNumFound());
+          // No additional admin requests sent
+          assertEquals(2, adminLogs.getCount());
+        }
       }
+    } finally {
+      System.clearProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS);
     }
   }
 

solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java

@@ -17,6 +17,7 @@
 
 package org.apache.solr.client.solrj.impl;
 
+import static org.apache.solr.client.solrj.impl.BaseHttpClusterStateProvider.SYS_PROP_CACHE_TIMEOUT_SECONDS;
 import static org.apache.solr.common.util.URLUtil.getNodeNameForBaseUrl;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
@@ -44,6 +45,7 @@
 import org.eclipse.jetty.http.HttpHeader;
 import org.hamcrest.Matchers;
 import org.junit.After;
+import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -84,7 +86,12 @@ public static void setupCluster() throws Exception {
                 .resolve("conf"))
         .configure();
     cluster.waitForAllNodes(30);
-    System.setProperty("solr.solrj.cache.timeout.sec", "1");
+    System.setProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS, "1");
+  }
+
+  @AfterClass
+  public static void clearSystemPrperty() {
+    System.clearProperty(SYS_PROP_CACHE_TIMEOUT_SECONDS);
   }
 
   @After