You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/07/14 21:57:30 UTC

[lucene-solr] 02/02: #156 - Pretty much no tests use MetricsHistoryHandler.

This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit a7b9847a7e98c27d74b13ac0a46d3e36945fbba3
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Tue Jul 14 16:57:11 2020 -0500

    #156 - Pretty much no tests use MetricsHistoryHandler.
---
 .../java/org/apache/solr/core/CoreContainer.java   |  8 ++--
 .../src/java/org/apache/solr/core/SolrCore.java    | 55 ++++++----------------
 .../solr/handler/admin/MetricsHistoryHandler.java  |  7 +--
 .../MetricsHistoryWithAuthIntegrationTest.java     |  1 +
 .../solr/metrics/SolrCoreMetricManagerTest.java    |  1 +
 .../src/java/org/apache/solr/SolrTestCase.java     |  2 +
 6 files changed, 27 insertions(+), 47 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index c9a5aa1..c9ad160 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -781,9 +781,11 @@ public class CoreContainer implements Closeable {
         metricsHandler.initializeMetrics(solrMetricsContext, METRICS_PATH);
       });
 
-      work.collect(() -> {
-        createMetricsHistoryHandler();
-      });
+      if (!Boolean.getBoolean("solr.disableMetricsHistoryHandler")) {
+        work.collect(() -> {
+          createMetricsHistoryHandler();
+        });
+      }
 
       work.collect(() -> {
         autoscalingHistoryHandler = createHandler(AUTOSCALING_HISTORY_PATH, AutoscalingHistoryHandler.class.getName(), AutoscalingHistoryHandler.class);
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
index 80edfd3..a192ab9 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -1549,11 +1549,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
     MDCLoggingContext.setCore(this);
   }
 
-  @Override
-  public void close() {
-    close(false);
-  }
-
   /**
    * Close all resources allocated by the core if it is no longer in use...
    * <ul>
@@ -1579,8 +1574,8 @@ public final class SolrCore implements SolrInfoBean, Closeable {
    *
    * @see #isClosed()
    */
-
-  public void close(boolean failedInConstructor) {
+  @Override
+  public void close() {
     int count = refCount.decrementAndGet();
     if (count > 0) return; // close is called often, and only actually closes if nothing is using it.
     if (count < 0) {
@@ -1600,9 +1595,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
         }
       }
 
-
-
-
       List<Callable<Object>> closeHookCalls = new ArrayList<>();
 
       if (closeHooks != null) {
@@ -1616,9 +1608,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
 
       assert ObjectReleaseTracker.release(searcherExecutor);
 
-    //  closer.add("PreCloseHooks", closeHookCalls);
-
-
       List<Callable<Object>> closeCalls = new ArrayList<Callable<Object>>();
       closeCalls.addAll(closeHookCalls);
       closeCalls.add(() -> {
@@ -1658,34 +1647,18 @@ public final class SolrCore implements SolrInfoBean, Closeable {
 
       AtomicBoolean coreStateClosed = new AtomicBoolean(false);
 
-      if (!failedInConstructor) {
-        closer.add("SolrCoreState", () -> {
-          boolean closed = false;
-          if (updateHandler != null && updateHandler instanceof IndexWriterCloser) {
-            closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
-          } else {
-            closed = solrCoreState.decrefSolrCoreState(null);
-          }
-          coreStateClosed.set(closed);
-          return solrCoreState;
-        });
-      }
+      closer.add("SolrCoreState", () -> {
+        boolean closed = false;
+        if (updateHandler != null && updateHandler instanceof IndexWriterCloser) {
+          closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
+        } else {
+          closed = solrCoreState.decrefSolrCoreState(null);
+        }
+        coreStateClosed.set(closed);
+        return solrCoreState;
+      });
 
       closer.add("coreAsyncTaskExecutor", coreAsyncTaskExecutor, () -> {
-        // Since we waited for the searcherExecutor to shut down,
-        // there should be no more searchers warming in the background
-        // that we need to take care of.
-        //
-        // For the case that a searcher was registered *before* warming
-        // then the searchExecutor will throw an exception when getSearcher()
-        // tries to use it, and the exception handling code should close it.
-// nocommit
-//        synchronized (searcherLock) {
-//          for (RefCounted<SolrIndexSearcher> searcher :  _searchers) {
-//            searcher.decref();
-//          }
-//        }
-
 
         return "Searcher";
       });
@@ -1705,11 +1678,11 @@ public final class SolrCore implements SolrInfoBean, Closeable {
 
       });
 
-      closer.add("closeSearcher", ()->{
+      closer.add("closeSearcher", () -> {
         closeSearcher();
       });
 
-      closer.add("ClearInfoReg&ReleaseSnapShotsDir", ()->{
+      closer.add("ClearInfoReg&ReleaseSnapShotsDir", () -> {
         closeSearcher();
         return "searcher";
       }, () -> {
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
index edda4ab..4378d65 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
@@ -665,11 +665,12 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
     if (log.isDebugEnabled()) {
       log.debug("Closing {}", hashCode());
     }
-    collectService.shutdownNow();
+
     try (ParWork closer = new ParWork(this)) {
-      closer.collect(collectService);
       closer.collect(factory);
-      closer.addCollect("metricsHistoryHandlerClose");
+      closer.addCollect("factory");
+      closer.collect(collectService);
+      closer.addCollect("collectService");
     }
 
     knownDbs.clear();
diff --git a/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java
index 48699a1..259ca8c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java
@@ -61,6 +61,7 @@ public class MetricsHistoryWithAuthIntegrationTest extends SolrCloudTestCase {
 
   @BeforeClass
   public static void setupCluster() throws Exception {
+    System.setProperty("solr.disableMetricsHistoryHandler", "false");
     System.setProperty("solr.disableJmxReporter", "false");
     String solrXml = MiniSolrCloudCluster.DEFAULT_CLOUD_SOLR_XML.replace("<metrics>\n",
         "<metrics>\n" + SOLR_XML_HISTORY_CONFIG);
diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
index 141537b..9e52d33 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
@@ -44,6 +44,7 @@ public class SolrCoreMetricManagerTest extends SolrTestCaseJ4 {
 
   @Before
   public void beforeTest() throws Exception {
+    useFactory(null);
     initCore("solrconfig-basic.xml", "schema.xml");
     coreMetricManager = h.getCore().getCoreMetricManager();
     metricManager = h.getCore().getCoreContainer().getMetricManager();
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
index 5c7070a..a7fe0f8 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
@@ -194,6 +194,8 @@ public class SolrTestCase extends LuceneTestCase {
       // nocommit - not used again yet
       System.setProperty("solr.OverseerStateUpdateDelay", "0");
 
+      System.setProperty("solr.disableMetricsHistoryHandler", "true");
+
       System.setProperty("solr.leaderThrottle", "0");
       System.setProperty("solr.recoveryThrottle", "0");