From 1e643fec06ee53b975ab0cbb451af42506a89043 Mon Sep 17 00:00:00 2001 From: Pierre Salagnac Date: Wed, 15 Jan 2025 21:36:43 +0100 Subject: [PATCH 1/5] SOLR-17581: Less ZK watches in tests. Introduce new test variant of waitForState(), that does not wait on live node changes when we're only interested in the collection state. --- solr/CHANGES.txt | 3 ++ .../solr/cloud/CollectionStateZnodeTest.java | 3 +- .../solr/cloud/CollectionsAPISolrJTest.java | 20 ++++---- .../solr/cloud/DeleteInactiveReplicaTest.java | 6 +-- .../DeleteLastCustomShardedReplicaTest.java | 4 +- .../apache/solr/cloud/DeleteReplicaTest.java | 27 +++------- .../apache/solr/cloud/DeleteShardTest.java | 12 ++--- ...stribDocExpirationUpdateProcessorTest.java | 11 ++-- .../cloud/LeaderElectionIntegrationTest.java | 2 +- .../solr/cloud/LeaderTragicEventTest.java | 4 +- .../solr/cloud/LeaderVoteWaitTimeoutTest.java | 4 +- .../solr/cloud/MigrateRouteKeyTest.java | 2 +- .../solr/cloud/ReindexCollectionTest.java | 21 +++----- .../org/apache/solr/cloud/SplitShardTest.java | 12 ++--- .../solr/cloud/TestCloudConsistency.java | 10 ++-- .../apache/solr/cloud/TestCloudRecovery2.java | 2 +- .../solr/cloud/TestCloudSearcherWarming.java | 6 +-- .../TestDeleteCollectionOnDownNodes.java | 3 +- .../apache/solr/cloud/TestPullReplica.java | 3 +- .../solr/cloud/TestRebalanceLeaders.java | 14 +++--- .../solr/cloud/TestTlogReplayVsRecovery.java | 4 +- .../apache/solr/cloud/TestTlogReplica.java | 8 +-- .../apache/solr/cloud/ZkControllerTest.java | 4 +- .../api/collections/CollectionReloadTest.java | 2 +- .../CollectionTooManyReplicasTest.java | 2 +- .../CollectionsAPIAsyncDistributedZkTest.java | 35 ++++++------- .../api/collections/CustomCollectionTest.java | 6 +-- .../api/collections/TestCollectionAPI.java | 2 +- .../maintenance/InactiveShardRemoverTest.java | 4 +- .../solr/search/TestCoordinatorRole.java | 8 +-- .../RoutedAliasUpdateProcessorTest.java | 2 +- .../hdfs/cloud/HdfsCollectionsApiTest.java | 4 +- .../PerReplicaStatesIntegrationTest.java | 50 ++++++++----------- .../apache/solr/cloud/SolrCloudTestCase.java | 48 +++++++++++++++++- 34 files changed, 177 insertions(+), 171 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 4c5bf3b5faa..702d99ab953 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -187,6 +187,9 @@ Other Changes * SOLR-17589: Prevent error log entry on solr server due to initial HEAD request from HttpJdkSolrClient. (Paul Blanchaert via James Dyer) +* SOLR-17581: Introduce new test variant of waitForState(), that does not wait on live node changes when we're only + interested in the collection state. (Pierre Salagnac) + ================== 9.8.0 ================== New Features --------------------- diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionStateZnodeTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionStateZnodeTest.java index 0c43b93ea80..454af307c97 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CollectionStateZnodeTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CollectionStateZnodeTest.java @@ -16,6 +16,7 @@ */ package org.apache.solr.cloud; +import java.util.Objects; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.common.cloud.DocCollection; import org.apache.zookeeper.data.Stat; @@ -64,7 +65,7 @@ public void testZkNodeLocation() throws Exception { // remove collection CollectionAdminRequest.deleteCollection(collectionName).process(cluster.getSolrClient()); - waitForState("Collection not deleted", collectionName, (n, coll) -> coll == null); + waitForState("Collection not deleted", collectionName, Objects::isNull); assertFalse( "collection state should not exist", diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java index 5239deeaeac..d1cec38fbf6 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java @@ -132,7 +132,7 @@ public void testCreateWithDefaultConfigSet() throws Exception { waitForState( "Expected " + collectionName + " to disappear from cluster state", collectionName, - (n, c) -> c == null); + Objects::isNull); } @Test @@ -260,7 +260,7 @@ public void testCreateAndDeleteCollection() throws Exception { waitForState( "Expected " + collectionName + " to disappear from cluster state", collectionName, - (n, c) -> c == null); + Objects::isNull); // Test Creating a new collection. collectionName = "solrj_test2"; @@ -274,7 +274,7 @@ public void testCreateAndDeleteCollection() throws Exception { waitForState( "Expected " + collectionName + " to appear in cluster state", collectionName, - (n, c) -> c != null); + Objects::nonNull); } @Test @@ -423,9 +423,7 @@ public void testSplitShard() throws Exception { assertTrue(response.isSuccess()); waitForState( - "Expected 5 slices to be active", - collectionName, - (n, c) -> c.getActiveSlices().size() == 5); + "Expected 5 slices to be active", collectionName, c -> c.getActiveSlices().size() == 5); } @Test @@ -493,7 +491,7 @@ public void testAddAndDeleteReplica() throws Exception { waitForState( "Expected replica " + newReplica.getName() + " to vanish from cluster state", collectionName, - (n, c) -> c.getSlice("shard1").getReplica(newReplica.getName()) == null); + c -> c.getSlice("shard1").getReplica(newReplica.getName()) == null); } private Replica grabNewReplica(CollectionAdminResponse response, DocCollection docCollection) { @@ -1277,7 +1275,7 @@ public void testAddAndDeleteReplicaProp() throws IOException, SolrServerExceptio waitForState( "Expecting property 'preferredleader' to appear on replica " + replica.getName(), collection, - (n, c) -> "true".equals(c.getReplica(replica.getName()).getProperty("preferredleader"))); + c -> "true".equals(c.getReplica(replica.getName()).getProperty("preferredleader"))); response = CollectionAdminRequest.deleteReplicaProperty( @@ -1288,7 +1286,7 @@ public void testAddAndDeleteReplicaProp() throws IOException, SolrServerExceptio waitForState( "Expecting property 'preferredleader' to be removed from replica " + replica.getName(), collection, - (n, c) -> c.getReplica(replica.getName()).getProperty("preferredleader") == null); + c -> c.getReplica(replica.getName()).getProperty("preferredleader") == null); } @Test @@ -1308,7 +1306,7 @@ public void testBalanceShardUnique() throws IOException, SolrServerException { waitForState( "Expecting 'preferredleader' property to be balanced across all shards", collection, - (n, c) -> { + c -> { for (Slice slice : c) { int count = 0; for (Replica replica : slice) { @@ -1335,7 +1333,7 @@ public void testModifyCollectionAttribute() throws IOException, SolrServerExcept waitForState( "Expecting attribute 'replicationFactor' to be 25", collection, - (n, c) -> 25 == c.getReplicationFactor()); + c -> 25 == c.getReplicationFactor()); expectThrows( IllegalArgumentException.class, diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java index 33bab69aeae..2d1b3564527 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DeleteInactiveReplicaTest.java @@ -82,7 +82,7 @@ public void deleteInactiveReplicaTest() throws Exception { waitForState( "Expected replica " + replica.getName() + " on down node to be removed from cluster state", collectionName, - (n, c) -> { + c -> { Replica r = c.getReplica(replica.getName()); return r == null || r.getState() != Replica.State.ACTIVE; }); @@ -95,9 +95,7 @@ public void deleteInactiveReplicaTest() throws Exception { waitForState( "Expected deleted replica " + replica.getName() + " to be removed from cluster state", collectionName, - (n, c) -> { - return c.getReplica(replica.getName()) == null; - }); + c -> c.getReplica(replica.getName()) == null); cluster.startJettySolrRunner(jetty); log.info("restarted jetty"); diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteLastCustomShardedReplicaTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteLastCustomShardedReplicaTest.java index b6883a22af1..25c1a89fc3f 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DeleteLastCustomShardedReplicaTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DeleteLastCustomShardedReplicaTest.java @@ -46,8 +46,6 @@ public void test() throws Exception { waitForState( "Expected shard 'a' to have no replicas", collectionName, - (n, c) -> { - return c.getSlice("a") == null || c.getSlice("a").getReplicas().size() == 0; - }); + c -> c.getSlice("a") == null || c.getSlice("a").getReplicas().isEmpty()); } } diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java index 16242bcc5eb..7984ca47e76 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java @@ -129,7 +129,7 @@ public void deleteLiveReplicaTest() throws Exception { waitForState( "Expected replica " + replica.getName() + " to have been removed", collectionName, - (n, c) -> { + c -> { Slice testShard = c.getSlice(shard.getName()); return testShard.getReplica(replica.getName()) == null; }); @@ -286,8 +286,7 @@ public void deleteReplicaFromClusterState() throws Exception { waitForState( "Timeout waiting for replica get deleted", collectionName, - (liveNodes, collectionState) -> - collectionState.getSlice("shard1").getReplicas().size() == 2); + collectionState -> collectionState.getSlice("shard1").getReplicas().size() == 2); TimeOut timeOut = new TimeOut(60, TimeUnit.SECONDS, TimeSource.NANO_TIME); timeOut.waitFor( @@ -430,7 +429,7 @@ public void raceConditionOnDeleteAndRegisterReplica() throws Exception { waitForState( "Expected replica:" + replica1 + " get down", collectionName, - (liveNodes, collectionState) -> + collectionState -> collectionState.getSlice("shard1").getReplica(replica1.getName()).getState() == DOWN); } @@ -465,7 +464,7 @@ public void raceConditionOnDeleteAndRegisterReplica() throws Exception { waitForState( "Expected new active leader", collectionName, - (liveNodes, collectionState) -> { + collectionState -> { Slice shard = collectionState.getSlice("shard1"); Replica newLeader = shard.getLeader(); return newLeader != null @@ -549,20 +548,10 @@ public void deleteReplicaOnIndexing() throws Exception { thread.join(); } - try { - cluster - .getZkStateReader() - .waitForState( - collectionName, - 20, - TimeUnit.SECONDS, - (liveNodes, collectionState) -> collectionState.getReplicas().size() == 1); - } catch (TimeoutException e) { - if (log.isInfoEnabled()) { - log.info("Timeout wait for state {}", getCollectionState(collectionName)); - } - throw e; - } + waitForState( + "Waiting for single replica in state", + collectionName, + collectionState -> collectionState.getReplicas().size() == 1); } /** diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java index f363bb58068..73375d8981e 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java @@ -67,14 +67,12 @@ public void test() throws Exception { // Can delete an INACTIVE shard CollectionAdminRequest.deleteShard(collection, "shard1").process(cluster.getSolrClient()); - waitForState( - "Expected 'shard1' to be removed", collection, (n, c) -> c.getSlice("shard1") == null); + waitForState("Expected 'shard1' to be removed", collection, c -> c.getSlice("shard1") == null); // Can delete a shard under construction setSliceState(collection, "shard2", Slice.State.CONSTRUCTION); CollectionAdminRequest.deleteShard(collection, "shard2").process(cluster.getSolrClient()); - waitForState( - "Expected 'shard2' to be removed", collection, (n, c) -> c.getSlice("shard2") == null); + waitForState("Expected 'shard2' to be removed", collection, c -> c.getSlice("shard2") == null); } @Test @@ -100,7 +98,7 @@ public void testDirectoryCleanupAfterDeleteShard() throws IOException, SolrServe // Delete shard 'a' CollectionAdminRequest.deleteShard(collection, "a").process(cluster.getSolrClient()); - waitForState("Expected 'a' to be removed", collection, (n, c) -> c.getSlice("a") == null); + waitForState("Expected 'a' to be removed", collection, c -> c.getSlice("a") == null); assertEquals(2, getCollectionState(collection).getActiveSlices().size()); assertFalse( @@ -116,7 +114,7 @@ public void testDirectoryCleanupAfterDeleteShard() throws IOException, SolrServe .setDeleteInstanceDir(false) .process(cluster.getSolrClient()); - waitForState("Expected 'b' to be removed", collection, (n, c) -> c.getSlice("b") == null); + waitForState("Expected 'b' to be removed", collection, c -> c.getSlice("b") == null); assertEquals(1, getCollectionState(collection).getActiveSlices().size()); assertTrue( @@ -130,6 +128,6 @@ private void setSliceState(String collectionName, String shardId, Slice.State st waitForState( "Expected shard " + shardId + " to be in state " + state, collectionName, - (n, c) -> c.getSlice(shardId).getState() == state); + c -> c.getSlice(shardId).getState() == state); } } diff --git a/solr/core/src/test/org/apache/solr/cloud/DistribDocExpirationUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/cloud/DistribDocExpirationUpdateProcessorTest.java index dfd9aa4dc14..b0a6419b233 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DistribDocExpirationUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DistribDocExpirationUpdateProcessorTest.java @@ -97,13 +97,10 @@ public void setupCluster(boolean security) throws Exception { setAuthIfNeeded(CollectionAdminRequest.createCollection(COLLECTION, "conf", 2, 2)) .process(cluster.getSolrClient()); - cluster - .getZkStateReader() - .waitForState( - COLLECTION, - DEFAULT_TIMEOUT, - TimeUnit.SECONDS, - (n, c) -> DocCollection.isFullyActive(n, c, 2, 2)); + waitForState( + "Waiting for collection creation", + COLLECTION, + (n, c) -> DocCollection.isFullyActive(n, c, 2, 2)); } @Test diff --git a/solr/core/src/test/org/apache/solr/cloud/LeaderElectionIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/LeaderElectionIntegrationTest.java index 32f921747d9..f2ca9c01367 100644 --- a/solr/core/src/test/org/apache/solr/cloud/LeaderElectionIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/LeaderElectionIntegrationTest.java @@ -84,7 +84,7 @@ public void testSimpleSliceLeaderElection() throws Exception { waitForState( "Leader should not be " + jettyNodeName, collection, - (n, c) -> + c -> c.getLeader("shard1") != null && !jettyNodeName.equals(c.getLeader("shard1").getNodeName())); } diff --git a/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java b/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java index 4fba87d940b..38d67942591 100644 --- a/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java @@ -88,7 +88,7 @@ public void testLeaderFailsOver() throws Exception { waitForState( "Now waiting for new replica to become leader", collection, - (liveNodes, collectionState) -> { + collectionState -> { Slice slice = collectionState.getSlice("shard1"); if (slice.getReplicas().size() != 2) return false; @@ -176,7 +176,7 @@ public void testOtherReplicasAreNotActive() throws Exception { waitForState( "Timeout waiting for replica get down", collection, - (liveNodes, collectionState) -> + collectionState -> getNonLeader(collectionState.getSlice("shard1")).getState() != Replica.State.ACTIVE); } diff --git a/solr/core/src/test/org/apache/solr/cloud/LeaderVoteWaitTimeoutTest.java b/solr/core/src/test/org/apache/solr/cloud/LeaderVoteWaitTimeoutTest.java index a54481b1f5e..7f64a5d8a94 100644 --- a/solr/core/src/test/org/apache/solr/cloud/LeaderVoteWaitTimeoutTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/LeaderVoteWaitTimeoutTest.java @@ -156,7 +156,7 @@ public void basicTest() throws Exception { waitForState( "Timeout waiting for replica win the election", collectionName, - (liveNodes, collectionState) -> { + collectionState -> { Replica newLeader = collectionState.getSlice("shard1").getLeader(); if (newLeader == null) { return false; @@ -268,7 +268,7 @@ public void testMostInSyncReplicasCanWinElection() throws Exception { waitForState( "Timeout waiting for new leader", collectionName, - (liveNodes, collectionState) -> { + collectionState -> { Replica newLeader = collectionState.getSlice("shard1").getLeader(); if (newLeader == null) { return false; diff --git a/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java b/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java index fcbec76bfbb..c01d9b3f242 100644 --- a/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java @@ -171,7 +171,7 @@ public void multipleShardMigrateTest() throws Exception { waitForState( "Expected to find routing rule for split key " + splitKey, sourceCollection, - (n, c) -> { + c -> { if (c == null) return false; Slice shard = c.getSlice("shard2"); if (shard == null) return false; diff --git a/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java b/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java index 0b79aa1c336..4a230e3bb5e 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java @@ -151,7 +151,7 @@ public void testBasicReindexing() throws Exception { waitForState( "did not finish copying in time", targetCollection, - (liveNodes, coll) -> { + coll -> { ReindexCollectionCmd.State state = ReindexCollectionCmd.State.get(coll.getStr(ReindexCollectionCmd.REINDEXING_STATE)); return ReindexCollectionCmd.State.FINISHED == state; @@ -207,7 +207,7 @@ private void doTestSameTargetReindexing(boolean sourceRemove, boolean followAlia waitForState( "did not finish copying in time", realTargetCollection, - (liveNodes, coll) -> { + coll -> { ReindexCollectionCmd.State state = ReindexCollectionCmd.State.get(coll.getStr(ReindexCollectionCmd.REINDEXING_STATE)); return ReindexCollectionCmd.State.FINISHED == state; @@ -248,7 +248,7 @@ public void testLossySchema() throws Exception { waitForState( "did not finish copying in time", targetCollection, - (liveNodes, coll) -> { + coll -> { ReindexCollectionCmd.State state = ReindexCollectionCmd.State.get(coll.getStr(ReindexCollectionCmd.REINDEXING_STATE)); return ReindexCollectionCmd.State.FINISHED == state; @@ -291,7 +291,7 @@ public void testReshapeReindexing() throws Exception { waitForState( "did not finish copying in time", targetCollection, - (liveNodes, coll) -> { + coll -> { ReindexCollectionCmd.State state = ReindexCollectionCmd.State.get(coll.getStr(ReindexCollectionCmd.REINDEXING_STATE)); return ReindexCollectionCmd.State.FINISHED == state; @@ -381,7 +381,7 @@ public void testFailure() throws Exception { waitForState( "collection state is incorrect", sourceCollection, - (liveNodes, collectionState) -> + collectionState -> !collectionState.isReadOnly() && collectionState.getStr(ReindexCollectionCmd.REINDEXING_STATE) == null); waitForReindexingState(sourceCollection, null); @@ -400,9 +400,7 @@ public void testAbort() throws Exception { String asyncId = req.processAsync(solrClient); // wait for the source collection to be put in readOnly mode waitForState( - "source collection didn't become readOnly", - sourceCollection, - (liveNodes, coll) -> coll.isReadOnly()); + "source collection didn't become readOnly", sourceCollection, DocCollection::isReadOnly); req = CollectionAdminRequest.reindexCollection(sourceCollection); req.setCommand("abort"); @@ -412,10 +410,7 @@ public void testAbort() throws Exception { assertNotNull(rsp.toString(), status); assertEquals(status.toString(), "aborting", status.get("state")); - waitForState( - "incorrect collection state", - sourceCollection, - (liveNodes, collectionState) -> collectionState.isReadOnly()); + waitForState("incorrect collection state", sourceCollection, DocCollection::isReadOnly); waitForReindexingState(sourceCollection, ReindexCollectionCmd.State.ABORTED); // verify status @@ -429,7 +424,7 @@ public void testAbort() throws Exception { waitForState( "source collection is in wrong state", sourceCollection, - (liveNodes, docCollection) -> !docCollection.isReadOnly()); + docCollection -> !docCollection.isReadOnly()); waitForReindexingState(sourceCollection, null); // verify the response diff --git a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java index ab194b291da..07c7ce0377e 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java @@ -357,14 +357,10 @@ public void testShardSplitWithNodeset() throws Exception { NamedList response = splitShard.process(cluster.getSolrClient()).getResponse(); assertNotNull(response.get("success")); - cluster - .getZkStateReader() - .waitForState( - COLL, - 10, - TimeUnit.SECONDS, - (liveNodes, collectionState) -> - testColl(jetty, collectionState, List.of("shard1_0", "shard1_1"))); + waitForState( + "Waiting for sub-shards", + COLL, + collectionState -> testColl(jetty, collectionState, List.of("shard1_0", "shard1_1"))); JettySolrRunner randomJetty = cluster.getRandomJetty(random()); splitShard = diff --git a/solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java b/solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java index a1f1470d668..009a1d48d07 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestCloudConsistency.java @@ -157,7 +157,7 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader, waitForState( "", collection, - (liveNodes, collectionState) -> + collectionState -> collectionState.getSlice("shard1").getReplicas().stream() .filter(replica -> replica.getState() == Replica.State.DOWN) .count() @@ -170,7 +170,7 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader, waitForState( "", collection, - (liveNodes, collectionState) -> + collectionState -> collectionState.getReplica(leader.getName()).getState() == Replica.State.DOWN); cluster.getJettySolrRunner(1).start(); @@ -213,7 +213,7 @@ private void addDocToWhenOtherReplicasAreDown(String collection, Replica leader, waitForState( "Timeout waiting for leader", collection, - (liveNodes, collectionState) -> { + collectionState -> { Replica newLeader = collectionState.getLeader("shard1"); return newLeader != null && newLeader.getName().equals(leader.getName()); }); @@ -240,7 +240,7 @@ private void addDocWhenOtherReplicasAreNetworkPartitioned( waitForState( "Timeout waiting for leader goes DOWN", collection, - (liveNodes, collectionState) -> + collectionState -> collectionState.getReplica(leader.getName()).getState() == Replica.State.DOWN); // the meat of the test -- wait to see if a different replica become a leader @@ -277,7 +277,7 @@ private void addDocWhenOtherReplicasAreNetworkPartitioned( waitForState( "Timeout waiting for leader", collection, - (liveNodes, collectionState) -> { + collectionState -> { Replica newLeader = collectionState.getLeader("shard1"); return newLeader != null && newLeader.getName().equals(leader.getName()); }); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery2.java b/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery2.java index 1e86167f299..3297529cfdf 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery2.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestCloudRecovery2.java @@ -159,7 +159,7 @@ public void test() throws Exception { waitForState( "", COLLECTION, - (liveNodes, collectionState) -> { + collectionState -> { Replica leader = collectionState.getLeader("shard1"); return leader != null && leader.getNodeName().equals(node2.getNodeName()); }); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestCloudSearcherWarming.java b/solr/core/src/test/org/apache/solr/cloud/TestCloudSearcherWarming.java index 99f61ce0c55..7f219b9b675 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestCloudSearcherWarming.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestCloudSearcherWarming.java @@ -21,12 +21,12 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Predicate; import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.common.cloud.CollectionStatePredicate; import org.apache.solr.common.cloud.CollectionStateWatcher; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Replica; @@ -213,8 +213,8 @@ public void testPeersyncFailureReplicationSuccess() throws Exception { "The collection should have 1 shard and 1 replica", collectionName, clusterShape(1, 1)); // the above call is not enough because we want to assert that the downed replica is not active // but clusterShape will also return true if replica is not live -- which we don't want - CollectionStatePredicate collectionStatePredicate = - (liveNodes, collectionState) -> { + Predicate collectionStatePredicate = + collectionState -> { for (Replica r : collectionState.getReplicas()) { if (r.getNodeName().equals(oldNodeName.get())) { return r.getState() == Replica.State.DOWN; diff --git a/solr/core/src/test/org/apache/solr/cloud/TestDeleteCollectionOnDownNodes.java b/solr/core/src/test/org/apache/solr/cloud/TestDeleteCollectionOnDownNodes.java index 316c4ee03f2..d66518578da 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestDeleteCollectionOnDownNodes.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestDeleteCollectionOnDownNodes.java @@ -17,6 +17,7 @@ package org.apache.solr.cloud; +import java.util.Objects; import java.util.concurrent.TimeUnit; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.embedded.JettySolrRunner; @@ -60,7 +61,7 @@ public void deleteCollectionWithDownNodes() throws Exception { waitForState( "Timed out waiting for collection to be deleted", "halfdeletedcollection2", - (n, c) -> c == null); + Objects::isNull); assertFalse( "Still found collection that should be gone", diff --git a/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java b/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java index ec0564c249d..1f333f5d4ab 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.http.HttpResponse; @@ -753,7 +754,7 @@ static void waitForDeletion(String collection) { waitForState( "Waiting for collection " + collection + " to be deleted", collection, - (n, c) -> c == null, + Objects::isNull, 10, TimeUnit.SECONDS); } diff --git a/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java b/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java index 20343487523..9fda9bbf4bd 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java @@ -179,7 +179,7 @@ private void doTestSetArbitraryPropertySliceUnique(String propIn) waitForState( "Check property is uniquely distributed in slice: " + prop, COLLECTION_NAME, - (n, c) -> { + c -> { forceUpdateCollectionStatus(); Slice modSlice = c.getSlice(slice.getName()); boolean rightRep = @@ -338,7 +338,7 @@ private void verifyPropCorrectlyDistributed(String prop) { waitForState( "Check property is distributed evenly: " + prop, COLLECTION_NAME, - (liveNodes, docCollection) -> { + docCollection -> { int maxPropCount = 0; int minPropCount = Integer.MAX_VALUE; for (Slice slice : docCollection.getSlices()) { @@ -372,7 +372,7 @@ private void verifyPropDistributedAsExpected( waitForState( message, COLLECTION_NAME, - (liveNodes, docCollection) -> { + docCollection -> { for (Map.Entry ent : expectedShardReplicaMap.entrySet()) { Replica rep = docCollection.getSlice(ent.getKey()).getReplica(ent.getValue()); if (rep.getBool("property." + propLC, false) == false) { @@ -486,7 +486,7 @@ void setPropWithStandardRequest(Slice slice, Replica rep, String prop) waitForState( "Expecting property '" + prop + "'to appear on replica " + rep.getName(), COLLECTION_NAME, - (n, c) -> "true".equals(c.getReplica(rep.getName()).getProperty(propLC))); + c -> "true".equals(c.getReplica(rep.getName()).getProperty(propLC))); } void setPropWithAdminRequest(Slice slice, Replica rep, String prop) @@ -504,7 +504,7 @@ void setPropWithAdminRequest(Slice slice, Replica rep, String prop) waitForState( "Expecting property '" + prop + "'to appear on replica " + rep.getName(), COLLECTION_NAME, - (n, c) -> "true".equals(c.getReplica(rep.getName()).getProperty(propLC))); + c -> "true".equals(c.getReplica(rep.getName()).getProperty(propLC))); } private void delProp(Slice slice, Replica rep, String prop) @@ -518,7 +518,7 @@ private void delProp(Slice slice, Replica rep, String prop) waitForState( "Expecting property '" + prop + "' to be removed from replica " + rep.getName(), COLLECTION_NAME, - (n, c) -> c.getReplica(rep.getName()).getProperty(prop) == null); + c -> c.getReplica(rep.getName()).getProperty(prop) == null); } // Intentionally un-balance the property to ensure that BALANCESHARDUNIQUE does its job. There was @@ -664,7 +664,7 @@ private void verifyPropUniquePerShard(String prop) { waitForState( "Waiting to have exactly one replica with " + prop + "set per shard", COLLECTION_NAME, - (liveNodes, docCollection) -> { + docCollection -> { for (Slice slice : docCollection.getSlices()) { int propCount = 0; for (Replica rep : slice.getReplicas()) { diff --git a/solr/core/src/test/org/apache/solr/cloud/TestTlogReplayVsRecovery.java b/solr/core/src/test/org/apache/solr/cloud/TestTlogReplayVsRecovery.java index 3a62bbc72f0..c45b57305ee 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestTlogReplayVsRecovery.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestTlogReplayVsRecovery.java @@ -176,7 +176,7 @@ public void testManyDocsInTlogReplayWhileReplicaIsTryingToRecover() throws Excep waitForState( "Timeout waiting for leader goes DOWN", COLLECTION, - (liveNodes, collectionState) -> + collectionState -> collectionState.getReplica(leader.getName()).getState() == Replica.State.DOWN); // Sanity check that a new (out of sync) replica doesn't come up in our place... @@ -216,7 +216,7 @@ public void testManyDocsInTlogReplayWhileReplicaIsTryingToRecover() throws Excep waitForState( "Timeout waiting for leader", COLLECTION, - (liveNodes, collectionState) -> { + collectionState -> { Replica newLeader = collectionState.getLeader("shard1"); return newLeader != null && newLeader.getName().equals(leader.getName()); }); 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 d559de6d333..480aefc019a 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java @@ -819,7 +819,7 @@ public void testRebalanceLeaders() throws Exception { waitForState( "Waiting for setting preferredleader flag", collectionName, - (n, c) -> { + c -> { Map slices = c.getSlicesMap(); Replica me = slices.get(slice.getName()).getReplica(newLeaderName); return me.getBool("property.preferredleader", false); @@ -840,7 +840,7 @@ public void testRebalanceLeaders() throws Exception { waitForState( "Waiting for a new leader to be elected", collectionName, - (n, c) -> { + c -> { Replica leader = c.getSlice(slice.getName()).getLeader(); return leader != null && leader.getName().equals(newLeaderName) @@ -864,7 +864,7 @@ private void waitForLeaderChange(JettySolrRunner oldLeaderJetty, String shardNam waitForState( "Expect new leader", collectionName, - (liveNodes, collectionState) -> { + collectionState -> { Replica leader = collectionState.getLeader(shardName); if (leader == null || !leader.isActive(cluster.getSolrClient().getClusterState().getLiveNodes())) { @@ -1030,7 +1030,7 @@ private void waitForDeletion(String collection) { waitForState( "Waiting for collection " + collection + " to be deleted", collection, - (n, c) -> c == null, + Objects::isNull, 10, TimeUnit.SECONDS); } diff --git a/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java b/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java index c5be38b570c..fae3fd1ef07 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java @@ -340,12 +340,12 @@ public List getCoreDescriptors() { collectionName, 10, TimeUnit.SECONDS, - ((liveNodes, collectionState) -> + collectionState -> Optional.ofNullable(collectionState) .map(DocCollection::getReplicas) .map(List::size) .orElse(0) - == 3)); + == 3); } Instant now = Instant.now(); diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java index a6c961f5820..f46e0a6225a 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java @@ -87,7 +87,7 @@ public void testReloadedLeaderStateAfterZkSessionLoss() throws Exception { waitForState( "Timed out waiting for core to re-register as ACTIVE after session expiry", testCollectionName, - (n, c) -> { + c -> { log.info("Collection state: {}", c); Replica expiredReplica = c.getReplica(leader.getName()); return expiredReplica.getState() == Replica.State.ACTIVE diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionTooManyReplicasTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionTooManyReplicasTest.java index 5dfd69e3b02..79cf1c444f0 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionTooManyReplicasTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionTooManyReplicasTest.java @@ -112,7 +112,7 @@ public void testAddTooManyReplicas() throws Exception { waitForState( "Expected to see all replicas active", collectionName, - (n, c) -> { + c -> { for (Replica r : c.getReplicas()) { if (r.getState() != Replica.State.ACTIVE) return false; } diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIAsyncDistributedZkTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIAsyncDistributedZkTest.java index ff64f01014f..535ae336392 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIAsyncDistributedZkTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIAsyncDistributedZkTest.java @@ -164,25 +164,22 @@ public void testAsyncRequests() throws Exception { assertSame("AddReplica did not complete", RequestStatusState.COMPLETED, state); // cloudClient watch might take a couple of seconds to reflect it - cluster - .getZkStateReader() - .waitForState( - collection, - 20, - TimeUnit.SECONDS, - (n, c) -> { - if (c == null) return false; - Slice slice = c.getSlice("shard1"); - if (slice == null) { - return false; - } - - if (slice.getReplicas().size() == 2) { - return true; - } - - return false; - }); + waitForState( + "Wait for replica in cluster state", + collection, + c -> { + if (c == null) return false; + Slice slice = c.getSlice("shard1"); + if (slice == null) { + return false; + } + + if (slice.getReplicas().size() == 2) { + return true; + } + + return false; + }); state = CollectionAdminRequest.createAlias("myalias", collection) diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CustomCollectionTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CustomCollectionTest.java index 1c1644394ee..2c2f4e8e2e2 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CustomCollectionTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CustomCollectionTest.java @@ -127,7 +127,7 @@ public void testCustomCollectionsAPI() throws Exception { waitForState( "Expected shard 'x' to be active", collection, - (n, c) -> { + c -> { if (c.getSlice("x") == null) return false; for (Replica r : c.getSlice("x")) { if (r.getState() != Replica.State.ACTIVE) return false; @@ -255,8 +255,6 @@ public void testCreateShardRepFactor() throws Exception { waitForState( "Not enough active replicas in shard 'x'", collectionName, - (n, c) -> { - return c.getSlice("x").getReplicas().size() == 1; - }); + c -> c.getSlice("x").getReplicas().size() == 1); } } diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java index 314655e85ce..0aa5d4ae2d0 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java @@ -377,7 +377,7 @@ private void clusterStatusWithCollectionHealthState() throws Exception { COLLECTION_NAME, 30, TimeUnit.SECONDS, - (liveNodes, docCollection) -> + docCollection -> docCollection != null && docCollection.getReplicas().stream() .anyMatch(r -> r.getState().equals(Replica.State.DOWN) && !r.isLeader())); diff --git a/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java b/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java index fb8f2b95715..3edb9bfcc3b 100644 --- a/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java +++ b/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java @@ -89,7 +89,7 @@ public void testTtl() throws Exception { waitForState( "Expected shard " + sliceName + " to be in state " + Slice.State.INACTIVE, collectionName, - (n, c) -> c.getSlice(sliceName).getState() == Slice.State.INACTIVE); + c -> c.getSlice(sliceName).getState() == Slice.State.INACTIVE); final long ttlStart = timeSource.getTimeNs(); @@ -216,7 +216,7 @@ private void setAllShardsInactive(final String collectionName) { waitForState( "Expected shard " + s + " to be in state " + Slice.State.INACTIVE, collection.getName(), - (n, c) -> c.getSlice(s.getName()).getState() == Slice.State.INACTIVE); + c -> c.getSlice(s.getName()).getState() == Slice.State.INACTIVE); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java index f75448e8b82..cafe7ed0268 100644 --- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java +++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java @@ -750,7 +750,7 @@ public void testSplitShard() throws Exception { waitForState( "Failed to wait for child shards after split", COLLECTION_NAME, - (liveNodes, collectionState) -> + collectionState -> collectionState.getSlice("shard1_0") != null && collectionState.getSlice("shard1_0").getState() == Slice.State.ACTIVE && collectionState.getSlice("shard1_1") != null @@ -761,7 +761,7 @@ public void testSplitShard() throws Exception { waitForState( "Parent shard is not yet deleted after split", COLLECTION_NAME, - (liveNodes, collectionState) -> collectionState.getSlice("shard1") == null); + collectionState -> collectionState.getSlice("shard1") == null); response = new QueryRequest(new SolrQuery("*:*")) @@ -796,7 +796,7 @@ public void testMoveReplica() throws Exception { waitForState( "Cannot find replica on first node yet", COLLECTION_NAME, - (liveNodes, collectionState) -> { + collectionState -> { if (collectionState.getReplicas().size() == 1) { Replica replica = collectionState.getReplicas().get(0); return fromNode.equals(replica.getNodeName()) @@ -837,7 +837,7 @@ public void testMoveReplica() throws Exception { waitForState( "Cannot find replica on second node yet after repliac move", COLLECTION_NAME, - (liveNodes, collectionState) -> { + collectionState -> { if (collectionState.getReplicas().size() == 1) { Replica replica = collectionState.getReplicas().get(0); return toNodeName.equals(replica.getNodeName()) diff --git a/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java index d2e442d0dd1..074598dc213 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/RoutedAliasUpdateProcessorTest.java @@ -284,7 +284,7 @@ void waitCol(int slices, String collection) { waitForState( "waiting for collections to be created", collection, - (liveNodes, collectionState) -> { + collectionState -> { if (collectionState == null) { // per predicate javadoc, this is what we get if the collection doesn't exist at all. return false; diff --git a/solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsCollectionsApiTest.java b/solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsCollectionsApiTest.java index c2afd069585..0b69ee4c8f8 100644 --- a/solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsCollectionsApiTest.java +++ b/solr/modules/hdfs/src/test/org/apache/solr/hdfs/cloud/HdfsCollectionsApiTest.java @@ -86,9 +86,9 @@ public void testDataDirIsNotReused() throws Exception { jettySolrRunner.stop(); waitForState( - "", + "Waiting for replica to be marked down", collection, - (liveNodes, collectionState) -> { + collectionState -> { Replica replica = collectionState.getSlice("shard1").getReplicas().iterator().next(); return replica.getState() == Replica.State.DOWN; }); diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java index ac7f4840734..1638e3e241b 100644 --- a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java @@ -245,7 +245,6 @@ public void testMultipleTransitions() throws Exception { .resolve("streaming") .resolve("conf")) .configure(); - PerReplicaStates original = null; try { CollectionAdminRequest.createCollection(COLL, "conf", 3, 1) .setPerReplicaState(Boolean.TRUE) @@ -253,42 +252,35 @@ public void testMultipleTransitions() throws Exception { cluster.waitForActiveCollection(COLL, 3, 3); PerReplicaStates prs1 = - original = - PerReplicaStatesOps.fetch( - DocCollection.getCollectionPath(COLL), cluster.getZkClient(), null); + PerReplicaStatesOps.fetch( + DocCollection.getCollectionPath(COLL), cluster.getZkClient(), null); log.info("prs1 : {}", prs1); CollectionAdminRequest.modifyCollection( COLL, Collections.singletonMap(PER_REPLICA_STATE, "false")) .process(cluster.getSolrClient()); - cluster - .getZkStateReader() - .waitForState( - COLL, - 5, - TimeUnit.SECONDS, - (liveNodes, collectionState) -> - "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE))); + waitForState( + "Waiting for PRS property", + COLL, + collectionState -> + "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE))); CollectionAdminRequest.modifyCollection( COLL, Collections.singletonMap(PER_REPLICA_STATE, "true")) .process(cluster.getSolrClient()); - cluster - .getZkStateReader() - .waitForState( - COLL, - 5, - TimeUnit.SECONDS, - (liveNodes, collectionState) -> { - AtomicBoolean anyFail = new AtomicBoolean(false); - PerReplicaStates prs2 = - PerReplicaStatesOps.fetch( - DocCollection.getCollectionPath(COLL), cluster.getZkClient(), null); - prs2.states.forEach( - (r, newState) -> { - if (newState.getDuplicate() != null) anyFail.set(true); - }); - return !anyFail.get(); - }); + waitForState( + "Waiting for PRS property", + COLL, + collectionState -> { + AtomicBoolean anyFail = new AtomicBoolean(false); + PerReplicaStates prs2 = + PerReplicaStatesOps.fetch( + DocCollection.getCollectionPath(COLL), cluster.getZkClient(), null); + prs2.states.forEach( + (r, newState) -> { + if (newState.getDuplicate() != null) anyFail.set(true); + }); + return !anyFail.get(); + }); } finally { cluster.shutdown(); diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java index d410c3abeb4..c517774cb05 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java @@ -160,6 +160,11 @@ protected static DocCollection getCollectionState(String collectionName) { return cluster.getSolrClient().getClusterState().getCollection(collectionName); } + /** + * Wait for a particular collection state to appear in the cluster client's state reader + * + *

This is a convenience method using the {@link #DEFAULT_TIMEOUT}. + */ protected static void waitForState( String message, String collection, CollectionStatePredicate predicate) { waitForState(message, collection, predicate, DEFAULT_TIMEOUT, TimeUnit.SECONDS); @@ -168,8 +173,6 @@ protected static void waitForState( /** * Wait for a particular collection state to appear in the cluster client's state reader * - *

This is a convenience method using the {@link #DEFAULT_TIMEOUT} - * * @param message a message to report on failure * @param collection the collection to watch * @param predicate a predicate to match against the collection state @@ -207,6 +210,47 @@ protected static void waitForState( } } + /** + * Wait for a particular collection state to appear in the cluster client's state reader. + * + *

This is a convenience method using the {@link #DEFAULT_TIMEOUT}. + */ + protected static void waitForState( + String message, String collection, Predicate predicate) { + waitForState(message, collection, predicate, DEFAULT_TIMEOUT, TimeUnit.SECONDS); + } + + /** + * Wait for a particular collection state to appear in the cluster client's state reader + * + * @param message a message to report on failure + * @param collection the collection to watch + * @param predicate a predicate to match against the collection state + */ + protected static void waitForState( + String message, + String collection, + Predicate predicate, + int timeout, + TimeUnit timeUnit) { + log.info("waitForState ({}): {}", collection, message); + AtomicReference state = new AtomicReference<>(); + try { + cluster + .getZkStateReader() + .waitForState( + collection, + timeout, + timeUnit, + c -> { + state.set(c); + return predicate.test(c); + }); + } catch (Exception e) { + fail(message + "\n" + e.getMessage() + "\nLast available state: " + state.get()); + } + } + /** * Return a {@link CollectionStatePredicate} that returns true if a collection has the expected * numbers of shards and active replicas From 6a0fdacfb794fd515d3a1829fc2f9ef9ac8c4db6 Mon Sep 17 00:00:00 2001 From: Pierre Salagnac Date: Thu, 16 Jan 2025 11:44:28 +0100 Subject: [PATCH 2/5] Restore timeouts not aligned with the default --- .../src/test/org/apache/solr/cloud/SplitShardTest.java | 4 +++- .../common/cloud/PerReplicaStatesIntegrationTest.java | 9 ++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java index 07c7ce0377e..fd196d223fe 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java @@ -360,7 +360,9 @@ public void testShardSplitWithNodeset() throws Exception { waitForState( "Waiting for sub-shards", COLL, - collectionState -> testColl(jetty, collectionState, List.of("shard1_0", "shard1_1"))); + collectionState -> testColl(jetty, collectionState, List.of("shard1_0", "shard1_1")), + 10, + TimeUnit.SECONDS); JettySolrRunner randomJetty = cluster.getRandomJetty(random()); splitShard = diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java index 1638e3e241b..63a2d2cc0f0 100644 --- a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java @@ -262,8 +262,9 @@ public void testMultipleTransitions() throws Exception { waitForState( "Waiting for PRS property", COLL, - collectionState -> - "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE))); + collectionState -> "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE)), + 5, + TimeUnit.SECONDS); CollectionAdminRequest.modifyCollection( COLL, Collections.singletonMap(PER_REPLICA_STATE, "true")) .process(cluster.getSolrClient()); @@ -280,7 +281,9 @@ public void testMultipleTransitions() throws Exception { if (newState.getDuplicate() != null) anyFail.set(true); }); return !anyFail.get(); - }); + }, + 5, + TimeUnit.SECONDS); } finally { cluster.shutdown(); From 795cc72c4cfd9bcd24d00ab3846204e3d2457c12 Mon Sep 17 00:00:00 2001 From: Pierre Salagnac Date: Thu, 16 Jan 2025 12:12:09 +0100 Subject: [PATCH 3/5] More usages of new method --- .../apache/solr/cloud/CollectionsAPISolrJTest.java | 11 ++++------- .../test/org/apache/solr/cloud/SplitShardTest.java | 14 ++++++-------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java index d1cec38fbf6..1c67b5e5af0 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java @@ -328,13 +328,10 @@ public void testCreateAndDeleteShard() throws Exception { assertEquals(0, response.getStatus()); assertTrue(response.isSuccess()); - cluster - .getZkStateReader() - .waitForState( - collectionName, - 30, - TimeUnit.SECONDS, - (l, c) -> c != null && c.getSlice("shardC") != null); + waitForState( + "Wait for shard to be visible", + collectionName, + c -> c != null && c.getSlice("shardC") != null); coresStatus = response.getCollectionCoresStatus(); assertEquals(3, coresStatus.size()); int replicaTlog = 0; diff --git a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java index fd196d223fe..00c3e48ece0 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java @@ -372,14 +372,12 @@ public void testShardSplitWithNodeset() throws Exception { response = splitShard.process(cluster.getSolrClient()).getResponse(); assertNotNull(response.get("success")); - cluster - .getZkStateReader() - .waitForState( - COLL, - 10, - TimeUnit.SECONDS, - (liveNodes, collectionState) -> - testColl(randomJetty, collectionState, List.of("shard2_0", "shard2_1"))); + waitForState( + "Waiting for sub-shards", + COLL, + collectionState -> testColl(randomJetty, collectionState, List.of("shard2_0", "shard2_1")), + 10, + TimeUnit.SECONDS); } private boolean testColl( From 67abf78b922e6a65de68bc5bf97208211e192b94 Mon Sep 17 00:00:00 2001 From: Pierre Salagnac Date: Thu, 16 Jan 2025 22:07:10 +0100 Subject: [PATCH 4/5] Invert parameter order --- .../org/apache/solr/cloud/SplitShardTest.java | 12 +++---- .../apache/solr/cloud/TestPrepRecovery.java | 4 +-- .../apache/solr/cloud/TestPullReplica.java | 4 +-- .../solr/cloud/TestRebalanceLeaders.java | 36 +++++++++---------- .../apache/solr/cloud/TestTlogReplica.java | 16 ++++----- .../maintenance/InactiveShardRemoverTest.java | 8 ++--- .../PerReplicaStatesIntegrationTest.java | 11 +++--- .../apache/solr/cloud/SolrCloudTestCase.java | 12 +++---- 8 files changed, 52 insertions(+), 51 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java index 00c3e48ece0..c7a0bcad305 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java @@ -359,10 +359,10 @@ public void testShardSplitWithNodeset() throws Exception { waitForState( "Waiting for sub-shards", - COLL, - collectionState -> testColl(jetty, collectionState, List.of("shard1_0", "shard1_1")), - 10, - TimeUnit.SECONDS); + COLL,10, + TimeUnit.SECONDS, + collectionState -> testColl(jetty, collectionState, List.of("shard1_0", "shard1_1")) + ); JettySolrRunner randomJetty = cluster.getRandomJetty(random()); splitShard = @@ -375,9 +375,9 @@ public void testShardSplitWithNodeset() throws Exception { waitForState( "Waiting for sub-shards", COLL, - collectionState -> testColl(randomJetty, collectionState, List.of("shard2_0", "shard2_1")), 10, - TimeUnit.SECONDS); + TimeUnit.SECONDS, + collectionState -> testColl(randomJetty, collectionState, List.of("shard2_0", "shard2_1"))); } private boolean testColl( diff --git a/solr/core/src/test/org/apache/solr/cloud/TestPrepRecovery.java b/solr/core/src/test/org/apache/solr/cloud/TestPrepRecovery.java index cfb6358f00b..01763499af3 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestPrepRecovery.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestPrepRecovery.java @@ -110,9 +110,9 @@ public void testLeaderNotResponding() throws Exception { waitForState( "Expected collection: testLeaderNotResponding to be live with 1 shard and 2 replicas", collectionName, - clusterShape(1, 2), 30, - TimeUnit.SECONDS); + TimeUnit.SECONDS, + clusterShape(1, 2)); } finally { TestInjection.prepRecoveryOpPauseForever = null; TestInjection.notifyPauseForeverDone(); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java b/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java index 1f333f5d4ab..cb551cc0d70 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java @@ -754,9 +754,9 @@ static void waitForDeletion(String collection) { waitForState( "Waiting for collection " + collection + " to be deleted", collection, - Objects::isNull, 10, - TimeUnit.SECONDS); + TimeUnit.SECONDS, + Objects::isNull); } private DocCollection assertNumberOfReplicas( diff --git a/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java b/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java index 9fda9bbf4bd..b7a15aea1b6 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java @@ -179,6 +179,8 @@ private void doTestSetArbitraryPropertySliceUnique(String propIn) waitForState( "Check property is uniquely distributed in slice: " + prop, COLLECTION_NAME, + timeoutMs, + TimeUnit.MILLISECONDS, c -> { forceUpdateCollectionStatus(); Slice modSlice = c.getSlice(slice.getName()); @@ -194,9 +196,7 @@ private void doTestSetArbitraryPropertySliceUnique(String propIn) .count(); return count == 1 && rightRep; - }, - timeoutMs, - TimeUnit.MILLISECONDS); + }); } } @@ -338,6 +338,8 @@ private void verifyPropCorrectlyDistributed(String prop) { waitForState( "Check property is distributed evenly: " + prop, COLLECTION_NAME, + timeoutMs, + TimeUnit.MILLISECONDS, docCollection -> { int maxPropCount = 0; int minPropCount = Integer.MAX_VALUE; @@ -352,9 +354,7 @@ private void verifyPropCorrectlyDistributed(String prop) { minPropCount = Math.min(minPropCount, repCount); } return Math.abs(maxPropCount - minPropCount) < 2; - }, - timeoutMs, - TimeUnit.MILLISECONDS); + }); } // Used when we concentrate the leader on a few nodes. @@ -372,6 +372,8 @@ private void verifyPropDistributedAsExpected( waitForState( message, COLLECTION_NAME, + timeoutMs, + TimeUnit.MILLISECONDS, docCollection -> { for (Map.Entry ent : expectedShardReplicaMap.entrySet()) { Replica rep = docCollection.getSlice(ent.getKey()).getReplica(ent.getValue()); @@ -380,9 +382,7 @@ private void verifyPropDistributedAsExpected( } } return true; - }, - timeoutMs, - TimeUnit.MILLISECONDS); + }); } // Just check that the property is distributed as expectecd. This does _not_ rebalance the leaders @@ -583,6 +583,8 @@ private void checkReplicasInactive(List downJettys) { waitForState( "Waiting for all replicas to become inactive", COLLECTION_NAME, + timeoutMs, + TimeUnit.MILLISECONDS, (liveNodes, docCollection) -> { boolean expectedInactive = true; @@ -598,9 +600,7 @@ private void checkReplicasInactive(List downJettys) { } } return expectedInactive; - }, - timeoutMs, - TimeUnit.MILLISECONDS); + }); } // We need to wait around until all replicas are active before expecting rebalancing or @@ -609,6 +609,8 @@ private void checkAllReplicasActive() { waitForState( "Waiting for all replicas to become active", COLLECTION_NAME, + timeoutMs, + TimeUnit.MILLISECONDS, (liveNodes, docCollection) -> { boolean allActive = true; for (Slice slice : docCollection.getSlices()) { @@ -619,9 +621,7 @@ private void checkAllReplicasActive() { } } return allActive; - }, - timeoutMs, - TimeUnit.MILLISECONDS); + }); } // use a simple heuristic to put as many replicas with the property on as few nodes as possible. @@ -664,6 +664,8 @@ private void verifyPropUniquePerShard(String prop) { waitForState( "Waiting to have exactly one replica with " + prop + "set per shard", COLLECTION_NAME, + timeoutMs, + TimeUnit.MILLISECONDS, docCollection -> { for (Slice slice : docCollection.getSlices()) { int propCount = 0; @@ -677,8 +679,6 @@ private void verifyPropUniquePerShard(String prop) { } } return true; - }, - timeoutMs, - TimeUnit.MILLISECONDS); + }); } } 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 480aefc019a..3d33f6da412 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java @@ -819,13 +819,13 @@ public void testRebalanceLeaders() throws Exception { waitForState( "Waiting for setting preferredleader flag", collectionName, + 10, + TimeUnit.SECONDS, c -> { Map slices = c.getSlicesMap(); Replica me = slices.get(slice.getName()).getReplica(newLeaderName); return me.getBool("property.preferredleader", false); - }, - 10, - TimeUnit.SECONDS); + }); // Rebalance leaders params = new ModifiableSolrParams(); @@ -840,14 +840,14 @@ public void testRebalanceLeaders() throws Exception { waitForState( "Waiting for a new leader to be elected", collectionName, + 30, + TimeUnit.SECONDS, c -> { Replica leader = c.getSlice(slice.getName()).getLeader(); return leader != null && leader.getName().equals(newLeaderName) && leader.isActive(cloudClient.getClusterState().getLiveNodes()); - }, - 30, - TimeUnit.SECONDS); + }); new UpdateRequest() .add(sdoc("id", "1")) @@ -1030,9 +1030,9 @@ private void waitForDeletion(String collection) { waitForState( "Waiting for collection " + collection + " to be deleted", collection, - Objects::isNull, 10, - TimeUnit.SECONDS); + TimeUnit.SECONDS, + Objects::isNull); } private DocCollection assertNumberOfReplicas( diff --git a/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java b/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java index 3edb9bfcc3b..b3a267bfdb7 100644 --- a/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java +++ b/solr/core/src/test/org/apache/solr/cluster/maintenance/InactiveShardRemoverTest.java @@ -64,9 +64,9 @@ public void testDeleteInactiveShard() throws Exception { waitForState( "Waiting for inactive shard to be deleted", collectionName, - clusterShape(0, 0), 5, - TimeUnit.SECONDS); + TimeUnit.SECONDS, + clusterShape(0, 0)); } finally { removePlugin(); } @@ -96,9 +96,9 @@ public void testTtl() throws Exception { waitForState( "Waiting for InactiveShardRemover to delete inactive shard", collectionName, - clusterShape(0, 0), ttlSeconds + 5, - TimeUnit.SECONDS); + TimeUnit.SECONDS, + clusterShape(0, 0)); final long ttlEnd = timeSource.getTimeNs(); final long ttlPeriodSeconds = TimeUnit.NANOSECONDS.toSeconds(ttlEnd - ttlStart); diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java index 63a2d2cc0f0..22d11e2f0b0 100644 --- a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java @@ -262,15 +262,18 @@ public void testMultipleTransitions() throws Exception { waitForState( "Waiting for PRS property", COLL, - collectionState -> "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE)), 5, - TimeUnit.SECONDS); + TimeUnit.SECONDS, + collectionState -> + "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE))); CollectionAdminRequest.modifyCollection( COLL, Collections.singletonMap(PER_REPLICA_STATE, "true")) .process(cluster.getSolrClient()); waitForState( "Waiting for PRS property", COLL, + 5, + TimeUnit.SECONDS, collectionState -> { AtomicBoolean anyFail = new AtomicBoolean(false); PerReplicaStates prs2 = @@ -281,9 +284,7 @@ public void testMultipleTransitions() throws Exception { if (newState.getDuplicate() != null) anyFail.set(true); }); return !anyFail.get(); - }, - 5, - TimeUnit.SECONDS); + }); } finally { cluster.shutdown(); diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java index c517774cb05..83ad32ca523 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudTestCase.java @@ -167,7 +167,7 @@ protected static DocCollection getCollectionState(String collectionName) { */ protected static void waitForState( String message, String collection, CollectionStatePredicate predicate) { - waitForState(message, collection, predicate, DEFAULT_TIMEOUT, TimeUnit.SECONDS); + waitForState(message, collection, DEFAULT_TIMEOUT, TimeUnit.SECONDS, predicate); } /** @@ -180,9 +180,9 @@ protected static void waitForState( protected static void waitForState( String message, String collection, - CollectionStatePredicate predicate, int timeout, - TimeUnit timeUnit) { + TimeUnit timeUnit, + CollectionStatePredicate predicate) { log.info("waitForState ({}): {}", collection, message); AtomicReference state = new AtomicReference<>(); AtomicReference> liveNodesLastSeen = new AtomicReference<>(); @@ -217,7 +217,7 @@ protected static void waitForState( */ protected static void waitForState( String message, String collection, Predicate predicate) { - waitForState(message, collection, predicate, DEFAULT_TIMEOUT, TimeUnit.SECONDS); + waitForState(message, collection, DEFAULT_TIMEOUT, TimeUnit.SECONDS, predicate); } /** @@ -230,9 +230,9 @@ protected static void waitForState( protected static void waitForState( String message, String collection, - Predicate predicate, int timeout, - TimeUnit timeUnit) { + TimeUnit timeUnit, + Predicate predicate) { log.info("waitForState ({}): {}", collection, message); AtomicReference state = new AtomicReference<>(); try { From cc6151497285b1ce0ac9e64feba317e928c9d490 Mon Sep 17 00:00:00 2001 From: Pierre Salagnac Date: Fri, 17 Jan 2025 09:00:11 +0100 Subject: [PATCH 5/5] tidy --- .../src/test/org/apache/solr/cloud/SplitShardTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java index c7a0bcad305..515664f1150 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java @@ -359,10 +359,10 @@ public void testShardSplitWithNodeset() throws Exception { waitForState( "Waiting for sub-shards", - COLL,10, - TimeUnit.SECONDS, - collectionState -> testColl(jetty, collectionState, List.of("shard1_0", "shard1_1")) - ); + COLL, + 10, + TimeUnit.SECONDS, + collectionState -> testColl(jetty, collectionState, List.of("shard1_0", "shard1_1"))); JettySolrRunner randomJetty = cluster.getRandomJetty(random()); splitShard =