You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2020/12/03 15:59:13 UTC

[lucene-solr] branch jira/solr-15016 updated: SOLR-15016: Fix issues found in review.

This is an automated email from the ASF dual-hosted git repository.

ab pushed a commit to branch jira/solr-15016
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/jira/solr-15016 by this push:
     new 9a2865b  SOLR-15016: Fix issues found in review.
9a2865b is described below

commit 9a2865b94045b061547bf85e57a47627f4889e45
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Thu Dec 3 16:58:51 2020 +0100

    SOLR-15016: Fix issues found in review.
---
 .../apache/solr/api/ContainerPluginsRegistry.java  |  6 ++---
 .../apache/solr/cloud/api/collections/Assign.java  |  2 ++
 .../impl/CollectionsRepairEventListener.java       | 29 +++++++++-------------
 .../cluster/placement/PlacementPluginFactory.java  |  2 ++
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java b/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
index 453b092..b9cc6c7 100644
--- a/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
+++ b/solr/core/src/java/org/apache/solr/api/ContainerPluginsRegistry.java
@@ -58,7 +58,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static org.apache.lucene.util.IOUtils.closeWhileHandlingException;
-import static org.apache.solr.common.util.Utils.makeMap;
 
 /**
  * This class manages the container-level plugins and their Api-s. It is
@@ -143,7 +142,7 @@ public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapW
   }
   @SuppressWarnings("unchecked")
   public synchronized void refresh() {
-    Map<String, Object> pluginInfos = null;
+    Map<String, Object> pluginInfos;
     try {
       pluginInfos = ContainerPluginsApi.plugins(coreContainer.zkClientSupplier);
     } catch (IOException e) {
@@ -236,9 +235,8 @@ public class ContainerPluginsRegistry implements ClusterPropertiesListener, MapW
     return path;
   }
 
-  @SuppressWarnings({"rawtypes", "unchecked"})
   private static  Map<String, String> getTemplateVars(PluginMeta pluginMeta) {
-    return (Map) makeMap("plugin-name", pluginMeta.name, "path-prefix", pluginMeta.pathPrefix);
+    return Map.of("plugin-name", pluginMeta.name, "path-prefix", pluginMeta.pathPrefix);
   }
 
   private static class ApiHolder extends Api {
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
index 0c249c9..786bfa9 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
@@ -492,6 +492,8 @@ public class Assign {
   /**
    * Creates the appropriate instance of {@link AssignStrategy} based on how the cluster and/or individual collections are
    * configured.
+   * <p>If {@link PlacementPlugin} instance is null this call will return {@link LegacyAssignStrategy}, otherwise
+   * {@link PlacementPluginAssignStrategy} will be used.</p>
    */
   public static AssignStrategy createAssignStrategy(PlacementPlugin placementPlugin, ClusterState clusterState, DocCollection collection) {
     if (placementPlugin != null) {
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/impl/CollectionsRepairEventListener.java b/solr/core/src/java/org/apache/solr/cluster/events/impl/CollectionsRepairEventListener.java
index fd5f610..8984d1d 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/impl/CollectionsRepairEventListener.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/impl/CollectionsRepairEventListener.java
@@ -41,8 +41,8 @@ import org.apache.solr.cloud.api.collections.Assign;
 import org.apache.solr.cluster.events.ClusterEvent;
 import org.apache.solr.cluster.events.ClusterEventListener;
 import org.apache.solr.cluster.events.NodesDownEvent;
+import org.apache.solr.cluster.placement.PlacementPluginConfig;
 import org.apache.solr.cluster.placement.PlacementPluginFactory;
-import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.cloud.ClusterState;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.ReplicaPosition;
@@ -78,7 +78,7 @@ public class CollectionsRepairEventListener implements ClusterEventListener, Clu
   private int waitForSecond = DEFAULT_WAIT_FOR_SEC;
 
   private ScheduledThreadPoolExecutor waitForExecutor;
-  private PlacementPluginFactory<? extends MapWriter> placementPluginFactory;
+  private final PlacementPluginFactory<? extends PlacementPluginConfig> placementPluginFactory;
 
   public CollectionsRepairEventListener(CoreContainer cc) {
     this.solrClient = cc.getSolrClientCache().getCloudSolrClient(cc.getZkController().getZkClient().getZkServerAddress());
@@ -114,7 +114,7 @@ public class CollectionsRepairEventListener implements ClusterEventListener, Clu
     }
   }
 
-  private Map<String, Long> nodeNameVsTimeRemoved = new ConcurrentHashMap<>();
+  private final Map<String, Long> nodeNameVsTimeRemoved = new ConcurrentHashMap<>();
 
   private void handleNodesDown(NodesDownEvent event) {
 
@@ -125,9 +125,7 @@ public class CollectionsRepairEventListener implements ClusterEventListener, Clu
     Set<String> trackingKeySet = nodeNameVsTimeRemoved.keySet();
     trackingKeySet.removeAll(solrCloudManager.getClusterStateProvider().getLiveNodes());
     // add any new lost nodes (old lost nodes are skipped)
-    event.getNodeNames().forEachRemaining(lostNode -> {
-      nodeNameVsTimeRemoved.computeIfAbsent(lostNode, n -> solrCloudManager.getTimeSource().getTimeNs());
-    });
+    event.getNodeNames().forEachRemaining(lostNode -> nodeNameVsTimeRemoved.computeIfAbsent(lostNode, n -> solrCloudManager.getTimeSource().getTimeNs()));
   }
 
   private void runRepair() {
@@ -195,7 +193,6 @@ public class CollectionsRepairEventListener implements ClusterEventListener, Clu
             newPositions.put(coll.getName(), positions);
           } catch (Exception e) {
             log.warn("Exception computing positions for {}/{}: {}", coll.getName(), shard, e);
-            return;
           }
         });
       });
@@ -210,15 +207,13 @@ public class CollectionsRepairEventListener implements ClusterEventListener, Clu
     // send ADDREPLICA admin requests for each lost replica
     // XXX should we use 'async' for that, to avoid blocking here?
     List<CollectionAdminRequest.AddReplica> addReplicas = new ArrayList<>();
-    newPositions.forEach((collection, positions) -> {
-      positions.forEach(position -> {
-        CollectionAdminRequest.AddReplica addReplica = CollectionAdminRequest
-            .addReplicaToShard(collection, position.shard, position.type);
-        addReplica.setNode(position.node);
-        addReplica.setAsyncId(ASYNC_ID_PREFIX + counter.incrementAndGet());
-        addReplicas.add(addReplica);
-      });
-    });
+    newPositions.forEach((collection, positions) -> positions.forEach(position -> {
+      CollectionAdminRequest.AddReplica addReplica = CollectionAdminRequest
+          .addReplicaToShard(collection, position.shard, position.type);
+      addReplica.setNode(position.node);
+      addReplica.setAsyncId(ASYNC_ID_PREFIX + counter.incrementAndGet());
+      addReplicas.add(addReplica);
+    }));
     addReplicas.forEach(addReplica -> {
       try {
         solrClient.request(addReplica);
@@ -235,7 +230,7 @@ public class CollectionsRepairEventListener implements ClusterEventListener, Clu
         new SolrNamedThreadFactory("collectionsRepair_waitFor"));
     waitForExecutor.setRemoveOnCancelPolicy(true);
     waitForExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
-    waitForExecutor.scheduleAtFixedRate(() -> runRepair(), 0, waitForSecond, TimeUnit.SECONDS);
+    waitForExecutor.scheduleAtFixedRate(this::runRepair, 0, waitForSecond, TimeUnit.SECONDS);
     state = State.RUNNING;
   }
 
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java
index bed9ea9..1358a93 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/PlacementPluginFactory.java
@@ -36,6 +36,8 @@ public interface PlacementPluginFactory<T extends PlacementPluginConfig> extends
    * Returns an instance of the plugin that will be repeatedly (and concurrently) called to compute placement. Multiple
    * instances of a plugin can be used in parallel (for example if configuration has to change, but plugin instances with
    * the previous configuration are still being used).
+   * <p>If this method returns null then a simple legacy assignment strategy will be used
+   * (see {@link org.apache.solr.cloud.api.collections.Assign.LegacyAssignStrategy}).</p>
    */
   PlacementPlugin createPluginInstance();