You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by md...@apache.org on 2021/01/29 16:22:03 UTC
[lucene-solr] branch branch_8x updated: SOLR-15120 Reduce
duplicated core creation work
This is an automated email from the ASF dual-hosted git repository.
mdrob 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 5a95d3f SOLR-15120 Reduce duplicated core creation work
5a95d3f is described below
commit 5a95d3f2942b65033b95f3e1806d48f3dc68f05f
Author: Mike Drob <md...@apache.org>
AuthorDate: Fri Jan 22 11:52:21 2021 -0600
SOLR-15120 Reduce duplicated core creation work
Use j.u.c collections instead of sync block
Rework how we load implicit handlers
Additional debug and trace logging for zookeeper comms
---
solr/CHANGES.txt | 2 ++
.../cloud/api/collections/DeleteCollectionCmd.java | 7 +++--
.../OverseerCollectionMessageHandler.java | 7 +----
.../java/org/apache/solr/core/ConfigOverlay.java | 2 +-
.../java/org/apache/solr/core/CoreContainer.java | 24 ++++++---------
.../src/java/org/apache/solr/core/SolrCore.java | 34 +++++++++++++---------
.../org/apache/solr/metrics/SolrMetricManager.java | 5 +++-
.../metrics/reporters/jmx/JmxMetricsReporter.java | 8 ++---
.../apache/solr/HelloWorldSolrCloudTestCase.java | 2 +-
.../org/apache/solr/core/TestCoreContainer.java | 2 +-
.../apache/solr/common/cloud/ZkCmdExecutor.java | 18 ++++++++++--
.../org/apache/solr/common/cloud/ZkOperation.java | 4 +--
.../apache/solr/common/cloud/ZkStateReader.java | 7 ++++-
.../apache/solr/common/util/ValidatingJsonMap.java | 5 ++--
14 files changed, 73 insertions(+), 54 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index c6fe563..2e49501 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -21,6 +21,8 @@ Optimizations
* SOLR-15079: Block Collapse - Faster collapse code when groups are co-located via Block Join style nested doc indexing.
Used by default when field=_root_, or explicitly requested for other fields via hint=block. (Joel Bernstein, hossman)
+* SOLR-15120 Reduce duplicated core creation work (Mike Drob, Mark Miller)
+
Bug Fixes
---------------------
* SOLR-15078: Fix ExpandComponent behavior when expanding on numeric fields to differentiate '0' group from null group (hossman)
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
index 5c08057..c7aeb90 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
@@ -20,7 +20,7 @@ package org.apache.solr.cloud.api.collections;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -61,6 +61,9 @@ import static org.apache.solr.common.params.CommonParams.NAME;
public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final Set<String> okayExceptions = Collections.singleton(NonExistentCoreException.class.getName());
+
private final OverseerCollectionMessageHandler ocmh;
private final TimeSource timeSource;
@@ -129,8 +132,6 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
String asyncId = message.getStr(ASYNC);
- Set<String> okayExceptions = new HashSet<>(1);
- okayExceptions.add(NonExistentCoreException.class.getName());
ZkNodeProps internalMsg = message.plus(NAME, collection);
@SuppressWarnings({"unchecked"})
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
index 3c474e5..7f62f03 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
@@ -313,7 +313,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler,
params.set(CoreAdminParams.ACTION, CoreAdminAction.RELOAD.toString());
String asyncId = message.getStr(ASYNC);
- collectionCmd(message, params, results, Replica.State.ACTIVE, asyncId);
+ collectionCmd(message, params, results, Replica.State.ACTIVE, asyncId, Collections.emptySet());
}
@SuppressWarnings("unchecked")
@@ -750,11 +750,6 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler,
}
}
- private List<Replica> collectionCmd(ZkNodeProps message, ModifiableSolrParams params,
- NamedList<Object> results, Replica.State stateMatcher, String asyncId) {
- return collectionCmd( message, params, results, stateMatcher, asyncId, Collections.emptySet());
- }
-
/**
* Send request to all replicas of a collection
* @return List of replicas which is not live for receiving the request
diff --git a/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java b/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java
index 7e05b2d..1bc8824 100644
--- a/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java
+++ b/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java
@@ -223,7 +223,7 @@ public class ConfigOverlay implements MapSerializable {
public Map<String, String> getEditableSubProperties(String xpath) {
Object o = Utils.getObjectByPath(props, false, StrUtils.splitSmart(xpath, '/'));
if (o instanceof Map) {
- return (Map) o;
+ return (Map<String,String>) o;
} else {
return null;
}
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 6022f83..d2d6b24 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -28,7 +28,6 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
@@ -1261,7 +1260,7 @@ public class CoreContainer {
return create(coreName, cfg.getCoreRootDirectory().resolve(coreName), parameters, false);
}
- Set<String> inFlightCreations = new HashSet<>(); // See SOLR-14969
+ final Set<String> inFlightCreations = ConcurrentHashMap.newKeySet(); // See SOLR-14969
/**
* Creates a new core in a specified instance directory, publishing the core state to the cluster
*
@@ -1271,17 +1270,13 @@ public class CoreContainer {
* @return the newly created core
*/
public SolrCore create(String coreName, Path instancePath, Map<String, String> parameters, boolean newCollection) {
- SolrCore core = null;
boolean iAdded = false;
try {
- synchronized (inFlightCreations) {
- if (inFlightCreations.add(coreName)) {
- iAdded = true;
- } else {
- String msg = "Already creating a core with name '" + coreName + "', call aborted '";
- log.warn(msg);
- throw new SolrException(ErrorCode.CONFLICT, msg);
- }
+ iAdded = inFlightCreations.add(coreName);
+ if (! iAdded) {
+ String msg = "Already creating a core with name '" + coreName + "', call aborted '";
+ log.warn(msg);
+ throw new SolrException(ErrorCode.CONFLICT, msg);
}
CoreDescriptor cd = new CoreDescriptor(coreName, instancePath, parameters, getContainerProperties(), getZkController());
@@ -1312,6 +1307,7 @@ public class CoreContainer {
// first and clean it up if there's an error.
coresLocator.create(this, cd);
+ SolrCore core;
try {
solrCores.waitAddPendingCoreOps(cd.getName());
core = createFromDescriptor(cd, true, newCollection);
@@ -1356,10 +1352,8 @@ public class CoreContainer {
"Error CREATEing SolrCore '" + coreName + "': " + ex.getMessage() + rootMsg, ex);
}
} finally {
- synchronized (inFlightCreations) {
- if (iAdded) {
- inFlightCreations.remove(coreName);
- }
+ if (iAdded) {
+ inFlightCreations.remove(coreName);
}
}
}
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 6c67e57..ae838fb 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -175,7 +175,6 @@ import org.apache.zookeeper.data.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.common.params.CommonParams.PATH;
/**
@@ -3226,20 +3225,29 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
}
}
- @SuppressWarnings({"rawtypes"})
- private static final Map implicitPluginsInfo = (Map) Utils.fromJSONResource("ImplicitPlugins.json");
+ private static final class ImplicitHolder {
+ private ImplicitHolder() { }
+
+ private static final List<PluginInfo> INSTANCE;
+
+ static {
+ @SuppressWarnings("unchecked")
+ Map<String,?> implicitPluginsInfo = (Map<String,?>) Utils.fromJSONResource("ImplicitPlugins.json");
+ @SuppressWarnings("unchecked")
+ Map<String, Map<String,Object>> requestHandlers = (Map<String, Map<String, Object>>) implicitPluginsInfo.get(SolrRequestHandler.TYPE);
+
+ List<PluginInfo> implicits = new ArrayList<>(requestHandlers.size());
+ for (Map.Entry<String, Map<String,Object>> entry : requestHandlers.entrySet()) {
+ Map<String,Object> info = entry.getValue();
+ info.put(CommonParams.NAME, entry.getKey());
+ implicits.add(new PluginInfo(SolrRequestHandler.TYPE, info));
+ }
+ INSTANCE = Collections.unmodifiableList(implicits);
+ }
+ }
- @SuppressWarnings({"unchecked", "rawtypes"})
public List<PluginInfo> getImplicitHandlers() {
- List<PluginInfo> implicits = new ArrayList<>();
- Map requestHandlers = (Map) implicitPluginsInfo.get(SolrRequestHandler.TYPE);
- for (Object o : requestHandlers.entrySet()) {
- Map.Entry<String, Map> entry = (Map.Entry<String, Map>) o;
- Map info = Utils.getDeepCopy(entry.getValue(), 4);
- info.put(NAME, entry.getKey());
- implicits.add(new PluginInfo(SolrRequestHandler.TYPE, info));
- }
- return implicits;
+ return ImplicitHolder.INSTANCE;
}
/**
diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
index 98a056f..b40bb4b 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java
@@ -1129,8 +1129,8 @@ public class SolrMetricManager {
log.warn("Interrupted while trying to obtain lock to modify reporters registry: {}", registry);
return Collections.emptySet();
}
- log.info("Closing metric reporters for registry={} tag={}", registry, tag);
try {
+ log.info("Closing metric reporters for registry={} tag={}", registry, tag);
Map<String, SolrMetricReporter> perRegistry = reporters.get(registry);
if (perRegistry != null) {
Set<String> names = new HashSet<>(perRegistry.keySet());
@@ -1156,6 +1156,9 @@ public class SolrMetricManager {
}
} finally {
reportersLock.unlock();
+ if (log.isDebugEnabled()) {
+ log.debug("Finished closing registry={}, tag={}", registry, tag);
+ }
}
}
diff --git a/solr/core/src/java/org/apache/solr/metrics/reporters/jmx/JmxMetricsReporter.java b/solr/core/src/java/org/apache/solr/metrics/reporters/jmx/JmxMetricsReporter.java
index 73ace0c..6f8da6d 100644
--- a/solr/core/src/java/org/apache/solr/metrics/reporters/jmx/JmxMetricsReporter.java
+++ b/solr/core/src/java/org/apache/solr/metrics/reporters/jmx/JmxMetricsReporter.java
@@ -527,7 +527,7 @@ public class JmxMetricsReporter implements Reporter, Closeable {
}
for (ObjectInstance inst : objects) {
if (log.isDebugEnabled()) {
- log.debug("## - tag={}{}", mBeanServer.getAttribute(inst.getObjectName(), INSTANCE_TAG));
+ log.debug("## - tag={}", mBeanServer.getAttribute(inst.getObjectName(), INSTANCE_TAG));
}
}
}
@@ -567,8 +567,8 @@ public class JmxMetricsReporter implements Reporter, Closeable {
if (filter.matches(name, gauge)) {
final ObjectName objectName = createName("gauges", name);
if (gauge instanceof SolrMetricManager.GaugeWrapper &&
- ((SolrMetricManager.GaugeWrapper)gauge).getGauge() instanceof MetricsMap) {
- MetricsMap mm = (MetricsMap)((SolrMetricManager.GaugeWrapper)gauge).getGauge();
+ ((SolrMetricManager.GaugeWrapper<?>)gauge).getGauge() instanceof MetricsMap) {
+ MetricsMap mm = (MetricsMap)((SolrMetricManager.GaugeWrapper<?>)gauge).getGauge();
mm.setAttribute(new Attribute(INSTANCE_TAG, tag));
// don't wrap it in a JmxGauge, it already supports all necessary JMX attributes
registerMBean(mm, objectName);
@@ -746,7 +746,7 @@ public class JmxMetricsReporter implements Reporter, Closeable {
} else if (v instanceof Timer) {
listener.onTimerAdded(k, (Timer)v);
} else if (v instanceof Gauge) {
- listener.onGaugeAdded(k, (Gauge)v);
+ listener.onGaugeAdded(k, (Gauge<?>)v);
} else {
log.warn("Unknown metric type {} for metric '{}', ignoring", v.getClass().getName(), k);
}
diff --git a/solr/core/src/test/org/apache/solr/HelloWorldSolrCloudTestCase.java b/solr/core/src/test/org/apache/solr/HelloWorldSolrCloudTestCase.java
index 56a813c..c906659 100644
--- a/solr/core/src/test/org/apache/solr/HelloWorldSolrCloudTestCase.java
+++ b/solr/core/src/test/org/apache/solr/HelloWorldSolrCloudTestCase.java
@@ -31,7 +31,7 @@ import org.junit.Test;
* How to use this test class:
* #1 Run the test, e.g.
* in Eclipse 'Run As JUnit Test' or
- * on the command line: cd solr/core ; ant test -Dtestcase=HelloWorldSolrCloudTestCase
+ * on the command line: ./gradlew -p solr/core test --tests HelloWorldSolrCloudTestCase
* #2 Modify the test, e.g.
* in setupCluster add further documents and then re-run the test.
*/
diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
index 8a1395c..e91e219 100644
--- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
+++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
@@ -221,7 +221,7 @@ public class TestCoreContainer extends SolrTestCaseJ4 {
// time around. The final unload in the loop should delete the core and
// allow the next time around to succeed.
// This also checks the bookkeeping in CoreContainer.create
- // that prevents muliple simulatneous creations,
+ // that prevents multiple simultaneous creations,
// currently "inFlightCreations"
String testName = "coreToTest";
for (int thread = 0; thread < NUM_THREADS; ++thread) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java
index aaba7ae..3598b0a 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkCmdExecutor.java
@@ -21,9 +21,15 @@ import org.apache.solr.common.cloud.ConnectionManager.IsClosed;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.KeeperException.NodeExistsException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.invoke.MethodHandles;
public class ZkCmdExecutor {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
private long retryDelay = 1500L; // 1 second would match timeout, so 500 ms over for padding
private int retryCount;
private double timeouts;
@@ -59,16 +65,18 @@ public class ZkCmdExecutor {
/**
* Perform the given operation, retrying if the connection fails
*/
- @SuppressWarnings("unchecked")
- public <T> T retryOperation(ZkOperation operation)
+ public <T> T retryOperation(ZkOperation<T> operation)
throws KeeperException, InterruptedException {
KeeperException exception = null;
for (int i = 0; i < retryCount; i++) {
try {
+ if (log.isTraceEnabled()) {
+ log.trace("Begin zookeeper operation {}, attempt={}", operation, i);
+ }
if (i > 0 && isClosed()) {
throw new AlreadyClosedException();
}
- return (T) operation.execute();
+ return operation.execute();
} catch (KeeperException.ConnectionLossException e) {
if (exception == null) {
exception = e;
@@ -80,6 +88,10 @@ public class ZkCmdExecutor {
if (i != retryCount -1) {
retryDelay(i);
}
+ } finally {
+ if (log.isTraceEnabled()) {
+ log.trace("End zookeeper operation {}", operation);
+ }
}
}
throw exception;
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkOperation.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkOperation.java
index e47e01f..13e687d 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkOperation.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkOperation.java
@@ -21,7 +21,7 @@ import org.apache.zookeeper.KeeperException;
/**
* A callback object which can be used for implementing retry-able operations.
*/
-public interface ZkOperation {
+public interface ZkOperation<T> {
/**
* Performs the operation - which may be involved multiple times if the connection
@@ -29,5 +29,5 @@ public interface ZkOperation {
*
* @return the result of the operation or null
*/
- Object execute() throws KeeperException, InterruptedException;
+ T execute() throws KeeperException, InterruptedException;
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index b793aa0..d9e831e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -1871,7 +1871,9 @@ public class ZkStateReader implements SolrCloseable {
*/
public void waitForState(final String collection, long wait, TimeUnit unit, Predicate<DocCollection> predicate)
throws InterruptedException, TimeoutException {
-
+ if (log.isDebugEnabled()) {
+ log.debug("Waiting up to {}ms for state {}", unit.toMillis(wait), predicate);
+ }
if (closed) {
throw new AlreadyClosedException();
}
@@ -1897,6 +1899,9 @@ public class ZkStateReader implements SolrCloseable {
} finally {
removeDocCollectionWatcher(collection, watcher);
waitLatches.remove(latch);
+ if (log.isDebugEnabled()) {
+ log.debug("Completed wait for {}", predicate);
+ }
}
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java b/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java
index dfa3173..fbdc015 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java
@@ -279,9 +279,8 @@ public class ValidatingJsonMap implements Map<String, Object>, NavigableObject {
map.putAll(includedMap);
}
if (maxDepth > 0) {
- map.entrySet().stream()
- .filter(e -> e.getValue() instanceof Map)
- .map(Map.Entry::getValue)
+ map.values().stream()
+ .filter(o -> o instanceof Map)
.forEach(m -> handleIncludes((ValidatingJsonMap) m, loc, maxDepth - 1));
}
}