You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2018/12/14 17:56:12 UTC

[1/3] lucene-solr:branch_7x: SOLR-13065: Harden TestSimExecutePlanAction

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_7x d5c4edfd6 -> a4112878f


SOLR-13065: Harden TestSimExecutePlanAction


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/a4112878
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/a4112878
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/a4112878

Branch: refs/heads/branch_7x
Commit: a4112878fcdb016dad67c78d3ec1aaabcefbc9c0
Parents: c6873e9
Author: Jason Gerlowski <ge...@apache.org>
Authored: Fri Dec 14 11:09:43 2018 -0500
Committer: Jason Gerlowski <ge...@apache.org>
Committed: Fri Dec 14 12:55:59 2018 -0500

----------------------------------------------------------------------
 .../sim/SimClusterStateProvider.java            | 35 ++++++++++++++------
 .../sim/TestSimExecutePlanAction.java           |  2 --
 2 files changed, 25 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/a4112878/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
index c411b21..fd6c955 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
@@ -411,16 +411,16 @@ public class SimClusterStateProvider implements ClusterStateProvider {
     lock.lockInterruptibly();
     try {
       setReplicaStates(nodeId, Replica.State.ACTIVE, collections);
+      if (!collections.isEmpty()) {
+        collectionsStatesRef.set(null);
+        simRunLeaderElection(collections, true);
+        return true;
+      } else {
+        return false;
+      }
     } finally {
       lock.unlock();
     }
-    if (!collections.isEmpty()) {
-      collectionsStatesRef.set(null);
-      simRunLeaderElection(collections, true);
-      return true;
-    } else {
-      return false;
-    }
   }
 
   /**
@@ -657,7 +657,12 @@ public class SimClusterStateProvider implements ClusterStateProvider {
   private void simRunLeaderElection(Collection<String> collections, boolean saveClusterState) throws Exception {
     ensureNotClosed();
     if (saveClusterState) {
-      collectionsStatesRef.set(null);
+      lock.lockInterruptibly();
+      try {
+        collectionsStatesRef.set(null);
+      } finally {
+        lock.unlock();
+      }
     }
     ClusterState state = getClusterState();
     state.forEachCollection(dc -> {
@@ -798,7 +803,12 @@ public class SimClusterStateProvider implements ClusterStateProvider {
     CreateCollectionCmd.checkReplicaTypes(props);
 
     // always force getting fresh state
-    collectionsStatesRef.set(null);
+    lock.lockInterruptibly();
+    try {
+      collectionsStatesRef.set(null);
+    } finally {
+      lock.unlock();
+    }
     final ClusterState clusterState = getClusterState();
 
     String withCollection = props.getStr(CollectionAdminParams.WITH_COLLECTION);
@@ -928,7 +938,12 @@ public class SimClusterStateProvider implements ClusterStateProvider {
     });
 
     // force recreation of collection states
-    collectionsStatesRef.set(null);
+    lock.lockInterruptibly();
+    try {
+      collectionsStatesRef.set(null);
+    } finally {
+      lock.unlock();
+    }
     //simRunLeaderElection(Collections.singleton(collectionName), true);
     if (waitForFinalState) {
       boolean finished = finalStateLatch.await(cloudManager.getTimeSource().convertDelay(TimeUnit.SECONDS, 60, TimeUnit.MILLISECONDS),

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/a4112878/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExecutePlanAction.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExecutePlanAction.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExecutePlanAction.java
index 1d36f14..8f95288 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExecutePlanAction.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimExecutePlanAction.java
@@ -83,7 +83,6 @@ public class TestSimExecutePlanAction extends SimSolrCloudTestCase {
 
   @Test
   @LuceneTestCase.BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 28-June-2018
-  @AwaitsFix(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028")
   public void testExecute() throws Exception {
     SolrClient solrClient = cluster.simGetSolrClient();
     String collectionName = "testExecute";
@@ -159,7 +158,6 @@ public class TestSimExecutePlanAction extends SimSolrCloudTestCase {
   }
 
   @Test
-  @AwaitsFix(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // this test can fail to elect a leader, seems to be common among sim tests
   public void testIntegration() throws Exception  {
     SolrClient solrClient = cluster.simGetSolrClient();
 


[2/3] lucene-solr:branch_7x: Fix active replica count reporting in SimClusterStateProvider

Posted by ge...@apache.org.
Fix active replica count reporting in SimClusterStateProvider

One codepath assumed all replicas were active, instead of checking the
state of each individually.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/c6873e9d
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/c6873e9d
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/c6873e9d

Branch: refs/heads/branch_7x
Commit: c6873e9d1988c341eeb8bbe541104e9f0754038d
Parents: d8cbe54
Author: Jason Gerlowski <ge...@apache.org>
Authored: Fri Dec 14 11:07:10 2018 -0500
Committer: Jason Gerlowski <ge...@apache.org>
Committed: Fri Dec 14 12:55:59 2018 -0500

----------------------------------------------------------------------
 .../solr/cloud/autoscaling/sim/SimClusterStateProvider.java   | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/c6873e9d/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
index 6197c41..c411b21 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
@@ -1998,7 +1998,12 @@ public class SimClusterStateProvider implements ClusterStateProvider {
           if (buffered != null) {
             bufferedDocs += buffered.get();
           }
-          activeReplicas += s.getReplicas().size();
+
+          for (Replica r : s.getReplicas()) {
+            if (r.getState() == Replica.State.ACTIVE) {
+              activeReplicas++;
+            }
+          }
           Replica leader = s.getLeader();
           if (leader == null) {
             noLeader++;


[3/3] lucene-solr:branch_7x: Indicate collection in `clusterShape` log messages

Posted by ge...@apache.org.
Indicate collection in `clusterShape` log messages

Many of Solr's tests use CloudTestUtils' `waitForState` and
`clusterShape` methods to wait until a SolrCloud cluster matches a
particular expected shape.  The code periodically checks on the cluster
state, and logs a message if a collection doesn't match the state we
expect of it.  Prior to this commit, these log messages omitted the
collection name though, which makes things a little confusing when a
test is checking on the state of multiple collections simultaneously (as
can happen when Triggers fire in the background).


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/d8cbe546
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/d8cbe546
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/d8cbe546

Branch: refs/heads/branch_7x
Commit: d8cbe5465666c593d208f531b0506d48678e83a2
Parents: d5c4edf
Author: Jason Gerlowski <ge...@apache.org>
Authored: Fri Dec 14 10:57:20 2018 -0500
Committer: Jason Gerlowski <ge...@apache.org>
Committed: Fri Dec 14 12:55:59 2018 -0500

----------------------------------------------------------------------
 solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/d8cbe546/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java b/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
index 7ac1c44..658c87b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
@@ -139,7 +139,7 @@ public class CloudTestUtils {
       }
       Collection<Slice> slices = withInactive ? collectionState.getSlices() : collectionState.getActiveSlices();
       if (slices.size() != expectedShards) {
-        log.info("-- wrong number of slices, expected={}, found={}: {}", expectedShards, collectionState.getSlices().size(), collectionState.getSlices());
+        log.info("-- wrong number of slices for collection {}, expected={}, found={}: {}", collectionState.getName(), expectedShards, collectionState.getSlices().size(), collectionState.getSlices());
         return false;
       }
       Set<String> leaderless = new HashSet<>();
@@ -158,7 +158,7 @@ public class CloudTestUtils {
             activeReplicas++;
         }
         if (activeReplicas != expectedReplicas) {
-          log.info("-- wrong number of active replicas in slice {}, expected={}, found={}", slice.getName(), expectedReplicas, activeReplicas);
+          log.info("-- wrong number of active replicas for collection {} in slice {}, expected={}, found={}", collectionState.getName(), slice.getName(), expectedReplicas, activeReplicas);
           return false;
         }
       }