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/25 05:23:35 UTC
[lucene-solr] branch branch_8x updated: SOLR-14652: SolrCore should
hold its own CoreDescriptor (#1675)
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 7705f58 SOLR-14652: SolrCore should hold its own CoreDescriptor (#1675)
7705f58 is described below
commit 7705f58e33a825f8374cad0e2cfa6fd0694000c9
Author: David Smiley <ds...@apache.org>
AuthorDate: Sat Jul 25 01:08:23 2020 -0400
SOLR-14652: SolrCore should hold its own CoreDescriptor (#1675)
(minor refactoring)
Also:
* SolrCore's constructors don't need a "name" since it's guaranteed to always be the name in the coreDescriptor. I checked.
* SolrCore's constructor shouldn't call coreContainer.solrCores.addCoreDescriptor(cd); because it's the container's responsibility to manage such things. I made SolrCores.putCore ensure the descriptor is added, and this is called by CoreContainer.registerCore which is called after new SolrCore instances are created.
* solrCore.setName should only be called when we expect the name to change. Furthermore that shouldn't ever happen in SolrCloud so I added checks.
* solrCore.setName calls coreMetricManager.afterCoreSetName() which is something that is really only related to a rename, not name initialization (from the constructor). I renamed that method and further only call it if the name did change from non-null.
(cherry picked from commit 5295007022b524160b76a7afc55b76d1eee26541)
---
.../java/org/apache/solr/core/CoreContainer.java | 17 ++++++----
.../src/java/org/apache/solr/core/SolrCore.java | 36 +++++++++++-----------
.../src/java/org/apache/solr/core/SolrCores.java | 2 ++
.../apache/solr/metrics/SolrCoreMetricManager.java | 4 +--
4 files changed, 33 insertions(+), 26 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 d17a4c3..7c16e8a 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1187,13 +1187,10 @@ public class CoreContainer {
core.close();
throw new IllegalStateException("This CoreContainer has been closed");
}
- SolrCore old = solrCores.putCore(cd, core);
- /*
- * set both the name of the descriptor and the name of the
- * core, since the descriptors name is used for persisting.
- */
- core.setName(cd.getName());
+ assert core.getName().equals(cd.getName()) : "core name " + core.getName() + " != cd " + cd.getName();
+
+ SolrCore old = solrCores.putCore(cd, core);
coreInitFailures.remove(cd.getName());
@@ -1718,6 +1715,7 @@ public class CoreContainer {
* Swaps two SolrCore descriptors.
*/
public void swap(String n0, String n1) {
+ apiAssumeStandalone();
if (n0 == null || n1 == null) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Can not swap unnamed cores.");
}
@@ -1813,6 +1811,7 @@ public class CoreContainer {
}
public void rename(String name, String toName) {
+ apiAssumeStandalone();
SolrIdentifierValidator.validateCoreName(toName);
try (SolrCore core = getCore(name)) {
if (core != null) {
@@ -1833,6 +1832,12 @@ public class CoreContainer {
}
}
+ private void apiAssumeStandalone() {
+ if (getZkController() != null) {
+ throw new SolrException(ErrorCode.BAD_REQUEST, "Not supported in SolrCloud");
+ }
+ }
+
/**
* Get the CoreDescriptors for all cores managed by this container
*
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 d3c23b5..0073c0f 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -199,6 +199,8 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
private boolean isReloaded = false;
+ private final CoreDescriptor coreDescriptor;
+ private final CoreContainer coreContainer;
private final SolrConfig solrConfig;
private final SolrResourceLoader resourceLoader;
private volatile IndexSchema schema;
@@ -237,7 +239,6 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
private Counter newSearcherCounter;
private Counter newSearcherMaxReachedCounter;
private Counter newSearcherOtherErrorsCounter;
- private final CoreContainer coreContainer;
private Set<String> metricNames = ConcurrentHashMap.newKeySet();
private final String metricTag = SolrMetricProducer.getUniqueMetricTag(this, null);
@@ -495,10 +496,13 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
}
public void setName(String v) {
+ Objects.requireNonNull(v);
+ boolean renamed = this.name != null && !this.name.equals(v);
+ assert !renamed || coreDescriptor.getCloudDescriptor() == null : "Cores are not renamed in SolrCloud";
this.name = v;
- this.logid = (v == null) ? "" : ("[" + v + "] ");
- if (coreMetricManager != null) {
- coreMetricManager.afterCoreSetName();
+ this.logid = "[" + v + "] "; // TODO remove; obsoleted by MDC
+ if (renamed && coreMetricManager != null) {
+ coreMetricManager.afterCoreRename();
}
}
@@ -704,7 +708,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
try {
CoreDescriptor cd = new CoreDescriptor(name, getCoreDescriptor());
cd.loadExtraProperties(); //Reload the extra properties
- core = new SolrCore(coreContainer, getName(), coreConfig, cd, getDataDir(),
+ core = new SolrCore(coreContainer, cd, coreConfig, getDataDir(),
updateHandler, solrDelPolicy, currentCore, true);
// we open a new IndexWriter to pick up the latest config
@@ -916,8 +920,8 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
return createReloadedUpdateHandler(className, "Update Handler", updateHandler);
}
- public SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet coreConfig) {
- this(coreContainer, cd.getName(), coreConfig, cd, null,
+ public SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) {
+ this(coreContainer, cd, configSet, null,
null, null, null, false);
}
@@ -930,22 +934,18 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
* Creates a new core and register it in the list of cores. If a core with the
* same name already exists, it will be stopped and replaced by this one.
*/
- private SolrCore(CoreContainer coreContainer, String name, ConfigSet configSet, CoreDescriptor coreDescriptor,
- String dataDir, UpdateHandler updateHandler,
- IndexDeletionPolicyWrapper delPolicy, SolrCore prev, boolean reload) {
+ private SolrCore(CoreContainer coreContainer, CoreDescriptor coreDescriptor, ConfigSet configSet,
+ String dataDir, UpdateHandler updateHandler,
+ IndexDeletionPolicyWrapper delPolicy, SolrCore prev, boolean reload) {
assert ObjectReleaseTracker.track(searcherExecutor); // ensure that in unclean shutdown tests we still close this
- this.coreContainer = coreContainer;
-
final CountDownLatch latch = new CountDownLatch(1);
-
try {
+ this.coreContainer = coreContainer;
+ this.coreDescriptor = Objects.requireNonNull(coreDescriptor, "coreDescriptor cannot be null");
+ setName(coreDescriptor.getName());
- CoreDescriptor cd = Objects.requireNonNull(coreDescriptor, "coreDescriptor cannot be null");
- coreContainer.solrCores.addCoreDescriptor(cd);
-
- setName(name);
this.solrConfig = configSet.getSolrConfig();
this.resourceLoader = configSet.getSolrConfig().getResourceLoader();
this.resourceLoader.core = this;
@@ -2957,7 +2957,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
}
public CoreDescriptor getCoreDescriptor() {
- return coreContainer.getCoreDescriptor(name);
+ return coreDescriptor;
}
public IndexDeletionPolicyWrapper getDeletionPolicy() {
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java b/solr/core/src/java/org/apache/solr/core/SolrCores.java
index 900738c..d18e3a1 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCores.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java
@@ -158,6 +158,8 @@ class SolrCores {
//WARNING! This should be the _only_ place you put anything into the list of transient cores!
protected SolrCore putCore(CoreDescriptor cd, SolrCore core) {
synchronized (modifyLock) {
+ addCoreDescriptor(cd); // cd must always be registered if we register a core
+
if (cd.isTransient()) {
if (getTransientCacheHandler() != null) {
return getTransientCacheHandler().addCore(cd.getName(), core);
diff --git a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java
index c318b8c..4fc3a9f 100644
--- a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java
+++ b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java
@@ -97,10 +97,10 @@ public class SolrCoreMetricManager implements Closeable {
* are carried over and will be used under the new core name.
* This method also reloads reporters so that they use the new core name.
*/
- public void afterCoreSetName() {
+ public void afterCoreRename() {
+ assert core.getCoreDescriptor().getCloudDescriptor() == null;
String oldRegistryName = solrMetricsContext.registry;
String oldLeaderRegistryName = leaderRegistryName;
- initCloudMode();
String newRegistryName = createRegistryName(cloudMode, collectionName, shardName, replicaName, core.getName());
leaderRegistryName = createLeaderRegistryName(cloudMode, collectionName, shardName);
if (oldRegistryName.equals(newRegistryName)) {