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.
This commit is contained in:
David Smiley 2020-07-25 01:08:23 -04:00 committed by GitHub
parent d0642600ff
commit 5295007022
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 33 additions and 26 deletions

View File

@ -1166,13 +1166,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());
@ -1693,6 +1690,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.");
}
@ -1788,6 +1786,7 @@ public class CoreContainer {
}
public void rename(String name, String toName) {
apiAssumeStandalone();
SolrIdentifierValidator.validateCoreName(toName);
try (SolrCore core = getCore(name)) {
if (core != null) {
@ -1808,6 +1807,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
*

View File

@ -199,6 +199,8 @@ public final class SolrCore implements SolrInfoBean, Closeable {
private boolean isReloaded = false;
private final CoreDescriptor coreDescriptor;
private final CoreContainer coreContainer;
private final SolrConfig solrConfig;
private final SolrResourceLoader resourceLoader;
private volatile IndexSchema schema;
@ -239,7 +241,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
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);
@ -497,10 +498,13 @@ public final class SolrCore implements SolrInfoBean, Closeable {
}
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();
}
}
@ -706,7 +710,7 @@ public final class SolrCore implements SolrInfoBean, Closeable {
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
@ -918,8 +922,8 @@ public final class SolrCore implements SolrInfoBean, Closeable {
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);
}
@ -932,22 +936,18 @@ public final class SolrCore implements SolrInfoBean, Closeable {
* 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, Closeable {
}
public CoreDescriptor getCoreDescriptor() {
return coreContainer.getCoreDescriptor(name);
return coreDescriptor;
}
public IndexDeletionPolicyWrapper getDeletionPolicy() {

View File

@ -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);

View File

@ -92,10 +92,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.getRegistryName();
String oldLeaderRegistryName = leaderRegistryName;
initCloudMode();
String newRegistryName = createRegistryName(cloudMode, collectionName, shardName, replicaName, core.getName());
leaderRegistryName = createLeaderRegistryName(cloudMode, collectionName, shardName);
if (oldRegistryName.equals(newRegistryName)) {