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
Changes from all 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
@@ -194,6 +194,9 @@ Other Changes
* SOLR-17611: SolrJ's User-Agent header now uses the version of SolrJ. There's a corresponding
HttpSolrCall.getUserAgentSolrVersion to parse it. (David Smiley)

* 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
---------------------
Original file line number Diff line number Diff line change
@@ -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",
Original file line number Diff line number Diff line change
@@ -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
@@ -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;
@@ -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
@@ -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) {
@@ -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(
@@ -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
@@ -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) {
@@ -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,
Original file line number Diff line number Diff line change
@@ -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");
Original file line number Diff line number Diff line change
@@ -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
@@ -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);
}

/**
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
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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()));
}
Original file line number Diff line number Diff line change
@@ -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);
}

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