This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit aafcd51f40be49c6cdcb6aa31e89463dd5896e7f Author: Serhiy Bzhezytskyy <[email protected]> AuthorDate: Wed Aug 26 01:17:31 2026 +0300 SOLR-18378: remove CoreContainer.getCores() and SolrCores.getCores() (#4764) semi-replaced with forEachLoadedCore(lambda) Co-authored-by: David Smiley <[email protected]> (cherry picked from commit 1400dbcbc9369a15f73fd79dff22789bdf1f4f48) --- ...378-remove-corecontainer-solrcores-getcores.yml | 8 +++ .../java/org/apache/solr/core/CoreContainer.java | 81 +++++++++++----------- .../src/java/org/apache/solr/core/SolrCores.java | 16 +---- .../apache/solr/handler/admin/api/NodeHealth.java | 25 ++++--- .../org/apache/solr/pkg/SolrPackageLoader.java | 9 +-- .../cloud/LeaderFailureAfterFreshStartTest.java | 2 +- .../org/apache/solr/cloud/TestCloudRecovery.java | 20 +++--- .../solr/cloud/TestPullReplicaErrorHandling.java | 22 +++--- .../solr/cloud/TestRandomRequestDistribution.java | 44 +++++++----- .../org/apache/solr/cloud/TestTlogReplica.java | 54 ++++++++------- .../apache/solr/cloud/UnloadDistributedZkTest.java | 19 +++-- .../SimpleCollectionCreateDeleteTest.java | 17 +++-- .../org/apache/solr/core/TestCoreContainer.java | 6 +- .../test/org/apache/solr/core/TimeAllowedTest.java | 4 +- .../apache/solr/handler/TestContainerPlugin.java | 6 +- .../solr/handler/TestReplicationHandler.java | 69 +++++++++--------- .../solr/update/PeerSyncWithBufferUpdatesTest.java | 16 +++-- .../solr/ltr/AbstractLTRSolrCloudTestBase.java | 6 +- .../solr/cloud/AbstractFullDistribZkTestBase.java | 10 +-- .../apache/solr/cloud/MiniSolrCloudCluster.java | 2 +- ...bstractCollectionsAPIDistributedZkTestBase.java | 35 +++++----- .../solr/cloud/MiniSolrCloudClusterTest.java | 11 +-- 22 files changed, 256 insertions(+), 226 deletions(-) diff --git a/changelog/unreleased/SOLR-18378-remove-corecontainer-solrcores-getcores.yml b/changelog/unreleased/SOLR-18378-remove-corecontainer-solrcores-getcores.yml new file mode 100644 index 00000000000..446c485a0c9 --- /dev/null +++ b/changelog/unreleased/SOLR-18378-remove-corecontainer-solrcores-getcores.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Remove the deprecated CoreContainer.getCores() and SolrCores.getCores() methods, which handed out cores without reserving them. Use getLoadedCoreNames() with getCore(String) and close each core, or getLoadedCoreNames().size() when only a count is needed. +type: removed +authors: + - name: Serhiy Bzhezytskyy +links: + - name: SOLR-18378 + url: https://issues.apache.org/jira/browse/SOLR-18378 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 86a3b2677d9..2cc5f8d9f5c 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -57,6 +57,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.lucene.index.CorruptIndexException; @@ -1375,21 +1376,32 @@ public class CoreContainer { metricManager.closeAllRegistries(); // Close all OTEL meter providers and metrics } - public void cancelCoreRecoveries() { - - List<SolrCore> cores = solrCores.getCores(); - - // we must cancel without holding the cores sync - // make sure we wait for any recoveries to stop - for (SolrCore core : cores) { - try { - core.getSolrCoreState().cancelRecovery(); - } catch (Exception e) { - log.error("Error canceling recovery for core", e); + /** + * Applies the given action to each currently loaded core, reserving and releasing each one + * without triggering a lazy load. A core unloaded concurrently after {@link + * #getLoadedCoreNames()} was called is skipped. + */ + public void forEachLoadedCore(Consumer<SolrCore> action) { + for (String coreName : solrCores.getLoadedCoreNames()) { + // getCoreFromAnyList, not getCore: never loads + try (SolrCore core = solrCores.getCoreFromAnyList(coreName, true)) { + if (core == null) continue; // unloaded since getLoadedCoreNames + action.accept(core); } } } + public void cancelCoreRecoveries() { + forEachLoadedCore( + core -> { + try { + core.getSolrCoreState().cancelRecovery(); + } catch (Exception e) { + log.error("Error canceling recovery for core", e); + } + }); + } + /** * Pause updates for all cores on this node and wait for all in-flight update requests to finish. * Here, we (slightly) delay leader election so that in-flight update requests succeed and we can @@ -1402,21 +1414,25 @@ public class CoreContainer { * <p>We do not need to unpause ever because the node is being shut down. */ private void pauseUpdatesAndAwaitInflightRequests() { - getCores().parallelStream() + solrCores.getLoadedCoreNames().parallelStream() .forEach( - solrCore -> { - SolrCoreState solrCoreState = solrCore.getSolrCoreState(); - try { - solrCoreState.pauseUpdatesAndAwaitInflightRequests(); - } catch (TimeoutException e) { - log.warn( - "Timed out waiting for in-flight update requests to complete for core: {}", - solrCore.getName()); - } catch (InterruptedException e) { - log.warn( - "Interrupted while waiting for in-flight update requests to complete for core: {}", - solrCore.getName()); - Thread.currentThread().interrupt(); + coreName -> { + // see cancelCoreRecoveries: reserve without loading, we are shutting down + try (SolrCore solrCore = solrCores.getCoreFromAnyList(coreName, true)) { + if (solrCore == null) return; // unloaded since getLoadedCoreNames + SolrCoreState solrCoreState = solrCore.getSolrCoreState(); + try { + solrCoreState.pauseUpdatesAndAwaitInflightRequests(); + } catch (TimeoutException e) { + log.warn( + "Timed out waiting for in-flight update requests to complete for core: {}", + solrCore.getName()); + } catch (InterruptedException e) { + log.warn( + "Interrupted while waiting for in-flight update requests to complete for core: {}", + solrCore.getName()); + Thread.currentThread().interrupt(); + } } }); } @@ -1832,21 +1848,6 @@ public class CoreContainer { } } - /** - * Gets all loaded cores, consistent with {@link #getLoadedCoreNames()}. Caller doesn't need to - * close. - * - * <p>NOTE: rather dangerous API because each core is not reserved (could in theory be closed). - * Prefer {@link #getLoadedCoreNames()} and then call {@link #getCore(String)} then close it. - * - * @return An unsorted list. This list is a new copy, it can be modified by the caller (e.g. it - * can be sorted). Don't need to close them. - */ - @Deprecated - public List<SolrCore> getCores() { - return solrCores.getCores(); - } - /** * Gets the cores that are currently loaded, i.e. cores that have 1: loadOnStartup=true and have * been loaded and 2: loadOnStartup=false and have been subsequently loaded. 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 3a8d7a058ba..465c3a9d683 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCores.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java @@ -142,20 +142,6 @@ class SolrCores implements SolrInfoBean { } } - /** - * @return A list of "permanent" cores, i.e. cores that may not be swapped out and are currently - * loaded. - * <p>A core may be non-transient but still lazily loaded. If it is "permanent" and lazy-load - * _and_ not yet loaded it will _not_ be returned by this call. - * <p>This list is a new copy, it can be modified by the caller (e.g. it can be sorted). - */ - @Deprecated - public List<SolrCore> getCores() { - synchronized (modifyLock) { - return new ArrayList<>(cores.values()); - } - } - /** * Gets the cores that are currently loaded, i.e. cores that have 1: loadOnStartup=true and are * either not-transient or, if transient, have been loaded and have not been aged out 2: @@ -189,7 +175,7 @@ class SolrCores implements SolrInfoBean { /** * Gets the number of currently loaded permanent (non transient) cores. Faster equivalent for - * {@link #getCores()}.size(). + * {@link #getLoadedCoreNames()}.size(). */ public int getNumLoadedPermanentCores() { synchronized (modifyLock) { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/NodeHealth.java b/solr/core/src/java/org/apache/solr/handler/admin/api/NodeHealth.java index de207f334d1..a4c0373317e 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/NodeHealth.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/NodeHealth.java @@ -31,6 +31,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Locale; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; import org.apache.lucene.index.IndexCommit; import org.apache.solr.api.JerseyResource; @@ -140,7 +141,6 @@ public class NodeHealth extends JerseyResource implements NodeHealthApi { private void healthCheckStandaloneMode(NodeHealthResponse response, Integer maxGenerationLag) { List<String> laggingCoresInfo = new ArrayList<>(); - boolean allCoresAreInSync = true; if (maxGenerationLag != null) { if (maxGenerationLag < 0) { @@ -151,17 +151,20 @@ public class NodeHealth extends JerseyResource implements NodeHealthApi { return; } - for (SolrCore core : coreContainer.getCores()) { - ReplicationHandler replicationHandler = - (ReplicationHandler) core.getRequestHandler(ReplicationHandler.PATH); - if (replicationHandler.isFollower()) { - boolean isCoreInSync = - isWithinGenerationLag(core, replicationHandler, maxGenerationLag, laggingCoresInfo); - allCoresAreInSync &= isCoreInSync; - } - } + AtomicBoolean allCoresAreInSync = new AtomicBoolean(true); + coreContainer.forEachLoadedCore( + core -> { + ReplicationHandler replicationHandler = + (ReplicationHandler) core.getRequestHandler(ReplicationHandler.PATH); + if (replicationHandler.isFollower()) { + boolean isCoreInSync = + isWithinGenerationLag( + core, replicationHandler, maxGenerationLag, laggingCoresInfo); + allCoresAreInSync.set(allCoresAreInSync.get() & isCoreInSync); + } + }); - if (allCoresAreInSync) { + if (allCoresAreInSync.get()) { response.message = String.format( Locale.ROOT, diff --git a/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java b/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java index 364bbb919bc..34f6b136eb6 100644 --- a/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java +++ b/solr/core/src/java/org/apache/solr/pkg/SolrPackageLoader.java @@ -39,7 +39,6 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.apache.solr.common.MapWriter; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.core.CoreContainer; -import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrResourceLoader; import org.apache.solr.filestore.FileStoreUtils; import org.slf4j.Logger; @@ -106,9 +105,7 @@ public class SolrPackageLoader implements Closeable { } } } - for (SolrCore core : coreContainer.getCores()) { - core.getPackageListeners().packagesUpdated(updated); - } + coreContainer.forEachLoadedCore(core -> core.getPackageListeners().packagesUpdated(updated)); myCopy = packageAPI.pkgs; } @@ -146,9 +143,7 @@ public class SolrPackageLoader implements Closeable { SolrPackage p = packageClassLoaders.get(pkg); if (p != null) { List<SolrPackage> l = List.of(p); - for (SolrCore core : coreContainer.getCores()) { - core.getPackageListeners().packagesUpdated(l); - } + coreContainer.forEachLoadedCore(core -> core.getPackageListeners().packagesUpdated(l)); } } diff --git a/solr/core/src/test/org/apache/solr/cloud/LeaderFailureAfterFreshStartTest.java b/solr/core/src/test/org/apache/solr/cloud/LeaderFailureAfterFreshStartTest.java index f061661f113..b84aae4b77f 100644 --- a/solr/core/src/test/org/apache/solr/cloud/LeaderFailureAfterFreshStartTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/LeaderFailureAfterFreshStartTest.java @@ -135,7 +135,7 @@ public class LeaderFailureAfterFreshStartTest extends AbstractFullDistribZkTestB // start the freshNode restartNodes(List.of(freshNode)); - String coreName = freshNode.jetty.getCoreContainer().getCores().iterator().next().getName(); + String coreName = freshNode.jetty.getCoreContainer().getLoadedCoreNames().get(0); Path replicationProperties = Path.of(freshNode.jetty.getSolrHome(), "cores", coreName, "data", "replication.properties"); String md5 = DigestUtils.md5Hex(Files.readAllBytes(replicationProperties)); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery.java b/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery.java index 2aafcb85e99..54ce0670656 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery.java @@ -179,14 +179,18 @@ public class TestCloudRecovery extends SolrCloudTestCase { int logHeaderSize = Integer.MAX_VALUE; Map<String, byte[]> contentFiles = new HashMap<>(); for (JettySolrRunner solrRunner : cluster.getJettySolrRunners()) { - for (SolrCore solrCore : solrRunner.getCoreContainer().getCores()) { - Path tlogFolder = Path.of(solrCore.getUpdateHandler().getUpdateLog().getTlogDir()); - try (Stream<Path> tLogFiles = Files.list(tlogFolder)) { - Path lastTLogFile = - tlogFolder.resolve(tLogFiles.sorted().toList().getLast().getFileName()); - byte[] tlogBytes = Files.readAllBytes(lastTLogFile); - contentFiles.put(lastTLogFile.toString(), tlogBytes); - logHeaderSize = Math.min(tlogBytes.length, logHeaderSize); + CoreContainer coreContainer = solrRunner.getCoreContainer(); + for (String coreName : coreContainer.getLoadedCoreNames()) { + try (SolrCore solrCore = coreContainer.getCore(coreName)) { + if (solrCore == null) continue; // unloaded since getLoadedCoreNames + Path tlogFolder = Path.of(solrCore.getUpdateHandler().getUpdateLog().getTlogDir()); + try (Stream<Path> tLogFiles = Files.list(tlogFolder)) { + Path lastTLogFile = + tlogFolder.resolve(tLogFiles.sorted().toList().getLast().getFileName()); + byte[] tlogBytes = Files.readAllBytes(lastTLogFile); + contentFiles.put(lastTLogFile.toString(), tlogBytes); + logHeaderSize = Math.min(tlogBytes.length, logHeaderSize); + } } } } diff --git a/solr/core/src/test/org/apache/solr/cloud/TestPullReplicaErrorHandling.java b/solr/core/src/test/org/apache/solr/cloud/TestPullReplicaErrorHandling.java index 5d0ee95b541..c4c4047487c 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestPullReplicaErrorHandling.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestPullReplicaErrorHandling.java @@ -39,6 +39,7 @@ import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.TimeSource; +import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; import org.apache.solr.embedded.JettySolrRunner; import org.apache.solr.util.SocketProxy; @@ -251,16 +252,19 @@ public class TestPullReplicaErrorHandling extends SolrCloudTestCase { DocCollection docCollection = assertNumberOfReplicas(1, 0, 1, false, true); Slice s = docCollection.getSlices().iterator().next(); JettySolrRunner jetty = getJettyForReplica(s.getReplicas(EnumSet.of(Replica.Type.PULL)).get(0)); - SolrCore core = jetty.getCoreContainer().getCores().iterator().next(); + CoreContainer coreContainer = jetty.getCoreContainer(); - for (int i = 0; i < (TEST_NIGHTLY ? 5 : 2); i++) { - cluster.expireZkSession(jetty); - waitForState( - "Expecting node to be disconnected", collectionName, activeReplicaCount(1, 0, 0)); - waitForState("Expecting node to reconnect", collectionName, activeReplicaCount(1, 0, 1)); - // We have two active ReplicationHandler with two close hooks each, one for triggering - // recovery and one for doing interval polling - assertEquals(5, core.getCloseHooks().size()); + // held open across the reconnects below, so the same core instance is checked each time + try (SolrCore core = coreContainer.getCore(coreContainer.getLoadedCoreNames().get(0))) { + for (int i = 0; i < (TEST_NIGHTLY ? 5 : 2); i++) { + cluster.expireZkSession(jetty); + waitForState( + "Expecting node to be disconnected", collectionName, activeReplicaCount(1, 0, 0)); + waitForState("Expecting node to reconnect", collectionName, activeReplicaCount(1, 0, 1)); + // We have two active ReplicationHandler with two close hooks each, one for triggering + // recovery and one for doing interval polling + assertEquals(5, core.getCloseHooks().size()); + } } } diff --git a/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java b/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java index 08330e310c2..d19541921e5 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestRandomRequestDistribution.java @@ -39,6 +39,7 @@ import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; import org.apache.solr.embedded.JettySolrRunner; import org.apache.solr.util.SolrMetricTestUtils; @@ -83,21 +84,21 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase ZkStateReader.from(cloudClient).forceUpdateCollection("b1x1"); - // get direct access to the SolrCore objects for each core/replica we're interested to monitor - final Map<String, SolrCore> cores = new LinkedHashMap<>(); + // build a map of "a1x2"'s core names to corresponding CoreContainer + final Map<String, CoreContainer> cores = new LinkedHashMap<>(); for (JettySolrRunner runner : jettys) { CoreContainer container = runner.getCoreContainer(); - for (SolrCore core : container.getCores()) { - if ("a1x2".equals(core.getCoreDescriptor().getCollectionName())) { - cores.put(core.getName(), core); + for (CoreDescriptor coreDescriptor : container.getCoreDescriptors()) { + if ("a1x2".equals(coreDescriptor.getCollectionName())) { + cores.put(coreDescriptor.getName(), container); } } } assertEquals("Sanity Check: we know there should be 2 replicas", 2, cores.size()); // Sanity check - all cores should start with 0 requests - for (Map.Entry<String, SolrCore> entry : cores.entrySet()) { - double initialCount = getSelectRequestCount(entry.getValue()); + for (Map.Entry<String, CoreContainer> entry : cores.entrySet()) { + double initialCount = getSelectRequestCount(entry.getValue(), entry.getKey()); assertEquals(entry.getKey() + " has already received some requests?", 0L, initialCount, 0.0); } @@ -123,8 +124,8 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase client.query("a1x2", new SolrQuery("*:*")); double actualTotalRequests = 0; - for (Map.Entry<String, SolrCore> entry : cores.entrySet()) { - final double coreCount = getSelectRequestCount(entry.getValue()); + for (Map.Entry<String, CoreContainer> entry : cores.entrySet()) { + final double coreCount = getSelectRequestCount(entry.getValue(), entry.getKey()); actualTotalRequests += coreCount; if (0 < coreCount) { uniqueCoreNames.add(entry.getKey()); @@ -225,17 +226,16 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase .withIdleTimeout(5000, TimeUnit.MILLISECONDS) .build()) { - SolrCore leaderCore = null; + String leaderCoreName = leader.getStr(ZkStateReader.CORE_NAME_PROP); + CoreContainer leaderContainer = null; for (JettySolrRunner jetty : jettys) { CoreContainer container = jetty.getCoreContainer(); - for (SolrCore core : container.getCores()) { - if (core.getName().equals(leader.getStr(ZkStateReader.CORE_NAME_PROP))) { - leaderCore = core; - break; - } + if (container.getCoreDescriptor(leaderCoreName) != null) { + leaderContainer = container; + break; } } - assertNotNull(leaderCore); + assertNotNull(leaderContainer); // All queries should be served by the active replica to make sure that's true we keep // querying the down replica. If queries are getting processed by the down replica then the @@ -246,7 +246,7 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase count++; client.query(new SolrQuery("*:*")); - double c = getSelectRequestCount(leaderCore); + double c = getSelectRequestCount(leaderContainer, leaderCoreName); if (c == 1) { break; // cluster state has got update locally @@ -267,13 +267,21 @@ public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase client.query(new SolrQuery("*:*")); count++; - double c = getSelectRequestCount(leaderCore); + double c = getSelectRequestCount(leaderContainer, leaderCoreName); assertEquals("Query wasn't served by leader", count, (long) c); } } } + /** Reserves the named core just long enough to read its /select request count. */ + private double getSelectRequestCount(CoreContainer container, String coreName) { + try (SolrCore core = container.getCore(coreName)) { + assertNotNull("Core " + coreName + " is no longer loaded", core); + return getSelectRequestCount(core); + } + } + private Double getSelectRequestCount(SolrCore core) { var labels = SolrMetricTestUtils.newCloudLabelsBuilder(core) diff --git a/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java b/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java index ce0fb84f6d5..855a6562ce4 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; import java.util.stream.Collectors; import org.apache.lucene.index.IndexWriter; import org.apache.solr.client.solrj.SolrClient; @@ -59,6 +60,8 @@ import org.apache.solr.common.params.CollectionParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.TimeSource; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; import org.apache.solr.embedded.JettySolrRunner; import org.apache.solr.update.SolrIndexWriter; @@ -1110,25 +1113,38 @@ public class TestTlogReplica extends SolrCloudTestCase { }; } - private List<SolrCore> getSolrCore(boolean isLeader) { - List<SolrCore> rs = new ArrayList<>(); - + /** + * Invokes {@code consumer} for each core, across the cluster, whose leader/replica status matches + * {@code isLeader}. + */ + private void forEachMatchingCloudDescriptor( + boolean isLeader, BiConsumer<JettySolrRunner, CoreDescriptor> consumer) { CloudSolrClient cloudClient = cluster.getSolrClient(); DocCollection docCollection = cloudClient.getClusterState().getCollection(collectionName); - for (JettySolrRunner solrRunner : cluster.getJettySolrRunners()) { - if (solrRunner.getCoreContainer() == null) continue; - for (SolrCore solrCore : solrRunner.getCoreContainer().getCores()) { - CloudDescriptor cloudDescriptor = solrCore.getCoreDescriptor().getCloudDescriptor(); + CoreContainer coreContainer = solrRunner.getCoreContainer(); + if (coreContainer == null) continue; + for (CoreDescriptor coreDescriptor : coreContainer.getCoreDescriptors()) { + CloudDescriptor cloudDescriptor = coreDescriptor.getCloudDescriptor(); Slice slice = docCollection.getSlice(cloudDescriptor.getShardId()); Replica replica = docCollection.getReplica(cloudDescriptor.getCoreNodeName()); - if (Objects.equals(slice.getLeader(), replica) && isLeader) { - rs.add(solrCore); - } else if (!Objects.equals(slice.getLeader(), replica) && !isLeader) { - rs.add(solrCore); + if (Objects.equals(slice.getLeader(), replica) == isLeader) { + consumer.accept(solrRunner, coreDescriptor); } } } + } + + /** NOT INC-REF'ED. Assumption: the returned cores are not closed or going to be closed yet. */ + private List<SolrCore> getSolrCore(boolean isLeader) { + List<SolrCore> rs = new ArrayList<>(); + forEachMatchingCloudDescriptor( + isLeader, + (solrRunner, coreDescriptor) -> { + SolrCore solrCore = solrRunner.getCoreContainer().getCore(coreDescriptor.getName()); + solrCore.close(); // dec-ref. + rs.add(solrCore); + }); return rs; } @@ -1151,21 +1167,7 @@ public class TestTlogReplica extends SolrCloudTestCase { private List<JettySolrRunner> getSolrRunner(boolean isLeader) { List<JettySolrRunner> rs = new ArrayList<>(); - CloudSolrClient cloudClient = cluster.getSolrClient(); - DocCollection docCollection = cloudClient.getClusterState().getCollection(collectionName); - for (JettySolrRunner solrRunner : cluster.getJettySolrRunners()) { - if (solrRunner.getCoreContainer() == null) continue; - for (SolrCore solrCore : solrRunner.getCoreContainer().getCores()) { - CloudDescriptor cloudDescriptor = solrCore.getCoreDescriptor().getCloudDescriptor(); - Slice slice = docCollection.getSlice(cloudDescriptor.getShardId()); - Replica replica = docCollection.getReplica(cloudDescriptor.getCoreNodeName()); - if (Objects.equals(slice.getLeader(), replica) && isLeader) { - rs.add(solrRunner); - } else if (!Objects.equals(slice.getLeader(), replica) && !isLeader) { - rs.add(solrRunner); - } - } - } + forEachMatchingCloudDescriptor(isLeader, (solrRunner, coreDescriptor) -> rs.add(solrRunner)); return rs; } diff --git a/solr/core/src/test/org/apache/solr/cloud/UnloadDistributedZkTest.java b/solr/core/src/test/org/apache/solr/cloud/UnloadDistributedZkTest.java index fcbc8b23ad1..93781dbbd1e 100644 --- a/solr/core/src/test/org/apache/solr/cloud/UnloadDistributedZkTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/UnloadDistributedZkTest.java @@ -188,14 +188,17 @@ public class UnloadDistributedZkTest extends AbstractFullDistribZkTestBase { cloudClient.getClusterState().hasCollection(collection)); } + /** + * A reserved core of the given collection, or null if the node hosts none; caller must close it. + */ protected SolrCore getFirstCore(String collection, JettySolrRunner jetty) { - SolrCore solrCore = null; - for (SolrCore core : jetty.getCoreContainer().getCores()) { - if (core.getName().startsWith(collection)) { - solrCore = core; + String coreName = null; + for (String name : jetty.getCoreContainer().getLoadedCoreNames()) { + if (name.startsWith(collection)) { + coreName = name; } } - return solrCore; + return coreName == null ? null : jetty.getCoreContainer().getCore(coreName); } /** @@ -218,8 +221,10 @@ public class UnloadDistributedZkTest extends AbstractFullDistribZkTestBase { int slices = zkStateReader.getClusterState().getCollection("unloadcollection").getSlices().size(); assertEquals(1, slices); - SolrCore solrCore = getFirstCore("unloadcollection", jetty1); - String core1DataDir = solrCore.getDataDir(); + String core1DataDir; + try (SolrCore solrCore = getFirstCore("unloadcollection", jetty1)) { + core1DataDir = solrCore.getDataDir(); + } assertTrue( CollectionAdminRequest.addReplicaToShard("unloadcollection", "shard1") diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java index 6b09a449e90..210cd1bed85 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java @@ -17,7 +17,6 @@ package org.apache.solr.cloud.api.collections; import java.time.Instant; -import java.util.Collection; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -33,6 +32,7 @@ import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.TimeSource; import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; import org.apache.solr.embedded.JettySolrRunner; @@ -85,12 +85,15 @@ public class SimpleCollectionCreateDeleteTest extends AbstractFullDistribZkTestB boolean allContainersEmpty = true; for (JettySolrRunner jetty : jettys) { - Collection<SolrCore> cores = jetty.getCoreContainer().getCores(); - for (SolrCore core : cores) { - CoreDescriptor cd = core.getCoreDescriptor(); - if (cd != null) { - if (cd.getCloudDescriptor().getCollectionName().equals(collectionName)) { - allContainersEmpty = false; + CoreContainer coreContainer = jetty.getCoreContainer(); + for (String coreName : coreContainer.getLoadedCoreNames()) { + try (SolrCore core = coreContainer.getCore(coreName)) { + if (core == null) continue; // unloaded since getLoadedCoreNames + CoreDescriptor cd = core.getCoreDescriptor(); + if (cd != null) { + if (cd.getCloudDescriptor().getCollectionName().equals(collectionName)) { + allContainersEmpty = false; + } } } } 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 a2ad9385c8f..ff85a5e4486 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java +++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java @@ -253,18 +253,18 @@ public class TestCoreContainer extends SolrTestCaseJ4 { try { // assert zero cores - assertEquals("There should not be cores", 0, cores.getCores().size()); + assertEquals("There should not be cores", 0, cores.getLoadedCoreNames().size()); // add a new core cores.create("core1", Map.of("configSet", "minimal")); // assert one registered core - assertEquals("There core registered", 1, cores.getCores().size()); + assertEquals("There core registered", 1, cores.getLoadedCoreNames().size()); cores.unload("core1"); // assert cero cores - assertEquals("There should not be cores", 0, cores.getCores().size()); + assertEquals("There should not be cores", 0, cores.getLoadedCoreNames().size()); // try and remove a core that does not exist SolrException thrown = diff --git a/solr/core/src/test/org/apache/solr/core/TimeAllowedTest.java b/solr/core/src/test/org/apache/solr/core/TimeAllowedTest.java index 7a88a470a90..a0c7b119577 100644 --- a/solr/core/src/test/org/apache/solr/core/TimeAllowedTest.java +++ b/solr/core/src/test/org/apache/solr/core/TimeAllowedTest.java @@ -81,8 +81,8 @@ public class TimeAllowedTest extends SolrTestCaseJ4 { // be doing something along the lines of this (but we lack api for it) // // SolrCores solrCores = ExitableDirectoryReaderTest.h.getCoreContainer().solrCores; - // List<SolrCore> cores = solrCores.getCores(); - // for (SolrCore core : cores) { + // for (String coreName : solrCores.getLoadedCoreNames()) { + // try (SolrCore core = ... .getCore(coreName)) { // if (<<< find the right core >>> ) { // ((SolrCache)core.getSearcher().get().<<<check cache for a key like name:a* >>> // } diff --git a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java index 8f6208449da..deed5251cf1 100644 --- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java +++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java @@ -132,11 +132,7 @@ public class TestContainerPlugin extends SolrCloudTestCase { for (JettySolrRunner jetty : cluster.getJettySolrRunners()) { CoreContainer cc = jetty.getCoreContainer(); cc.getContainerPluginsRegistry().setPhaser(phaser); - cc.getCores() - .forEach( - c -> { - c.getPackageListeners().addListener(listener); - }); + cc.forEachLoadedCore(c -> c.getPackageListeners().addListener(listener)); } } diff --git a/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java b/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java index 7f45190869a..055d6ba5720 100644 --- a/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java +++ b/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java @@ -31,7 +31,6 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.Collection; import java.util.Date; import java.util.List; import java.util.Set; @@ -992,41 +991,43 @@ public class TestReplicationHandler extends SolrTestCaseJ4 { checkForSingleIndex(jetty, false); } - private void checkForSingleIndex(JettySolrRunner jetty, boolean afterReload) throws IOException { + private void checkForSingleIndex(JettySolrRunner jetty, boolean afterReload) { CoreContainer cores = jetty.getCoreContainer(); - Collection<SolrCore> theCores = cores.getCores(); - for (SolrCore core : theCores) { - String ddir = core.getDataDir(); - CachingDirectoryFactory dirFactory = getCachingDirectoryFactory(core); - synchronized (dirFactory) { - Set<String> livePaths = dirFactory.getLivePaths(); - // one for data, one for the index under data and one for the snapshot metadata. - // we also allow one extra index dir - it may not be removed until the core is closed - if (afterReload) { - assertTrue( - livePaths.toString() + ":" + livePaths.size(), - 3 == livePaths.size() || 4 == livePaths.size()); - } else { - assertEquals(livePaths.toString() + ":" + livePaths.size(), 3, livePaths.size()); - } - - // :TODO: assert that one of the paths is a subpath of hte other - } - if (dirFactory instanceof StandardDirectoryFactory) { - try (Stream<Path> files = Files.list(Path.of(ddir))) { - List<Path> filesList = files.toList(); - System.out.println(filesList); - // we also allow one extra index dir - it may not be removed until the core is closed - int cnt = indexDirCount(ddir); - // if after reload, there may be 2 index dirs while the reloaded SolrCore closes. - if (afterReload) { - assertTrue("found:" + cnt + filesList, 1 == cnt || 2 == cnt); - } else { - assertEquals("found:" + cnt + filesList, 1, cnt); + cores.forEachLoadedCore( + core -> { + String ddir = core.getDataDir(); + CachingDirectoryFactory dirFactory = getCachingDirectoryFactory(core); + synchronized (dirFactory) { + Set<String> livePaths = dirFactory.getLivePaths(); + // one for data, one for the index under data and one for the snapshot metadata. + // we also allow one extra index dir - it may not be removed until the core is closed + if (afterReload) { + assertTrue( + livePaths.toString() + ":" + livePaths.size(), + 3 == livePaths.size() || 4 == livePaths.size()); + } else { + assertEquals(livePaths.toString() + ":" + livePaths.size(), 3, livePaths.size()); + } + + // :TODO: assert that one of the paths is a subpath of hte other } - } - } - } + if (dirFactory instanceof StandardDirectoryFactory) { + try (Stream<Path> files = Files.list(Path.of(ddir))) { + List<Path> filesList = files.toList(); + System.out.println(filesList); + // we also allow one extra index dir - it may not be removed until the core is closed + int cnt = indexDirCount(ddir); + // if after reload, there may be 2 index dirs while the reloaded SolrCore closes. + if (afterReload) { + assertTrue("found:" + cnt + filesList, 1 == cnt || 2 == cnt); + } else { + assertEquals("found:" + cnt + filesList, 1, cnt); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + }); } private int indexDirCount(String ddir) throws IOException { diff --git a/solr/core/src/test/org/apache/solr/update/PeerSyncWithBufferUpdatesTest.java b/solr/core/src/test/org/apache/solr/update/PeerSyncWithBufferUpdatesTest.java index d3932f7a304..f2b9a54ccc1 100644 --- a/solr/core/src/test/org/apache/solr/update/PeerSyncWithBufferUpdatesTest.java +++ b/solr/core/src/test/org/apache/solr/update/PeerSyncWithBufferUpdatesTest.java @@ -34,6 +34,7 @@ import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; +import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; import org.apache.solr.update.processor.DistributedUpdateProcessor; import org.junit.Test; @@ -82,8 +83,11 @@ public class PeerSyncWithBufferUpdatesTest extends BaseDistributedSearchTestCase } // it restarted and must do PeerSync - SolrCore jetty1Core = jettys.get(1).getCoreContainer().getCores().iterator().next(); - jetty1Core.getUpdateHandler().getUpdateLog().bufferUpdates(); + CoreContainer jetty1Cores = jettys.get(1).getCoreContainer(); + String jetty1CoreName = jetty1Cores.getLoadedCoreNames().get(0); + try (SolrCore jetty1Core = jetty1Cores.getCore(jetty1CoreName)) { + jetty1Core.getUpdateHandler().getUpdateLog().bufferUpdates(); + } for (int i = 16; i <= 20; i++) { add(client0, seenLeader, sdoc("id", String.valueOf(i), "_version_", ++v)); add(client1, seenLeader, sdoc("id", String.valueOf(i), "_version_", v)); @@ -105,7 +109,9 @@ public class PeerSyncWithBufferUpdatesTest extends BaseDistributedSearchTestCase add(client1, seenLeader, sdoc("id", "22", "_version_", v - 1)); log.info("Apply buffered updates"); - jetty1Core.getUpdateHandler().getUpdateLog().applyBufferedUpdates().get(); + try (SolrCore jetty1Core = jetty1Cores.getCore(jetty1CoreName)) { + jetty1Core.getUpdateHandler().getUpdateLog().applyBufferedUpdates().get(); + } for (int i = 1; i <= 23; i++) docsAdded.add(i); @@ -121,7 +127,9 @@ public class PeerSyncWithBufferUpdatesTest extends BaseDistributedSearchTestCase } log.info("After buffer updates"); - jetty1Core.getUpdateHandler().getUpdateLog().bufferUpdates(); + try (SolrCore jetty1Core = jetty1Cores.getCore(jetty1CoreName)) { + jetty1Core.getUpdateHandler().getUpdateLog().bufferUpdates(); + } Set<Integer> docIds = new HashSet<>(); for (int i = 0; i <= 50; i++) { diff --git a/solr/modules/ltr/src/test/org/apache/solr/ltr/AbstractLTRSolrCloudTestBase.java b/solr/modules/ltr/src/test/org/apache/solr/ltr/AbstractLTRSolrCloudTestBase.java index 1cbba422526..42af13ba686 100644 --- a/solr/modules/ltr/src/test/org/apache/solr/ltr/AbstractLTRSolrCloudTestBase.java +++ b/solr/modules/ltr/src/test/org/apache/solr/ltr/AbstractLTRSolrCloudTestBase.java @@ -73,9 +73,9 @@ public abstract class AbstractLTRSolrCloudTestBase extends TestRerankBase { createCollection(COLLECTION, "conf1", numShards, numReplicas); indexDocuments(COLLECTION); for (JettySolrRunner solrRunner : solrCluster.getJettySolrRunners()) { - if (!solrRunner.getCoreContainer().getCores().isEmpty()) { - String coreName = solrRunner.getCoreContainer().getCores().iterator().next().getName(); - restTestHarness = solrRunner.getRestClient(coreName); + List<String> coreNames = solrRunner.getCoreContainer().getLoadedCoreNames(); + if (!coreNames.isEmpty()) { + restTestHarness = solrRunner.getRestClient(coreNames.get(0)); break; } } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java index 5a3afd70ed0..7a62af77d0e 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java @@ -1871,11 +1871,11 @@ public abstract class AbstractFullDistribZkTestBase extends BaseDistributedSearc for (List<CloudJettyRunner> jettyList : shardToJetty.values()) { for (CloudJettyRunner jetty : jettyList) { CoreContainer cores = jetty.jetty.getCoreContainer(); - for (SolrCore core : cores.getCores()) { - ((DirectUpdateHandler2) core.getUpdateHandler()) - .getSoftCommitTracker() - .setTimeUpperBound(time); - } + cores.forEachLoadedCore( + core -> + ((DirectUpdateHandler2) core.getUpdateHandler()) + .getSoftCommitTracker() + .setTimeUpperBound(time)); } } } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java index bc56d66cd6e..161760e3e70 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java @@ -571,7 +571,7 @@ public class MiniSolrCloudCluster implements SolrBackend { boolean allContainersEmpty = true; for (JettySolrRunner jetty : jettys) { CoreContainer cc = jetty.getCoreContainer(); - if (cc != null && cc.getCores().size() != 0) { + if (cc != null && !cc.getLoadedCoreNames().isEmpty()) { allContainersEmpty = false; } } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCollectionsAPIDistributedZkTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCollectionsAPIDistributedZkTestBase.java index cc64a27a865..59a66bec4b2 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCollectionsAPIDistributedZkTestBase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCollectionsAPIDistributedZkTestBase.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -503,22 +502,26 @@ public abstract class AbstractCollectionsAPIDistributedZkTestBase extends SolrCl assertTrue("some core start times did not change on reload", allTimesAreCorrect); } - private void checkInstanceDirs(JettySolrRunner jetty) throws IOException { + private void checkInstanceDirs(JettySolrRunner jetty) { CoreContainer cores = jetty.getCoreContainer(); - Collection<SolrCore> theCores = cores.getCores(); - for (SolrCore core : theCores) { - // look for core props file - Path instancedir = core.getInstancePath(); - assertTrue( - "Could not find expected core.properties file", - Files.exists(instancedir.resolve("core.properties"))); - - Path expected = Path.of(jetty.getSolrHome()).resolve(core.getName()); - - assertTrue( - "Expected: " + expected + "\nFrom core stats: " + instancedir, - Files.isSameFile(expected, instancedir)); - } + cores.forEachLoadedCore( + core -> { + // look for core props file + Path instancedir = core.getInstancePath(); + assertTrue( + "Could not find expected core.properties file", + Files.exists(instancedir.resolve("core.properties"))); + + Path expected = Path.of(jetty.getSolrHome()).resolve(core.getName()); + + try { + assertTrue( + "Expected: " + expected + "\nFrom core stats: " + instancedir, + Files.isSameFile(expected, instancedir)); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); } private boolean waitForReloads(String collectionName, Map<String, Long> urlToTimeBefore) diff --git a/solr/test-framework/src/test/org/apache/solr/cloud/MiniSolrCloudClusterTest.java b/solr/test-framework/src/test/org/apache/solr/cloud/MiniSolrCloudClusterTest.java index c5f5bb51043..2083046c514 100644 --- a/solr/test-framework/src/test/org/apache/solr/cloud/MiniSolrCloudClusterTest.java +++ b/solr/test-framework/src/test/org/apache/solr/cloud/MiniSolrCloudClusterTest.java @@ -28,6 +28,7 @@ import java.util.concurrent.atomic.AtomicInteger; import org.apache.lucene.tests.util.LuceneTestCase; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; import org.apache.solr.embedded.JettyConfig; import org.apache.solr.embedded.JettySolrRunner; @@ -139,10 +140,12 @@ public class MiniSolrCloudClusterTest extends SolrTestCaseJ4 { CollectionAdminRequest.createCollection("test", 1, 1) .process(cluster.getSolrClient()) .isSuccess()); - final SolrCore core = jetty.getCoreContainer().getCores().get(0); - assertTrue( - core.getInstancePath() + " vs " + workDir, core.getInstancePath().startsWith(workDir)); - assertEquals(core.getInstancePath(), core.getResourceLoader().getInstancePath()); + final CoreContainer cc = jetty.getCoreContainer(); + try (SolrCore core = cc.getCore(cc.getLoadedCoreNames().get(0))) { + assertTrue( + core.getInstancePath() + " vs " + workDir, core.getInstancePath().startsWith(workDir)); + assertEquals(core.getInstancePath(), core.getResourceLoader().getInstancePath()); + } } finally { cluster.shutdown(); }
