cleanups in o.a.s.cluster.events files

solr/core/src/java/org/apache/solr/cluster/events/ClusterEvent.java

@@ -32,8 +32,8 @@ enum EventType {
     COLLECTIONS_REMOVED,
     /** Cluster properties have changed. */
     CLUSTER_PROPERTIES_CHANGED
-    // other types? eg. Overseer leader change, shard leader change,
-    // node overload (eg. CPU / MEM circuit breakers tripped)?
+    // other types? e.g. Overseer leader change, shard leader change,
+    // node overload (e.g. CPU / MEM circuit breakers tripped)?
   }
 
   /** Get event type. */

solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducerBase.java

@@ -84,9 +84,7 @@ public State getState() {
   @Override
   public void close() throws IOException {
     synchronized (listeners) {
-      listeners
-          .values()
-          .forEach(listenerSet -> listenerSet.forEach(listener -> IOUtils.closeQuietly(listener)));
+      listeners.values().forEach(listenerSet -> listenerSet.forEach(IOUtils::closeQuietly));
     }
   }
 

solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerFactory.java

@@ -93,13 +93,13 @@ public ContainerPluginsRegistry.PluginRegistryListener getPluginRegistryListener
   /**
    * Create a {@link ClusterEventProducer} based on the current plugin configurations.
    *
-   * <p>NOTE: this method can only be called once because it has side-effects, such as transferring
+   * <p>NOTE: this method can only be called once because it has side effects, such as transferring
    * the initially collected listeners to the resulting producer's instance, and installing a {@link
    * org.apache.solr.api.ContainerPluginsRegistry.PluginRegistryListener}. Calling this method more
    * than once will result in an exception.
    *
    * @param plugins current plugin configurations
-   * @return configured instance of cluster event producer (with side-effects, see above)
+   * @return configured instance of cluster event producer (with side effects, see above)
    */
   public DelegatingClusterEventProducer create(ContainerPluginsRegistry plugins) {
     if (created) {
@@ -201,9 +201,8 @@ private void transferListeners(ClusterEventProducer target, ContainerPluginsRegi
       plugins.unregisterListener(initialPluginListener);
       // transfer listeners that are already registered
       listeners.forEach(
-          (type, listenersSet) -> {
-            listenersSet.forEach(listener -> target.registerListener(listener, type));
-          });
+          (type, listenersSet) ->
+              listenersSet.forEach(listener -> target.registerListener(listener, type)));
       listeners.clear();
     }
   }

solr/core/src/java/org/apache/solr/cluster/events/impl/CollectionsRepairEventListener.java

@@ -195,10 +195,7 @@ private void runRepair() {
                         newPositions.put(coll.getName(), positions);
                       } catch (Exception e) {
                         log.warn(
-                            "Exception computing positions for {}/{}: {}",
-                            coll.getName(),
-                            shard,
-                            e);
+                            "Exception computing positions for {}/{}", coll.getName(), shard, e);
                       }
                     });
               });
@@ -229,8 +226,7 @@ private void runRepair() {
           try {
             solrClient.request(addReplica);
           } catch (Exception e) {
-            log.warn(
-                "Exception calling ADDREPLICA {}: {}", addReplica.getParams().toQueryString(), e);
+            log.warn("Exception calling ADDREPLICA {}", addReplica.getParams().toQueryString(), e);
           }
         });
   }

solr/core/src/java/org/apache/solr/cluster/events/impl/DelegatingClusterEventProducer.java

@@ -96,7 +96,7 @@ public void setDelegate(ClusterEventProducer newDelegate) {
           log.debug("--- started delegate {}", delegate);
         }
       } catch (Exception e) {
-        log.warn("Unable to start the new delegate {}: {}", delegate.getClass().getName(), e);
+        log.warn("Unable to start the new delegate {}", delegate.getClass().getName(), e);
       }
     } else {
       if (log.isDebugEnabled()) {

solr/core/src/java/org/apache/solr/cluster/placement/AttributeFetcher.java

@@ -65,7 +65,7 @@ AttributeFetcher requestCollectionMetrics(
   /**
    * Fetches all requested node attributes from all nodes passed to {@link #fetchFrom(Set)} as well
    * as non-node attributes (those requested using e.g. {@link
-   * #requestCollectionMetrics(SolrCollection, Set)}.
+   * #requestCollectionMetrics(SolrCollection, Set)}).
    *
    * @return An instance allowing retrieval of all attributes that could be fetched.
    */

solr/core/src/java/org/apache/solr/cluster/placement/BalanceRequest.java

@@ -43,6 +43,4 @@ public interface BalanceRequest extends ModificationRequest {
    *     the error itself rather than calling the plugin).
    */
   Set<Node> getNodes();
-
-  int getMaximumBalanceSkew();
 }

solr/core/src/java/org/apache/solr/cluster/placement/CollectionMetrics.java

@@ -21,7 +21,7 @@
 import java.util.Optional;
 
 /**
- * Collection-level metrics. Currently this class is a container just for shard-level metrics but
+ * Collection-level metrics. Currently, this class is a container just for shard-level metrics but
  * future versions may add other primitive collection-level metrics.
  */
 public interface CollectionMetrics {

solr/core/src/java/org/apache/solr/cluster/placement/Metric.java

@@ -23,7 +23,7 @@
  */
 public interface Metric<T> {
 
-  /** Return the short-hand name that identifies this attribute. */
+  /** Return the shorthand name that identifies this attribute. */
   String getName();
 
   /** Return the internal name of a Solr metric associated with this attribute. */

solr/core/src/java/org/apache/solr/cluster/placement/PlacementContext.java

@@ -33,7 +33,7 @@ public interface PlacementContext {
 
   /**
    * Factory used by the plugin to fetch additional attributes from the cluster nodes, such as count
-   * of cores, system properties etc..
+   * of cores, system properties etc.
    */
   AttributeFetcher getAttributeFetcher();
 

solr/core/src/java/org/apache/solr/cluster/placement/PlacementException.java

@@ -39,9 +39,4 @@ public PlacementException(String message, Throwable cause) {
   public PlacementException(Throwable cause) {
     super(cause);
   }
-
-  protected PlacementException(
-      String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
-    super(message, cause, enableSuppression, writableStackTrace);
-  }
 }

solr/core/src/java/org/apache/solr/cluster/placement/PlacementModificationException.java

@@ -27,10 +27,6 @@
 public class PlacementModificationException extends PlacementException {
   private final Map<String, String> rejectedModifications = new HashMap<>();
 
-  public PlacementModificationException() {
-    super();
-  }
-
   public PlacementModificationException(String message) {
     super(message);
   }
@@ -39,10 +35,6 @@ public PlacementModificationException(String message, Throwable cause) {
     super(message, cause);
   }
 
-  public PlacementModificationException(Throwable cause) {
-    super(cause);
-  }
-
   /**
    * Add information about the modification that cause this exception.
    *

solr/core/src/java/org/apache/solr/cluster/placement/PlacementPlugin.java

@@ -98,5 +98,5 @@ BalancePlan computeBalancing(BalanceRequest balanceRequest, PlacementContext pla
    */
   default void verifyAllowedModification(
       ModificationRequest modificationRequest, PlacementContext placementContext)
-      throws PlacementException, InterruptedException {}
+      throws PlacementException {}
 }

solr/core/src/java/org/apache/solr/cluster/placement/ShardMetrics.java

@@ -20,7 +20,7 @@
 import java.util.Optional;
 
 /**
- * Shard-level metrics. Currently this is just a container for replica-level metrics but future
+ * Shard-level metrics. Currently, this is just a container for replica-level metrics but future
  * versions may add other primitive shard-level metrics.
  */
 public interface ShardMetrics {

solr/core/src/java/org/apache/solr/cluster/placement/impl/AttributeFetcherImpl.java

@@ -101,7 +101,7 @@ public AttributeValues fetchAttributes() {
     // In order to match the returned values for the various snitches, we need to keep track of
     // where each received value goes. Given the target maps are of different types (the maps from
     // Node to whatever defined above) we instead pass a function taking two arguments, the node and
-    // the (non null) returned value, that will cast the value into the appropriate type for the
+    // the (non-null) returned value, that will cast the value into the appropriate type for the
     // snitch tag and insert it into the appropriate map with the node as the key.
     for (String sysPropSnitch : requestedNodeSystemSnitchTags) {
       systemSnitchToNodeToValue.put(sysPropSnitch, new HashMap<>());
@@ -207,11 +207,10 @@ private void fetchNodeValues(
       Map<String, Map<Node, String>> systemSnitchToNodeToValue,
       Map<NodeMetric<?>, Map<Node, Object>> metricSnitchToNodeToValue) {
 
-    Set<String> allRequestedTags = new HashSet<>();
     Map<String, NodeMetric<?>> tagToMetric = new HashMap<>();
 
     // Add system property tags we need to request
-    allRequestedTags.addAll(requestedNodeSystemSnitchTags);
+    Set<String> allRequestedTags = new HashSet<>(requestedNodeSystemSnitchTags);
 
     // Add metric tags we need to request
     for (NodeMetric<?> metric : requestedNodeMetricSnitchTags) {

solr/core/src/java/org/apache/solr/cluster/placement/impl/BalanceRequestImpl.java

@@ -45,11 +45,6 @@ public Set<Node> getNodes() {
     return nodes;
   }
 
-  @Override
-  public int getMaximumBalanceSkew() {
-    return maximumBalanceSkew;
-  }
-
   @Override
   public SolrCollection getCollection() {
     return solrCollection;

solr/core/src/java/org/apache/solr/cluster/placement/impl/MetricImpl.java

@@ -68,7 +68,7 @@ public abstract class MetricImpl<T> implements Metric<T> {
   /**
    * Create a metric attribute.
    *
-   * @param name short-hand name that identifies this attribute.
+   * @param name shorthand name that identifies this attribute.
    * @param internalName internal name of a Solr metric.
    */
   public MetricImpl(String name, String internalName) {
@@ -78,7 +78,7 @@ public MetricImpl(String name, String internalName) {
   /**
    * Create a metric attribute.
    *
-   * @param name short-hand name that identifies this attribute.
+   * @param name shorthand name that identifies this attribute.
    * @param internalName internal name of a Solr metric.
    * @param converter optional raw value converter. If null then {@link #IDENTITY_CONVERTER} will be
    *     used.
@@ -90,7 +90,7 @@ public MetricImpl(String name, String internalName, Function<Object, T> converte
   /**
    * Create a metric attribute with labels.
    *
-   * @param name short-hand name that identifies this attribute.
+   * @param name shorthand name that identifies this attribute.
    * @param internalName internal name of a Solr metric.
    * @param labelKey optional label key for Prometheus format labeled metrics.
    * @param labelValue optional label value for Prometheus format labeled metrics.

solr/core/src/java/org/apache/solr/cluster/placement/impl/ModificationRequestImpl.java

@@ -24,9 +24,7 @@
 import org.apache.solr.cluster.SolrCollection;
 import org.apache.solr.cluster.placement.DeleteCollectionRequest;
 import org.apache.solr.cluster.placement.DeleteReplicasRequest;
-import org.apache.solr.cluster.placement.DeleteShardsRequest;
 import org.apache.solr.common.cloud.DocCollection;
-import org.apache.solr.common.cloud.Slice;
 
 /** Helper class to create modification request instances. */
 public class ModificationRequestImpl {
@@ -77,39 +75,11 @@ public static DeleteReplicasRequest createDeleteReplicasRequest(
     SolrCollection solrCollection =
         SimpleClusterAbstractionsImpl.SolrCollectionImpl.fromDocCollection(docCollection);
     Shard shard = solrCollection.getShard(shardName);
-    Slice slice = docCollection.getSlice(shardName);
     Set<Replica> solrReplicas = new HashSet<>();
     replicas.forEach(
         replica -> {
           solrReplicas.add(shard.getReplica(replica.getName()));
         });
     return createDeleteReplicasRequest(solrCollection, solrReplicas);
   }
-
-  public static DeleteShardsRequest createDeleteShardsRequest(
-      SolrCollection collection, Set<String> shardNames) {
-    return new DeleteShardsRequest() {
-      @Override
-      public Set<String> getShardNames() {
-        return shardNames;
-      }
-
-      @Override
-      public SolrCollection getCollection() {
-        return collection;
-      }
-
-      @Override
-      public String toString() {
-        return "DeleteShardsRequest{collection=" + collection.getName() + ",shards=" + shardNames;
-      }
-    };
-  }
-
-  public static DeleteShardsRequest createDeleteShardsRequest(
-      DocCollection docCollection, Set<String> shardNames) {
-    SolrCollection solrCollection =
-        SimpleClusterAbstractionsImpl.SolrCollectionImpl.fromDocCollection(docCollection);
-    return createDeleteShardsRequest(solrCollection, shardNames);
-  }
 }

solr/core/src/java/org/apache/solr/cluster/placement/impl/NodeMetricImpl.java

@@ -56,15 +56,6 @@ public NodeMetricImpl(String key, Function<Object, T> converter) {
     super(key, key, null, null, converter);
   }
 
-  public NodeMetricImpl(
-      String name,
-      String internalName,
-      String labelKey,
-      String labelValue,
-      Function<Object, T> converter) {
-    super(name, internalName, labelKey, labelValue, converter);
-  }
-
   public NodeMetricImpl(String key) {
     super(key, key);
   }

solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginAssignStrategy.java

@@ -18,7 +18,6 @@
 package org.apache.solr.cluster.placement.impl;
 
 import java.io.IOException;
-import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -41,14 +40,10 @@
 import org.apache.solr.common.cloud.ReplicaPosition;
 import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.util.CollectionUtil;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /** This assign strategy delegates placement computation to "plugin" code. */
 public class PlacementPluginAssignStrategy implements Assign.AssignStrategy {
 
-  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-
   private final PlacementPlugin plugin;
 
   public PlacementPluginAssignStrategy(PlacementPlugin plugin) {
@@ -127,7 +122,7 @@ public Map<Replica, String> computeReplicaBalancing(
 
   private Replica findReplica(
       SolrCloudManager solrCloudManager, org.apache.solr.cluster.Replica replica) {
-    DocCollection collection = null;
+    DocCollection collection;
     try {
       collection =
           solrCloudManager

solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementPluginFactoryLoader.java

@@ -108,22 +108,17 @@ public static PlacementPluginFactory<?> getDefaultPlacementPluginFactory() {
           "Default replica placement plugin set in {} to {}",
           PLACEMENTPLUGIN_DEFAULT_SYSPROP,
           defaultPluginId);
-      switch (defaultPluginId.toLowerCase(Locale.ROOT)) {
-        case "simple":
-          return new SimplePlacementFactory();
-        case "affinity":
-          return new AffinityPlacementFactory();
-        case "minimizecores":
-          return new MinimizeCoresPlacementFactory();
-        case "random":
-          return new RandomPlacementFactory();
-        default:
-          throw new SolrException(
-              SolrException.ErrorCode.SERVER_ERROR,
-              "Invalid value for system property '"
-                  + PLACEMENTPLUGIN_DEFAULT_SYSPROP
-                  + "'. Supported values are 'simple', 'random', 'affinity' and 'minimizecores'");
-      }
+      return switch (defaultPluginId.toLowerCase(Locale.ROOT)) {
+        case "simple" -> new SimplePlacementFactory();
+        case "affinity" -> new AffinityPlacementFactory();
+        case "minimizecores" -> new MinimizeCoresPlacementFactory();
+        case "random" -> new RandomPlacementFactory();
+        default -> throw new SolrException(
+            SolrException.ErrorCode.SERVER_ERROR,
+            "Invalid value for system property '"
+                + PLACEMENTPLUGIN_DEFAULT_SYSPROP
+                + "'. Supported values are 'simple', 'random', 'affinity' and 'minimizecores'");
+      };
     } else {
       // TODO: Consider making the ootb default AffinityPlacementFactory, see
       // https://issues.apache.org/jira/browse/SOLR-16492