From 5295007022b524160b76a7afc55b76d1eee26541 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sat, 25 Jul 2020 01:08:23 -0400 Subject: [PATCH] SOLR-14652: SolrCore should hold its own CoreDescriptor (#1675) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (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.   --- .../org/apache/solr/core/CoreContainer.java | 17 +++++---- .../java/org/apache/solr/core/SolrCore.java | 36 +++++++++---------- .../java/org/apache/solr/core/SolrCores.java | 2 ++ .../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 35787317ea2..1cec6e40bae 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -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 * 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 4c9fae68e7a..dc6ef6edad6 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, 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 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() { 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 bfce95923d0..205b70fcf48 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 572c01cf06b..f05889c8803 100644 --- a/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java +++ b/solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java @@ -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)) {