You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/03/19 17:37:42 UTC

[GitHub] [lucene-solr] dsmiley opened a new pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

dsmiley opened a new pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366
 
 
   https://issues.apache.org/jira/browse/SOLR-14342
   (see the issue).
   
   I chose to rewrite much of the test to give me greater confidence that it works as designed.  If I, for example, comment out much of CoreSorter.init then the test fails.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley closed pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley closed pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395422074
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
+
+    // compute nodes, some live, some down
+    final int maxNodesOfAType = perShardCounts.stream() // not too important how many we have, but lets have plenty
+        .mapToInt(c -> c.totalReplicasInLiveNodes + c.totalReplicasInDownNodes + c.myReplicas).max().getAsInt();
+    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + "_8983").collect(Collectors.toList());
+    Collections.shuffle(liveNodes, random());
+    String thisNode = liveNodes.get(0);
+    List<String> otherLiveNodes = liveNodes.subList(1, liveNodes.size());
+    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + "_8983").collect(Collectors.toList());
+
+    // divide into two collections
+    int numCol1 = random().nextInt(perShardCounts.size());
+    Map<String,List<CountsForEachShard>> collToCounts = new HashMap<>();
+    collToCounts.put("col1", perShardCounts.subList(0, numCol1));
+    collToCounts.put("col2", perShardCounts.subList(numCol1, perShardCounts.size()));
+
+    Map<String,DocCollection> collToState = new HashMap<>();
+    Map<CountsForEachShard, List<CoreDescriptor>> myCountsToDescs = new HashMap<>();
+    for (Map.Entry<String, List<CountsForEachShard>> entry : collToCounts.entrySet()) {
+      String collection = entry.getKey();
+      List<CountsForEachShard> collCounts = entry.getValue();
+      Map<String, Slice> sliceMap = new HashMap<>(collCounts.size());
+      for (CountsForEachShard shardCounts : collCounts) {
+        String slice = "s" + shardCounts.hashCode();
+        List<Replica> replicas = new ArrayList<>();
+        for (int myRepNum = 0; myRepNum < shardCounts.myReplicas; myRepNum++) {
+          addNewReplica(replicas, collection, slice, Collections.singletonList(thisNode));
+          // save this mapping for later
+          myCountsToDescs.put(shardCounts, replicas.stream().map(this::newCoreDescriptor).collect(Collectors.toList()));
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInLiveNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, otherLiveNodes);
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInDownNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, downNodes);
+        }
+        Map<String, Replica> replicaMap = replicas.stream().collect(Collectors.toMap(Replica::getName, Function.identity()));
+        sliceMap.put(slice, new Slice(slice, replicaMap, map(), collection));
       }
-      return false;
+      DocCollection col = new DocCollection(collection, sliceMap, map(), DocRouter.DEFAULT);
+      collToState.put(collection, col);
     }
-
-
-    @Override
-    public int hashCode() {
-      return replicaName.hashCode();
-    }
-
-    CloudDescriptor getCloudDescriptor() {
-      return cd;
-
-    }
-
-    public Replica getReplica(String node) {
-      return new Replica(replicaName, Utils.makeMap("core", replicaName, "node_name", node), cd.getCollectionName(), cd.getShardId());
-    }
-
-    public boolean equals(String coll, String slice) {
-      return cd.getCollectionName().equals(coll) && slice.equals(cd.getShardId());
+    // reverse map
+    Map<CoreDescriptor, CountsForEachShard> myDescsToCounts = new HashMap<>();
+    for (Map.Entry<CountsForEachShard, List<CoreDescriptor>> entry : myCountsToDescs.entrySet()) {
+      for (CoreDescriptor descriptor : entry.getValue()) {
+        CountsForEachShard prev = myDescsToCounts.put(descriptor, entry.getKey());
+        assert prev == null;
+      }
     }
-  }
-
 
-  class MockCoreSorter extends CoreSorter {
-    int numColls = 1 + random().nextInt(3);
-    int numReplicas = 2 + random().nextInt(2);
-    int numShards = 50 + random().nextInt(10);
-    String myNodeName;
-    Collection<CloudDescriptor> myCores = new ArrayList<>();
-    List<CoreDescriptor> localCores = new ArrayList<>();
-
-    Map<ReplicaInfo, String> replicaPositions = new LinkedHashMap<>();//replicaname vs. nodename
-
-    public MockCoreSorter() {
-      int totalNodes = 50 + random().nextInt(10);
-      int myNode = random().nextInt(totalNodes);
-      List<String> nodeNames = new ArrayList<>();
-      for (int i = 0; i < totalNodes; i++) {
-        String s = "192.168.1." + i + ":8983_solr";
-        if (i == myNode) myNodeName = s;
-        boolean on = random().nextInt(100) < 70;
-        nodes.put(s,
-            on);//70% chance that the node is up;
-        nodeNames.add(s);
-        if(on) liveNodes.add(s);
-      }
+    assert myCountsToDescs.size() == perShardCounts.size(); // just a sanity check
 
-      for (int i = 0; i < numColls; i++) {
-        for (int j = 0; j < numShards; j++) {
-          for (int k = 0; k < numReplicas; k++) {
-            ReplicaInfo ri = new ReplicaInfo(i, j, k);
-            replicaPositions.put(ri, nodeNames.get(random().nextInt(totalNodes)));
+    CoreContainer mockCC = mock(CoreContainer.class);
+    {
+      when(mockCC.isZooKeeperAware()).thenReturn(true);
+
+      ZkController mockZKC = mock(ZkController.class);
+      when(mockCC.getZkController()).thenReturn(mockZKC);
+      {
+        ClusterState mockClusterState = mock(ClusterState.class);
+        when(mockZKC.getClusterState()).thenReturn(mockClusterState);
+        {
+          when(mockClusterState.getLiveNodes()).thenReturn(new HashSet<>(liveNodes));
+          for (Map.Entry<String, DocCollection> entry : collToState.entrySet()) {
+            when(mockClusterState.getCollectionOrNull(entry.getKey())).thenReturn(entry.getValue());
           }
         }
       }
 
-      for (Map.Entry<ReplicaInfo, String> e : replicaPositions.entrySet()) {
-        if (e.getValue().equals(myNodeName)) {
-          myCores.add(e.getKey().getCloudDescriptor());
-          localCores.add(new MockCoreContainer.MockCoreDescriptor() {
-            @Override
-            public CloudDescriptor getCloudDescriptor() {
-              return e.getKey().getCloudDescriptor();
-            }
-          });
-        }
-      }
-    }
-
-    @Override
-    String getNodeName() {
-      return myNodeName;
-    }
-
-    @Override
-    Collection<CloudDescriptor> getCloudDescriptors() {
-      return myCores;
-
-    }
+      NodeConfig mockNodeConfig = mock(NodeConfig.class);
+      when(mockNodeConfig.getNodeName()).thenReturn(thisNode);
+      when(mockCC.getNodeConfig()).thenReturn(mockNodeConfig);
 
-    public List<CoreDescriptor> getLocalCores() {
-      return localCores;
     }
 
-    @Override
-    Collection<Replica> getReplicas(ClusterState cs, String coll, String slice) {
-      List<Replica> r = new ArrayList<>();
-      for (Map.Entry<ReplicaInfo, String> e : replicaPositions.entrySet()) {
-        if (e.getKey().equals(coll, slice)) {
-          r.add(e.getKey().getReplica(e.getValue()));
+    List<CoreDescriptor> myDescs = new ArrayList<>(myDescsToCounts.keySet());
+    for (int i = 0; i < 10; i++) {
+      Collections.shuffle(myDescs, random());
+
+      List<CoreDescriptor> resultDescs = CoreSorter.sortCores(mockCC, myDescs);
+      // map descriptors back to counts, removing consecutive duplicates
+      List<CountsForEachShard> resultCounts = new ArrayList<>();
+      CountsForEachShard lastCounts = null;
+      for (CoreDescriptor resultDesc : resultDescs) {
+        CountsForEachShard counts = myDescsToCounts.get(resultDesc);
+        if (counts != lastCounts) {
+          resultCounts.add(counts);
         }
+        lastCounts = counts;
       }
-      return r;
+      assertEquals(expectedCounts, resultCounts);
     }
   }
 
+  private CoreDescriptor newCoreDescriptor(Replica r) {
+    Map<String,String> props = map(
+        CoreDescriptor.CORE_SHARD, r.getSlice(),
+        CoreDescriptor.CORE_COLLECTION, r.getCollection(),
+        CoreDescriptor.CORE_NODE_NAME, r.getNodeName()
+    );
+    return new CoreDescriptor(r.getCoreName(), TEST_PATH(), props , null, mock(ZkController.class));
+  }
+
+  protected Replica addNewReplica(List<Replica> replicaList, String collection, String slice, List<String> possibleNodes) {
+    String replica = "r" + replicaList.size();
+    String node = possibleNodes.get(random().nextInt(possibleNodes.size())); // place on a random node
+    Replica r = newReplica(collection, slice, replica, node);
+    replicaList.add(r);
+    return r;
+  }
+
+  protected Replica newReplica(String collection, String slice, String replica, String node) {
+    return new Replica(replica, Utils.makeMap("core", replica, "node_name", node), collection, slice);
 
 Review comment:
   That'd work too; I took this bit here from an existing line of code in the previous version of this test.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395420007
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
+
+    // compute nodes, some live, some down
+    final int maxNodesOfAType = perShardCounts.stream() // not too important how many we have, but lets have plenty
+        .mapToInt(c -> c.totalReplicasInLiveNodes + c.totalReplicasInDownNodes + c.myReplicas).max().getAsInt();
+    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + "_8983").collect(Collectors.toList());
+    Collections.shuffle(liveNodes, random());
+    String thisNode = liveNodes.get(0);
+    List<String> otherLiveNodes = liveNodes.subList(1, liveNodes.size());
+    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + "_8983").collect(Collectors.toList());
+
+    // divide into two collections
+    int numCol1 = random().nextInt(perShardCounts.size());
+    Map<String,List<CountsForEachShard>> collToCounts = new HashMap<>();
+    collToCounts.put("col1", perShardCounts.subList(0, numCol1));
+    collToCounts.put("col2", perShardCounts.subList(numCol1, perShardCounts.size()));
+
+    Map<String,DocCollection> collToState = new HashMap<>();
+    Map<CountsForEachShard, List<CoreDescriptor>> myCountsToDescs = new HashMap<>();
+    for (Map.Entry<String, List<CountsForEachShard>> entry : collToCounts.entrySet()) {
+      String collection = entry.getKey();
+      List<CountsForEachShard> collCounts = entry.getValue();
+      Map<String, Slice> sliceMap = new HashMap<>(collCounts.size());
+      for (CountsForEachShard shardCounts : collCounts) {
+        String slice = "s" + shardCounts.hashCode();
+        List<Replica> replicas = new ArrayList<>();
+        for (int myRepNum = 0; myRepNum < shardCounts.myReplicas; myRepNum++) {
+          addNewReplica(replicas, collection, slice, Collections.singletonList(thisNode));
+          // save this mapping for later
+          myCountsToDescs.put(shardCounts, replicas.stream().map(this::newCoreDescriptor).collect(Collectors.toList()));
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInLiveNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, otherLiveNodes);
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInDownNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, downNodes);
+        }
+        Map<String, Replica> replicaMap = replicas.stream().collect(Collectors.toMap(Replica::getName, Function.identity()));
+        sliceMap.put(slice, new Slice(slice, replicaMap, map(), collection));
       }
-      return false;
+      DocCollection col = new DocCollection(collection, sliceMap, map(), DocRouter.DEFAULT);
+      collToState.put(collection, col);
     }
-
-
-    @Override
-    public int hashCode() {
-      return replicaName.hashCode();
-    }
-
-    CloudDescriptor getCloudDescriptor() {
-      return cd;
-
-    }
-
-    public Replica getReplica(String node) {
-      return new Replica(replicaName, Utils.makeMap("core", replicaName, "node_name", node), cd.getCollectionName(), cd.getShardId());
-    }
-
-    public boolean equals(String coll, String slice) {
-      return cd.getCollectionName().equals(coll) && slice.equals(cd.getShardId());
+    // reverse map
+    Map<CoreDescriptor, CountsForEachShard> myDescsToCounts = new HashMap<>();
+    for (Map.Entry<CountsForEachShard, List<CoreDescriptor>> entry : myCountsToDescs.entrySet()) {
+      for (CoreDescriptor descriptor : entry.getValue()) {
+        CountsForEachShard prev = myDescsToCounts.put(descriptor, entry.getKey());
+        assert prev == null;
+      }
     }
-  }
-
 
-  class MockCoreSorter extends CoreSorter {
-    int numColls = 1 + random().nextInt(3);
-    int numReplicas = 2 + random().nextInt(2);
-    int numShards = 50 + random().nextInt(10);
-    String myNodeName;
-    Collection<CloudDescriptor> myCores = new ArrayList<>();
-    List<CoreDescriptor> localCores = new ArrayList<>();
-
-    Map<ReplicaInfo, String> replicaPositions = new LinkedHashMap<>();//replicaname vs. nodename
-
-    public MockCoreSorter() {
-      int totalNodes = 50 + random().nextInt(10);
-      int myNode = random().nextInt(totalNodes);
-      List<String> nodeNames = new ArrayList<>();
-      for (int i = 0; i < totalNodes; i++) {
-        String s = "192.168.1." + i + ":8983_solr";
-        if (i == myNode) myNodeName = s;
-        boolean on = random().nextInt(100) < 70;
-        nodes.put(s,
-            on);//70% chance that the node is up;
-        nodeNames.add(s);
-        if(on) liveNodes.add(s);
-      }
+    assert myCountsToDescs.size() == perShardCounts.size(); // just a sanity check
 
-      for (int i = 0; i < numColls; i++) {
-        for (int j = 0; j < numShards; j++) {
-          for (int k = 0; k < numReplicas; k++) {
-            ReplicaInfo ri = new ReplicaInfo(i, j, k);
-            replicaPositions.put(ri, nodeNames.get(random().nextInt(totalNodes)));
+    CoreContainer mockCC = mock(CoreContainer.class);
+    {
+      when(mockCC.isZooKeeperAware()).thenReturn(true);
+
+      ZkController mockZKC = mock(ZkController.class);
+      when(mockCC.getZkController()).thenReturn(mockZKC);
+      {
+        ClusterState mockClusterState = mock(ClusterState.class);
+        when(mockZKC.getClusterState()).thenReturn(mockClusterState);
+        {
+          when(mockClusterState.getLiveNodes()).thenReturn(new HashSet<>(liveNodes));
+          for (Map.Entry<String, DocCollection> entry : collToState.entrySet()) {
+            when(mockClusterState.getCollectionOrNull(entry.getKey())).thenReturn(entry.getValue());
           }
         }
       }
 
-      for (Map.Entry<ReplicaInfo, String> e : replicaPositions.entrySet()) {
-        if (e.getValue().equals(myNodeName)) {
-          myCores.add(e.getKey().getCloudDescriptor());
-          localCores.add(new MockCoreContainer.MockCoreDescriptor() {
-            @Override
-            public CloudDescriptor getCloudDescriptor() {
-              return e.getKey().getCloudDescriptor();
-            }
-          });
-        }
-      }
-    }
-
-    @Override
-    String getNodeName() {
-      return myNodeName;
-    }
-
-    @Override
-    Collection<CloudDescriptor> getCloudDescriptors() {
-      return myCores;
-
-    }
+      NodeConfig mockNodeConfig = mock(NodeConfig.class);
+      when(mockNodeConfig.getNodeName()).thenReturn(thisNode);
+      when(mockCC.getNodeConfig()).thenReturn(mockNodeConfig);
 
-    public List<CoreDescriptor> getLocalCores() {
-      return localCores;
     }
 
-    @Override
-    Collection<Replica> getReplicas(ClusterState cs, String coll, String slice) {
-      List<Replica> r = new ArrayList<>();
-      for (Map.Entry<ReplicaInfo, String> e : replicaPositions.entrySet()) {
-        if (e.getKey().equals(coll, slice)) {
-          r.add(e.getKey().getReplica(e.getValue()));
+    List<CoreDescriptor> myDescs = new ArrayList<>(myDescsToCounts.keySet());
+    for (int i = 0; i < 10; i++) {
+      Collections.shuffle(myDescs, random());
+
+      List<CoreDescriptor> resultDescs = CoreSorter.sortCores(mockCC, myDescs);
+      // map descriptors back to counts, removing consecutive duplicates
+      List<CountsForEachShard> resultCounts = new ArrayList<>();
+      CountsForEachShard lastCounts = null;
+      for (CoreDescriptor resultDesc : resultDescs) {
+        CountsForEachShard counts = myDescsToCounts.get(resultDesc);
+        if (counts != lastCounts) {
+          resultCounts.add(counts);
         }
+        lastCounts = counts;
       }
-      return r;
+      assertEquals(expectedCounts, resultCounts);
     }
   }
 
+  private CoreDescriptor newCoreDescriptor(Replica r) {
+    Map<String,String> props = map(
+        CoreDescriptor.CORE_SHARD, r.getSlice(),
+        CoreDescriptor.CORE_COLLECTION, r.getCollection(),
+        CoreDescriptor.CORE_NODE_NAME, r.getNodeName()
+    );
+    return new CoreDescriptor(r.getCoreName(), TEST_PATH(), props , null, mock(ZkController.class));
+  }
+
+  protected Replica addNewReplica(List<Replica> replicaList, String collection, String slice, List<String> possibleNodes) {
+    String replica = "r" + replicaList.size();
+    String node = possibleNodes.get(random().nextInt(possibleNodes.size())); // place on a random node
+    Replica r = newReplica(collection, slice, replica, node);
+    replicaList.add(r);
+    return r;
+  }
+
+  protected Replica newReplica(String collection, String slice, String replica, String node) {
+    return new Replica(replica, Utils.makeMap("core", replica, "node_name", node), collection, slice);
 
 Review comment:
   `s/Utils.makeMap/map/`?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395422857
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
 
 Review comment:
   Doh!  And this is what forbiddenAPIs caught.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395417214
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
 
 Review comment:
   What's the value of combining what used to be two test methods into one?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on issue #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley commented on issue #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#issuecomment-601508721
 
 
   The previous CoreSorter had a getCloudDescriptors method that called CoreContainer.getCores().  But there are no cores registered yet!  It was a pitty because it was only calling this method to get the core descriptors... but of course the core descriptors should be the input to the CoreSorter and so it's silly to try to find them some other way.  I think this demonstrated that the API to use CoreSorter was more complicated than it needed to be; CoreSorter should have exactly one method that CoreContainer needs to call.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395422456
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
 
 Review comment:
   At least there are separate methods and so a test failure would indicate which from the call stack.
   I wanted both the existing "comparator" test and the new integration test to use the same input.  I suppose I could move the input into fields of the class and then have two test methods that operate on them.  LMK and I'll change it.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395417505
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
 
 Review comment:
   pass `random()`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395421972
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
+
+    // compute nodes, some live, some down
+    final int maxNodesOfAType = perShardCounts.stream() // not too important how many we have, but lets have plenty
+        .mapToInt(c -> c.totalReplicasInLiveNodes + c.totalReplicasInDownNodes + c.myReplicas).max().getAsInt();
+    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + "_8983").collect(Collectors.toList());
+    Collections.shuffle(liveNodes, random());
+    String thisNode = liveNodes.get(0);
+    List<String> otherLiveNodes = liveNodes.subList(1, liveNodes.size());
+    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + "_8983").collect(Collectors.toList());
+
+    // divide into two collections
+    int numCol1 = random().nextInt(perShardCounts.size());
+    Map<String,List<CountsForEachShard>> collToCounts = new HashMap<>();
+    collToCounts.put("col1", perShardCounts.subList(0, numCol1));
+    collToCounts.put("col2", perShardCounts.subList(numCol1, perShardCounts.size()));
+
+    Map<String,DocCollection> collToState = new HashMap<>();
+    Map<CountsForEachShard, List<CoreDescriptor>> myCountsToDescs = new HashMap<>();
+    for (Map.Entry<String, List<CountsForEachShard>> entry : collToCounts.entrySet()) {
+      String collection = entry.getKey();
+      List<CountsForEachShard> collCounts = entry.getValue();
+      Map<String, Slice> sliceMap = new HashMap<>(collCounts.size());
+      for (CountsForEachShard shardCounts : collCounts) {
+        String slice = "s" + shardCounts.hashCode();
+        List<Replica> replicas = new ArrayList<>();
+        for (int myRepNum = 0; myRepNum < shardCounts.myReplicas; myRepNum++) {
+          addNewReplica(replicas, collection, slice, Collections.singletonList(thisNode));
+          // save this mapping for later
+          myCountsToDescs.put(shardCounts, replicas.stream().map(this::newCoreDescriptor).collect(Collectors.toList()));
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInLiveNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, otherLiveNodes);
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInDownNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, downNodes);
+        }
+        Map<String, Replica> replicaMap = replicas.stream().collect(Collectors.toMap(Replica::getName, Function.identity()));
+        sliceMap.put(slice, new Slice(slice, replicaMap, map(), collection));
       }
-      return false;
+      DocCollection col = new DocCollection(collection, sliceMap, map(), DocRouter.DEFAULT);
+      collToState.put(collection, col);
     }
-
-
-    @Override
-    public int hashCode() {
-      return replicaName.hashCode();
-    }
-
-    CloudDescriptor getCloudDescriptor() {
-      return cd;
-
-    }
-
-    public Replica getReplica(String node) {
-      return new Replica(replicaName, Utils.makeMap("core", replicaName, "node_name", node), cd.getCollectionName(), cd.getShardId());
-    }
-
-    public boolean equals(String coll, String slice) {
-      return cd.getCollectionName().equals(coll) && slice.equals(cd.getShardId());
+    // reverse map
+    Map<CoreDescriptor, CountsForEachShard> myDescsToCounts = new HashMap<>();
+    for (Map.Entry<CountsForEachShard, List<CoreDescriptor>> entry : myCountsToDescs.entrySet()) {
+      for (CoreDescriptor descriptor : entry.getValue()) {
+        CountsForEachShard prev = myDescsToCounts.put(descriptor, entry.getKey());
+        assert prev == null;
 
 Review comment:
   I deliberately used a plain assert because this assert would mean the test _itself_ is broken.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on issue #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley commented on issue #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#issuecomment-603492062
 
 
   I'll commit in two days according to lazy concensus if there's no further input.
   
   The original issue was filed as a bug fix with a CHANGES.txt entry: "In cloud-mode sort the cores smartly before loading & limit threads to improve cluster stability".
   
   There were aspects to the original other than core load order -- namely limiting the number of threads that load cores at once from 24 to 8.
   
   I don't really think of core load order as a bug fix even if it helps with stability because this is a best-effort sort of thing.  It's an improvement (perhaps optimization).  AFAICT users would see this during cross-cluster simultaneous node startup but also to a degree even the local node since some collections should come online quickly if there isn't agreement needed on other nodes.  
   
   Proposed CHANGES entry:
   Improvement:
   * Load cores in an order that makes collections available sooner and reduces leaderVoteWait timeouts in large SolrCloud clusters.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395421862
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
+
+    // compute nodes, some live, some down
+    final int maxNodesOfAType = perShardCounts.stream() // not too important how many we have, but lets have plenty
+        .mapToInt(c -> c.totalReplicasInLiveNodes + c.totalReplicasInDownNodes + c.myReplicas).max().getAsInt();
+    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + "_8983").collect(Collectors.toList());
+    Collections.shuffle(liveNodes, random());
+    String thisNode = liveNodes.get(0);
+    List<String> otherLiveNodes = liveNodes.subList(1, liveNodes.size());
+    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + "_8983").collect(Collectors.toList());
+
+    // divide into two collections
+    int numCol1 = random().nextInt(perShardCounts.size());
 
 Review comment:
   I know; yup.  Then we get one collection and that's fine.  The actual collection distribution shouldn't matter but I wanted to add some multi-collection in there to mix things up.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395419361
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
+
+    // compute nodes, some live, some down
+    final int maxNodesOfAType = perShardCounts.stream() // not too important how many we have, but lets have plenty
+        .mapToInt(c -> c.totalReplicasInLiveNodes + c.totalReplicasInDownNodes + c.myReplicas).max().getAsInt();
+    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + "_8983").collect(Collectors.toList());
+    Collections.shuffle(liveNodes, random());
+    String thisNode = liveNodes.get(0);
+    List<String> otherLiveNodes = liveNodes.subList(1, liveNodes.size());
+    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + "_8983").collect(Collectors.toList());
+
+    // divide into two collections
+    int numCol1 = random().nextInt(perShardCounts.size());
 
 Review comment:
   this could be zero, is that ok?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1366: SOLR-14342: Optimize core loading order in SolrCloud.
URL: https://github.com/apache/lucene-solr/pull/1366#discussion_r395419608
 
 

 ##########
 File path: solr/core/src/test/org/apache/solr/core/CoreSorterTest.java
 ##########
 @@ -71,168 +69,141 @@ public void testComparator() {
 
     );
 
+    testComparator(expected, l);
+
+    integrationTest(expected, l);
+  }
+
+  private void testComparator(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> inputCounts) {
     for (int i = 0; i < 10; i++) {
-      List<CountsForEachShard> copy = new ArrayList<>(l);
+      List<CountsForEachShard> copy = new ArrayList<>(inputCounts);
       Collections.shuffle(copy, random());
       Collections.sort(copy, CoreSorter.countsComparator);
       for (int j = 0; j < copy.size(); j++) {
-        assertEquals(expected.get(j), copy.get(j));
+        assertEquals(expectedCounts.get(j), copy.get(j));
       }
     }
   }
 
-  public void testSort() throws Exception {
-    CoreContainer mockCC = getMockContainer();
-    MockCoreSorter coreSorter = (MockCoreSorter) new MockCoreSorter().init(mockCC);
-    List<CoreDescriptor> copy = new ArrayList<>(coreSorter.getLocalCores());
-    Collections.sort(copy, coreSorter::compare);
-    List<CountsForEachShard> l = copy.stream()
-        .map(CoreDescriptor::getCloudDescriptor)
-        .map(it -> coreSorter.shardsVsReplicaCounts.get(getShardName(it)))
-        .collect(toList());
-    for (int i = 1; i < l.size(); i++) {
-      CountsForEachShard curr = l.get(i);
-      CountsForEachShard prev = l.get(i-1);
-      assertTrue(CoreSorter.countsComparator.compare(prev, curr) < 1);
-    }
-
-    for (CountsForEachShard c : l) {
-      System.out.println(c);
-    }
-  }
-
-  private CoreContainer getMockContainer() {
+  private void integrationTest(List<CountsForEachShard> expectedCounts, List<CountsForEachShard> _inputCounts) {
     assumeWorkingMockito();
-    
-    CoreContainer mockCC = mock(CoreContainer.class);
-    ZkController mockZKC = mock(ZkController.class);
-    ClusterState mockClusterState = mock(ClusterState.class);
-    when(mockCC.isZooKeeperAware()).thenReturn(true);
-    when(mockCC.getZkController()).thenReturn(mockZKC);
-    when(mockClusterState.getLiveNodes()).thenReturn(liveNodes);
-    when(mockZKC.getClusterState()).thenReturn(mockClusterState);
-    return mockCC;
-  }
 
-  static class ReplicaInfo {
-    final int coll, slice, replica;
-    final String replicaName;
-    CloudDescriptor cd;
-
-    ReplicaInfo(int coll, int slice, int replica) {
-      this.coll = coll;
-      this.slice = slice;
-      this.replica = replica;
-      replicaName = "coll_" + coll + "_" + slice + "_" + replica;
-      Properties p = new Properties();
-      p.setProperty(CoreDescriptor.CORE_SHARD, "shard_" + slice);
-      p.setProperty(CoreDescriptor.CORE_COLLECTION, "coll_" + slice);
-      p.setProperty(CoreDescriptor.CORE_NODE_NAME, replicaName);
-      cd = new CloudDescriptor(null, replicaName, p);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-      if (obj instanceof ReplicaInfo) {
-        ReplicaInfo replicaInfo = (ReplicaInfo) obj;
-        return replicaInfo.replicaName.equals(replicaName);
+    List<CountsForEachShard> perShardCounts = new ArrayList<>(_inputCounts);
+    Collections.shuffle(perShardCounts);
+
+    // compute nodes, some live, some down
+    final int maxNodesOfAType = perShardCounts.stream() // not too important how many we have, but lets have plenty
+        .mapToInt(c -> c.totalReplicasInLiveNodes + c.totalReplicasInDownNodes + c.myReplicas).max().getAsInt();
+    List<String> liveNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.0." + i + "_8983").collect(Collectors.toList());
+    Collections.shuffle(liveNodes, random());
+    String thisNode = liveNodes.get(0);
+    List<String> otherLiveNodes = liveNodes.subList(1, liveNodes.size());
+    List<String> downNodes = IntStream.range(0, maxNodesOfAType).mapToObj(i -> "192.168.1." + i + "_8983").collect(Collectors.toList());
+
+    // divide into two collections
+    int numCol1 = random().nextInt(perShardCounts.size());
+    Map<String,List<CountsForEachShard>> collToCounts = new HashMap<>();
+    collToCounts.put("col1", perShardCounts.subList(0, numCol1));
+    collToCounts.put("col2", perShardCounts.subList(numCol1, perShardCounts.size()));
+
+    Map<String,DocCollection> collToState = new HashMap<>();
+    Map<CountsForEachShard, List<CoreDescriptor>> myCountsToDescs = new HashMap<>();
+    for (Map.Entry<String, List<CountsForEachShard>> entry : collToCounts.entrySet()) {
+      String collection = entry.getKey();
+      List<CountsForEachShard> collCounts = entry.getValue();
+      Map<String, Slice> sliceMap = new HashMap<>(collCounts.size());
+      for (CountsForEachShard shardCounts : collCounts) {
+        String slice = "s" + shardCounts.hashCode();
+        List<Replica> replicas = new ArrayList<>();
+        for (int myRepNum = 0; myRepNum < shardCounts.myReplicas; myRepNum++) {
+          addNewReplica(replicas, collection, slice, Collections.singletonList(thisNode));
+          // save this mapping for later
+          myCountsToDescs.put(shardCounts, replicas.stream().map(this::newCoreDescriptor).collect(Collectors.toList()));
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInLiveNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, otherLiveNodes);
+        }
+        for (int myRepNum = 0; myRepNum < shardCounts.totalReplicasInDownNodes; myRepNum++) {
+          addNewReplica(replicas, collection, slice, downNodes);
+        }
+        Map<String, Replica> replicaMap = replicas.stream().collect(Collectors.toMap(Replica::getName, Function.identity()));
+        sliceMap.put(slice, new Slice(slice, replicaMap, map(), collection));
       }
-      return false;
+      DocCollection col = new DocCollection(collection, sliceMap, map(), DocRouter.DEFAULT);
+      collToState.put(collection, col);
     }
-
-
-    @Override
-    public int hashCode() {
-      return replicaName.hashCode();
-    }
-
-    CloudDescriptor getCloudDescriptor() {
-      return cd;
-
-    }
-
-    public Replica getReplica(String node) {
-      return new Replica(replicaName, Utils.makeMap("core", replicaName, "node_name", node), cd.getCollectionName(), cd.getShardId());
-    }
-
-    public boolean equals(String coll, String slice) {
-      return cd.getCollectionName().equals(coll) && slice.equals(cd.getShardId());
+    // reverse map
+    Map<CoreDescriptor, CountsForEachShard> myDescsToCounts = new HashMap<>();
+    for (Map.Entry<CountsForEachShard, List<CoreDescriptor>> entry : myCountsToDescs.entrySet()) {
+      for (CoreDescriptor descriptor : entry.getValue()) {
+        CountsForEachShard prev = myDescsToCounts.put(descriptor, entry.getKey());
+        assert prev == null;
 
 Review comment:
   junit assert?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org