Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17581: Less ZK watches in tests. #3036

Merged
merged 6 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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";
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -423,9 +420,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
Expand Down Expand Up @@ -493,7 +488,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) {
Expand Down Expand Up @@ -1277,7 +1272,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(
Expand All @@ -1288,7 +1283,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
Expand All @@ -1308,7 +1303,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) {
Expand All @@ -1335,7 +1330,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
27 changes: 8 additions & 19 deletions solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down
12 changes: 5 additions & 7 deletions solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading