You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2020/07/28 20:50:05 UTC
[lucene-solr] branch branch_8x updated: SOLR-14651: Metrics History
could disable better (#1672)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new f4f0163 SOLR-14651: Metrics History could disable better (#1672)
f4f0163 is described below
commit f4f0163e6aae977b85fffbbc36007ef1611e41f1
Author: David Smiley <ds...@salesforce.com>
AuthorDate: Tue Jul 28 16:46:27 2020 -0400
SOLR-14651: Metrics History could disable better (#1672)
* SolrRrdBackendFactory should not be created if history is disabled
* Disable MetricsHistoryHandler by default in tests
* Await shutdown of all executors
(cherry picked from commit a3624029ad262489b091d9a474430359f782d307)
---
solr/CHANGES.txt | 6 ++++++
.../java/org/apache/solr/core/CoreContainer.java | 10 +++-------
.../solr/handler/admin/MetricsHistoryHandler.java | 21 ++++++---------------
.../solr/metrics/rrd/SolrRrdBackendFactory.java | 5 +++--
.../org/apache/solr/common/util/ExecutorUtil.java | 8 +++++++-
.../src/java/org/apache/solr/util/TestHarness.java | 6 +++++-
6 files changed, 30 insertions(+), 26 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a59d945..8cb40f0 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -28,6 +28,12 @@ Improvements
* SOLR-11262: Implement writeMap and writeIterator in XMLWriter (yonik, Munendra S N)
+* SOLR-13205: Prevent StringIndexOutOfBoundsException when parsing field names in SolrQueryParserBase
+ (pramodkumar9 via Jason Gerlowski)
+
+* SOLR-14651: The MetricsHistoryHandler can more completely disable itself when you tell it to.
+ Also, it now shuts down more thoroughly. (David Smiley)
+
Optimizations
---------------------
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 7c16e8a..6c4eb58 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -903,7 +903,7 @@ public class CoreContainer {
Map<String, Object> initArgs;
if (plugin != null && plugin.initArgs != null) {
initArgs = plugin.initArgs.asMap(5);
- initArgs.put(MetricsHistoryHandler.ENABLE_PROP, plugin.isEnabled());
+ initArgs.putIfAbsent(MetricsHistoryHandler.ENABLE_PROP, plugin.isEnabled());
} else {
initArgs = new HashMap<>();
}
@@ -929,12 +929,8 @@ public class CoreContainer {
}
};
// enable local metrics unless specifically set otherwise
- if (!initArgs.containsKey(MetricsHistoryHandler.ENABLE_NODES_PROP)) {
- initArgs.put(MetricsHistoryHandler.ENABLE_NODES_PROP, true);
- }
- if (!initArgs.containsKey(MetricsHistoryHandler.ENABLE_REPLICAS_PROP)) {
- initArgs.put(MetricsHistoryHandler.ENABLE_REPLICAS_PROP, true);
- }
+ initArgs.putIfAbsent(MetricsHistoryHandler.ENABLE_NODES_PROP, true);
+ initArgs.putIfAbsent(MetricsHistoryHandler.ENABLE_REPLICAS_PROP, true);
}
metricsHistoryHandler = new MetricsHistoryHandler(name, metricsHandler,
client, cloudManager, initArgs);
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 5c475a1..43b7f38 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
@@ -208,8 +208,6 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
this.metricsHandler = metricsHandler;
this.cloudManager = cloudManager;
this.timeSource = cloudManager != null ? cloudManager.getTimeSource() : TimeSource.NANO_TIME;
- factory = new SolrRrdBackendFactory(solrClient, CollectionAdminParams.SYSTEM_COLL,
- syncPeriod, this.timeSource);
counters.put(Group.core.toString(), DEFAULT_CORE_COUNTERS);
counters.put(Group.node.toString(), Collections.emptyList());
@@ -229,6 +227,9 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
}
if (enable) {
+ factory = new SolrRrdBackendFactory(solrClient, CollectionAdminParams.SYSTEM_COLL,
+ syncPeriod, this.timeSource);
+
collectService = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(1,
new SolrNamedThreadFactory("MetricsHistoryHandler"));
collectService.setRemoveOnCancelPolicy(true);
@@ -238,6 +239,8 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
timeSource.convertDelay(TimeUnit.SECONDS, collectPeriod, TimeUnit.MILLISECONDS),
TimeUnit.MILLISECONDS);
checkSystemCollection();
+ } else {
+ factory = null;
}
}
@@ -655,19 +658,7 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
if (log.isDebugEnabled()) {
log.debug("Closing {}", hashCode());
}
- if (collectService != null) {
- boolean shutdown = false;
- while (!shutdown) {
- try {
- // Wait a while for existing tasks to terminate
- collectService.shutdownNow();
- shutdown = collectService.awaitTermination(5, TimeUnit.SECONDS);
- } catch (InterruptedException ie) {
- // Preserve interrupt status
- Thread.currentThread().interrupt();
- }
- }
- }
+ ExecutorUtil.shutdownNowAndAwaitTermination(collectService);
if (factory != null) {
factory.close();
}
diff --git a/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java b/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java
index 97d28f1..105fd64 100644
--- a/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java
+++ b/solr/core/src/java/org/apache/solr/metrics/rrd/SolrRrdBackendFactory.java
@@ -44,6 +44,7 @@ import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.ExecutorUtil;
import org.apache.solr.common.util.IOUtils;
import org.apache.solr.common.util.Pair;
import org.apache.solr.common.util.TimeSource;
@@ -459,9 +460,9 @@ public class SolrRrdBackendFactory extends RrdBackendFactory implements SolrClos
log.debug("Closing {}", hashCode());
}
closed = true;
- backends.forEach((p, b) -> IOUtils.closeQuietly(b));
+ backends.values().forEach(IOUtils::closeQuietly);
backends.clear();
- syncService.shutdownNow();
+ ExecutorUtil.shutdownNowAndAwaitTermination(syncService);
syncService = null;
}
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
index 670a6b4..6664dbf 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
@@ -72,11 +72,17 @@ public class ExecutorUtil {
}
public static void shutdownAndAwaitTermination(ExecutorService pool) {
- if(pool == null) return;
+ if (pool == null) return;
pool.shutdown(); // Disable new tasks from being submitted
awaitTermination(pool);
}
+ public static void shutdownNowAndAwaitTermination(ExecutorService pool) {
+ if (pool == null) return;
+ pool.shutdownNow(); // Disable new tasks from being submitted; interrupt existing tasks
+ awaitTermination(pool);
+ }
+
public static void awaitTermination(ExecutorService pool) {
boolean shutdown = false;
while (!shutdown) {
diff --git a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
index 5814ad4..7657c3f 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -191,7 +192,10 @@ public class TestHarness extends BaseTestHarness {
attributes.put("class", SolrJmxReporter.class.getName());
PluginInfo defaultPlugin = new PluginInfo("reporter", attributes);
MetricsConfig metricsConfig = new MetricsConfig.MetricsConfigBuilder()
- .setMetricReporterPlugins(new PluginInfo[] {defaultPlugin})
+ .setMetricReporterPlugins(new PluginInfo[]{defaultPlugin})
+ .setHistoryHandler(
+ Boolean.getBoolean("metricsHistory")
+ ? null : new PluginInfo("typeUnused", Collections.singletonMap("enable", "false")))
.build();
return new NodeConfig.NodeConfigBuilder("testNode", solrHome)