From 963c6522b6e10bfeaad340457d1e96351d0aecc1 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Sat, 14 May 2016 11:17:57 +0100 Subject: [PATCH] SOLR-8323: DocCollection.isFullyActive needs to know how many replicas to expect --- .../solr/common/cloud/DocCollection.java | 11 +++++- .../solr/common/cloud/ZkStateReader.java | 5 ++- .../cloud/TestCollectionStateWatchers.java | 37 ++++++++++++------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java b/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java index 405d69d0077..b5c65a6d847 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java @@ -226,17 +226,24 @@ public class DocCollection extends ZkNodeProps implements Iterable { * * @see CollectionStatePredicate */ - public static boolean isFullyActive(Set liveNodes, DocCollection collectionState) { + public static boolean isFullyActive(Set liveNodes, DocCollection collectionState, + int expectedShards, int expectedReplicas) { Objects.requireNonNull(liveNodes); if (collectionState == null) return false; + int activeShards = 0; for (Slice slice : collectionState) { + int activeReplicas = 0; for (Replica replica : slice) { if (replica.isActive(liveNodes) == false) return false; + activeReplicas++; } + if (activeReplicas != expectedReplicas) + return false; + activeShards++; } - return true; + return activeShards == expectedShards; } @Override diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java index 2bb838907a7..7c6b244d95e 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java @@ -518,9 +518,10 @@ public class ZkStateReader implements Closeable { if (ref == null) continue; // legacy collections are always in-memory - DocCollection newState = ref.get(); + DocCollection oldState = ref.get(); + DocCollection newState = loadedData.getCollectionStates().get(coll).get(); if (!collWatch.stateWatchers.isEmpty() - && !Objects.equals(loadedData.getCollectionStates().get(coll).get(), newState)) { + && !Objects.equals(oldState, newState)) { notifyStateWatchers(liveNodes, coll, newState); } } diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java b/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java index 2862f08525d..057cf5f4df2 100644 --- a/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java +++ b/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java @@ -89,9 +89,11 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { public void testSimpleCollectionWatch() throws Exception { CloudSolrClient client = cluster.getSolrClient(); - cluster.createCollection("testcollection", CLUSTER_SIZE, 1, "config", new HashMap<>()); + CollectionAdminRequest.createCollection("testcollection", "config", 4, 1) + .processAndWait(client, MAX_WAIT_TIMEOUT); - client.waitForState("testcollection", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, DocCollection::isFullyActive); + client.waitForState("testcollection", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, + (n, c) -> DocCollection.isFullyActive(n, c, 4, 1)); // shutdown a node and check that we get notified about the change final AtomicInteger nodeCount = new AtomicInteger(0); @@ -121,14 +123,16 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { public void testWaitForStateChecksCurrentState() throws Exception { CloudSolrClient client = cluster.getSolrClient(); - cluster.createCollection("waitforstate", 1, 1, "config", new HashMap<>()); + CollectionAdminRequest.createCollection("waitforstate", "config", 1, 1) + .processAndWait(client, MAX_WAIT_TIMEOUT); - client.waitForState("waitforstate", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, DocCollection::isFullyActive); + client.waitForState("waitforstate", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, + (n, c) -> DocCollection.isFullyActive(n, c, 1, 1)); // several goes, to check that we're not getting delayed state changes for (int i = 0; i < 10; i++) { try { - client.waitForState("waitforstate", 1, TimeUnit.SECONDS, DocCollection::isFullyActive); + client.waitForState("waitforstate", 1, TimeUnit.SECONDS, (n, c) -> DocCollection.isFullyActive(n, c, 1, 1)); } catch (TimeoutException e) { fail("waitForState should return immediately if the predicate is already satisfied"); @@ -140,8 +144,12 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { @Test public void testCanWaitForNonexistantCollection() throws Exception { - Future future = waitInBackground("delayed", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, DocCollection::isFullyActive); - cluster.createCollection("delayed", 1, 1, "config", new HashMap<>()); + Future future = waitInBackground("delayed", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, + (n, c) -> DocCollection.isFullyActive(n, c, 1, 1)); + + CollectionAdminRequest.createCollection("delayed", "config", 1, 1) + .processAndWait(cluster.getSolrClient(), MAX_WAIT_TIMEOUT); + assertTrue("waitForState was not triggered by collection creation", future.get()); } @@ -162,8 +170,11 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { public void testWaitForStateWatcherIsRetainedOnPredicateFailure() throws Exception { CloudSolrClient client = cluster.getSolrClient(); - cluster.createCollection("falsepredicate", 4, 1, "config", new HashMap<>()); - client.waitForState("falsepredicate", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, DocCollection::isFullyActive); + CollectionAdminRequest.createCollection("falsepredicate", "config", 4, 1) + .processAndWait(client, MAX_WAIT_TIMEOUT); + + client.waitForState("falsepredicate", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, + (n, c) -> DocCollection.isFullyActive(n, c, 4, 1)); final CountDownLatch firstCall = new CountDownLatch(1); @@ -172,7 +183,7 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { Future future = waitInBackground("falsepredicate", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, (liveNodes, collectionState) -> { firstCall.countDown(); - return DocCollection.isFullyActive(liveNodes, collectionState); + return DocCollection.isFullyActive(liveNodes, collectionState, 4, 1); }); // first, stop another node; the watch should not be fired after this! @@ -195,7 +206,7 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { client.getZkStateReader().getStateWatchers("no-such-collection") == null); expectThrows(TimeoutException.class, () -> { - client.waitForState("no-such-collection", 10, TimeUnit.MILLISECONDS, DocCollection::isFullyActive); + client.waitForState("no-such-collection", 10, TimeUnit.MILLISECONDS, (n, c) -> DocCollection.isFullyActive(n, c, 1, 1)); }); Set watchers = client.getZkStateReader().getStateWatchers("no-such-collection"); @@ -219,8 +230,8 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { final CloudSolrClient client = cluster.getSolrClient(); - Future future - = waitInBackground("stateformat1", MAX_WAIT_TIMEOUT, TimeUnit.SECONDS, DocCollection::isFullyActive); + Future future = waitInBackground("stateformat1", 10, TimeUnit.SECONDS, + (n, c) -> DocCollection.isFullyActive(n, c, 1, 1)); CollectionAdminRequest.createCollection("stateformat1", "config", 1, 1).setStateFormat(1) .processAndWait(client, MAX_WAIT_TIMEOUT);