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/09/21 11:03:28 UTC

[lucene-solr] branch jira/solr-14749 updated: SOLR-14749: Fix issues from review.

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

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


The following commit(s) were added to refs/heads/jira/solr-14749 by this push:
     new 346d07f  SOLR-14749: Fix issues from review.
346d07f is described below

commit 346d07f34e540e78d017fa3fd2e3eeb6e397faa7
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Mon Sep 21 13:03:01 2020 +0200

    SOLR-14749: Fix issues from review.
---
 .../solr/cluster/events/ClusterEventListener.java  |  7 --
 .../solr/cluster/events/ClusterEventProducer.java  | 42 ++++++++--
 .../events/ClusterPropertiesChangedEvent.java      |  2 -
 .../solr/cluster/events/CollectionsAddedEvent.java |  4 +-
 .../cluster/events/CollectionsRemovedEvent.java    |  4 +-
 .../apache/solr/cluster/events/NodesDownEvent.java |  4 +-
 .../apache/solr/cluster/events/NodesUpEvent.java   |  4 +-
 .../solr/cluster/events/ReplicasDownEvent.java     |  4 +-
 .../events/impl/ClusterEventProducerImpl.java      | 40 ++++------
 .../impl/CollectionsRepairEventListener.java       | 93 +++++++++++++++++-----
 .../solr/cluster/events/AllEventsListener.java     | 59 ++++++++++++++
 .../cluster/events/ClusterEventProducerTest.java   | 64 +++++----------
 .../impl/CollectionsRepairEventListenerTest.java   | 55 +++++++++++++
 13 files changed, 267 insertions(+), 115 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventListener.java b/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventListener.java
index adf2e2b..6f84457 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventListener.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventListener.java
@@ -16,8 +16,6 @@
  */
 package org.apache.solr.cluster.events;
 
-import java.util.Set;
-
 /**
  * Components that want to be notified of cluster-wide events should use this.
  *
@@ -27,11 +25,6 @@ import java.util.Set;
 public interface ClusterEventListener {
 
   /**
-   * The types of events that this listener can process.
-   */
-  Set<ClusterEvent.EventType> getEventTypes();
-
-  /**
    * Handle the event. Implementations should be non-blocking - if any long
    * processing is needed it should be performed asynchronously.
    * @param event cluster event
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java b/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
index 7832ee5..cd47d0e 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/ClusterEventProducer.java
@@ -18,8 +18,10 @@ package org.apache.solr.cluster.events;
 
 import org.apache.solr.cloud.ClusterSingleton;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -37,16 +39,21 @@ public interface ClusterEventProducer extends ClusterSingleton {
   Map<ClusterEvent.EventType, Set<ClusterEventListener>> getEventListeners();
 
   /**
-   * Register an event listener. This listener will be notified about event
-   * of the types that it declares in {@link ClusterEventListener#getEventTypes()}
+   * Register an event listener for processing the specified event types.
    * @param listener non-null listener. If the same instance of the listener is
    *                 already registered it will be ignored.
+   * @param eventTypes non-empty array of event types that this listener
+   *                   is being registered for. If this is null or empty then all types will be used.
    */
-  default void registerListener(ClusterEventListener listener) throws Exception {
-    listener.getEventTypes().forEach(type -> {
+  default void registerListener(ClusterEventListener listener, ClusterEvent.EventType... eventTypes) throws Exception {
+    Objects.requireNonNull(listener);
+    if (eventTypes == null || eventTypes.length == 0) {
+      eventTypes = ClusterEvent.EventType.values();
+    }
+    for (ClusterEvent.EventType type : eventTypes) {
       Set<ClusterEventListener> perType = getEventListeners().computeIfAbsent(type, t -> ConcurrentHashMap.newKeySet());
       perType.add(listener);
-    });
+    }
   }
 
   /**
@@ -54,9 +61,28 @@ public interface ClusterEventProducer extends ClusterSingleton {
    * @param listener non-null listener.
    */
   default void unregisterListener(ClusterEventListener listener) {
-    listener.getEventTypes().forEach(type ->
-        getEventListeners().getOrDefault(type, Collections.emptySet()).remove(listener)
-    );
+    Objects.requireNonNull(listener);
+    getEventListeners().forEach((type, listeners) -> {
+      listeners.remove(listener);
+    });
+  }
+
+  /**
+   * Unregister an event listener for specified event types.
+   * @param listener non-null listener.
+   * @param eventTypes event types from which the listener will be unregistered. If this
+   *                   is null or empty then all event types will be used
+   */
+  default void unregisterListener(ClusterEventListener listener, ClusterEvent.EventType... eventTypes) {
+    Objects.requireNonNull(listener);
+    if (eventTypes == null || eventTypes.length == 0) {
+      eventTypes = ClusterEvent.EventType.values();
+    }
+    for (ClusterEvent.EventType type : eventTypes) {
+      getEventListeners()
+          .getOrDefault(type, Collections.emptySet())
+          .remove(listener);
+    }
   }
 
   /**
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/ClusterPropertiesChangedEvent.java b/solr/core/src/java/org/apache/solr/cluster/events/ClusterPropertiesChangedEvent.java
index 0906e7e..ee513d8 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/ClusterPropertiesChangedEvent.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/ClusterPropertiesChangedEvent.java
@@ -28,8 +28,6 @@ public interface ClusterPropertiesChangedEvent extends ClusterEvent {
     return EventType.CLUSTER_PROPERTIES_CHANGED;
   }
 
-  Map<String, Object> getOldClusterProperties();
-
   Map<String, Object> getNewClusterProperties();
 
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/CollectionsAddedEvent.java b/solr/core/src/java/org/apache/solr/cluster/events/CollectionsAddedEvent.java
index baeae52..0b1c46b 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/CollectionsAddedEvent.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/CollectionsAddedEvent.java
@@ -16,7 +16,7 @@
  */
 package org.apache.solr.cluster.events;
 
-import java.util.Collection;
+import java.util.Iterator;
 
 /**
  * Event generated when some collections have been added.
@@ -28,5 +28,5 @@ public interface CollectionsAddedEvent extends ClusterEvent {
     return EventType.COLLECTIONS_ADDED;
   }
 
-  Collection<String> getCollectionNames();
+  Iterator<String> getCollectionNames();
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/CollectionsRemovedEvent.java b/solr/core/src/java/org/apache/solr/cluster/events/CollectionsRemovedEvent.java
index 36327a9..e6fc64e 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/CollectionsRemovedEvent.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/CollectionsRemovedEvent.java
@@ -16,7 +16,7 @@
  */
 package org.apache.solr.cluster.events;
 
-import java.util.Collection;
+import java.util.Iterator;
 
 /**
  * Event generated when some collections have been removed.
@@ -28,5 +28,5 @@ public interface CollectionsRemovedEvent extends ClusterEvent {
     return EventType.COLLECTIONS_REMOVED;
   }
 
-  Collection<String> getCollectionNames();
+  Iterator<String> getCollectionNames();
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/NodesDownEvent.java b/solr/core/src/java/org/apache/solr/cluster/events/NodesDownEvent.java
index d749ac8..a8e7a2e 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/NodesDownEvent.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/NodesDownEvent.java
@@ -16,7 +16,7 @@
  */
 package org.apache.solr.cluster.events;
 
-import java.util.Collection;
+import java.util.Iterator;
 
 /**
  * Event generated when some nodes went down.
@@ -28,5 +28,5 @@ public interface NodesDownEvent extends ClusterEvent {
     return EventType.NODES_DOWN;
   }
 
-  Collection<String> getNodeNames();
+  Iterator<String> getNodeNames();
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/NodesUpEvent.java b/solr/core/src/java/org/apache/solr/cluster/events/NodesUpEvent.java
index c947aac..f83bf91 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/NodesUpEvent.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/NodesUpEvent.java
@@ -16,7 +16,7 @@
  */
 package org.apache.solr.cluster.events;
 
-import java.util.Collection;
+import java.util.Iterator;
 
 /**
  * Event generated when some nodes went up.
@@ -28,5 +28,5 @@ public interface NodesUpEvent extends ClusterEvent {
     return EventType.NODES_UP;
   }
 
-  Collection<String> getNodeNames();
+  Iterator<String> getNodeNames();
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/ReplicasDownEvent.java b/solr/core/src/java/org/apache/solr/cluster/events/ReplicasDownEvent.java
index 5d2c3e7..69ec48c 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/ReplicasDownEvent.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/ReplicasDownEvent.java
@@ -18,7 +18,7 @@ package org.apache.solr.cluster.events;
 
 import org.apache.solr.common.cloud.Replica;
 
-import java.util.Collection;
+import java.util.Iterator;
 
 /**
  * Event generated when some replicas went down.
@@ -30,5 +30,5 @@ public interface ReplicasDownEvent extends ClusterEvent {
     return EventType.REPLICAS_DOWN;
   }
 
-  Collection<Replica> getReplicas();
+  Iterator<Replica> getReplicas();
 }
diff --git a/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerImpl.java b/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerImpl.java
index d8f5082..5886d67 100644
--- a/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/events/impl/ClusterEventProducerImpl.java
@@ -24,6 +24,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
@@ -60,7 +61,6 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
   private LiveNodesListener liveNodesListener;
   private CloudCollectionsListener cloudCollectionsListener;
   private ClusterPropertiesListener clusterPropertiesListener;
-  private Map<String, Object> lastClusterProperties;
   private ZkController zkController;
   private boolean running;
 
@@ -110,8 +110,8 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
       if (!downNodes.isEmpty()) {
         fireEvent(new NodesDownEvent() {
           @Override
-          public Collection<String> getNodeNames() {
-            return downNodes;
+          public Iterator<String> getNodeNames() {
+            return downNodes.iterator();
           }
 
           @Override
@@ -125,8 +125,8 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
       if (!upNodes.isEmpty()) {
         fireEvent(new NodesUpEvent() {
           @Override
-          public Collection<String> getNodeNames() {
-            return upNodes;
+          public Iterator<String> getNodeNames() {
+            return upNodes.iterator();
           }
 
           @Override
@@ -149,8 +149,8 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
       if (!removed.isEmpty()) {
         fireEvent(new CollectionsRemovedEvent() {
           @Override
-          public Collection<String> getCollectionNames() {
-            return removed;
+          public Iterator<String> getCollectionNames() {
+            return removed.iterator();
           }
 
           @Override
@@ -164,8 +164,8 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
       if (!added.isEmpty()) {
         fireEvent(new CollectionsAddedEvent() {
           @Override
-          public Collection<String> getCollectionNames() {
-            return added;
+          public Iterator<String> getCollectionNames() {
+            return added.iterator();
           }
 
           @Override
@@ -177,18 +177,9 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
     });
     zkController.zkStateReader.registerCloudCollectionsListener(cloudCollectionsListener);
 
-    lastClusterProperties = new LinkedHashMap<>(zkController.zkStateReader.getClusterProperties());
     clusterPropertiesListener = (newProperties) -> {
-      if (newProperties.equals(lastClusterProperties)) {
-        return false;
-      }
       fireEvent(new ClusterPropertiesChangedEvent() {
-        final Map<String, Object> oldProps = lastClusterProperties;
-        @Override
-        public Map<String, Object> getOldClusterProperties() {
-          return oldProps;
-        }
-
+        final Instant now = Instant.now();
         @Override
         public Map<String, Object> getNewClusterProperties() {
           return newProperties;
@@ -196,10 +187,9 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
 
         @Override
         public Instant getTimestamp() {
-          return Instant.now();
+          return now;
         }
       });
-      lastClusterProperties = new LinkedHashMap<>(newProperties);
       return false;
     };
     zkController.zkStateReader.registerClusterPropertiesListener(clusterPropertiesListener);
@@ -235,17 +225,17 @@ public class ClusterEventProducerImpl implements ClusterEventProducer, ClusterSi
   }
 
   @Override
-  public void registerListener(ClusterEventListener listener) throws Exception {
+  public void registerListener(ClusterEventListener listener, ClusterEvent.EventType... eventTypes) throws Exception {
     try {
-      listener.getEventTypes().forEach(type -> {
+      for (ClusterEvent.EventType type : eventTypes) {
         if (!supportedEvents.contains(type)) {
           log.warn("event type {} not supported yet.", type);
         }
-      });
+      }
     } catch (Throwable e) {
       throw new Exception(e);
     }
-    ClusterEventProducer.super.registerListener(listener);
+    ClusterEventProducer.super.registerListener(listener, eventTypes);
   }
 
   @Override
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 0c6ce9c..6166db2 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
@@ -20,13 +20,18 @@ import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 
-import org.apache.solr.client.solrj.cloud.NodeStateProvider;
+import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.cloud.SolrCloudManager;
-import org.apache.solr.client.solrj.io.SolrClientCache;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+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.cloud.ClusterSingleton;
@@ -34,13 +39,15 @@ import org.apache.solr.cluster.events.NodesDownEvent;
 import org.apache.solr.cluster.events.ReplicasDownEvent;
 import org.apache.solr.common.cloud.ClusterState;
 import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.cloud.ReplicaPosition;
 import org.apache.solr.core.CoreContainer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * This is an (incomplete) illustration how to re-implement the combination of 8x
+ * This is an illustration how to re-implement the combination of 8x
  * NodeLostTrigger and AutoAddReplicasPlanAction to maintain the collection's replication factor.
+ * <p>NOTE: there's no support for 'waitFor' yet.</p>
  */
 public class CollectionsRepairEventListener implements ClusterSingleton, ClusterEventListener {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@@ -51,24 +58,20 @@ public class CollectionsRepairEventListener implements ClusterSingleton, Cluster
           ClusterEvent.EventType.REPLICAS_DOWN
       ));
 
-  private final CoreContainer cc;
-  private final SolrClientCache solrClientCache;
+  private static final String ASYNC_ID_PREFIX = "_col_repair_";
+  private static final AtomicInteger counter = new AtomicInteger();
+
+  private final SolrClient solrClient;
   private final SolrCloudManager solrCloudManager;
 
   private boolean running = false;
 
   public CollectionsRepairEventListener(CoreContainer cc) {
-    this.cc = cc;
-    this.solrClientCache = cc.getSolrClientCache();
+    this.solrClient = cc.getSolrClientCache().getCloudSolrClient(cc.getZkController().getZkClient().getZkServerAddress());
     this.solrCloudManager = cc.getZkController().getSolrCloudManager();
   }
 
   @Override
-  public Set<ClusterEvent.EventType> getEventTypes() {
-    return EVENT_TYPES;
-  }
-
-  @Override
   public void onEvent(ClusterEvent event) {
     if (!isRunning()) {
       // ignore the event
@@ -78,9 +81,6 @@ public class CollectionsRepairEventListener implements ClusterSingleton, Cluster
       case NODES_DOWN:
         handleNodesDown((NodesDownEvent) event);
         break;
-      case NODES_UP:
-        // ignore? rebalance replicas?
-        break;
       case REPLICAS_DOWN:
         handleReplicasDown((ReplicasDownEvent) event);
         break;
@@ -91,13 +91,47 @@ public class CollectionsRepairEventListener implements ClusterSingleton, Cluster
 
   private void handleNodesDown(NodesDownEvent event) {
     // collect all lost replicas
-    List<Replica> lostReplicas = new ArrayList<>();
+    // collection / positions
+    Map<String, List<ReplicaPosition>> newPositions = new HashMap<>();
     try {
       ClusterState clusterState = solrCloudManager.getClusterStateProvider().getClusterState();
+      Set<String> lostNodeNames = new HashSet<>();
+      event.getNodeNames().forEachRemaining(lostNodeNames::add);
       clusterState.forEachCollection(coll -> {
+        // shard / type / count
+        Map<String, Map<Replica.Type, AtomicInteger>> lostReplicas = new HashMap<>();
         coll.forEachReplica((shard, replica) -> {
-          if (event.getNodeNames().contains(replica.getNodeName())) {
-            lostReplicas.add(replica);
+          if (lostNodeNames.contains(replica.getNodeName())) {
+            lostReplicas.computeIfAbsent(shard, s -> new HashMap<>())
+                .computeIfAbsent(replica.type, t -> new AtomicInteger())
+                .incrementAndGet();
+          }
+        });
+        Assign.AssignStrategy assignStrategy = Assign.createAssignStrategy(solrCloudManager, clusterState, coll);
+        lostReplicas.forEach((shard, types) -> {
+          Assign.AssignRequestBuilder assignRequestBuilder = new Assign.AssignRequestBuilder()
+              .forCollection(coll.getName())
+              .forShard(Collections.singletonList(shard));
+          types.forEach((type, count) -> {
+            switch (type) {
+              case NRT:
+                assignRequestBuilder.assignNrtReplicas(count.get());
+                break;
+              case PULL:
+                assignRequestBuilder.assignPullReplicas(count.get());
+                break;
+              case TLOG:
+                assignRequestBuilder.assignTlogReplicas(count.get());
+                break;
+            }
+          });
+          Assign.AssignRequest assignRequest = assignRequestBuilder.build();
+          try {
+            List<ReplicaPosition> positions = assignStrategy.assign(solrCloudManager, assignRequest);
+            newPositions.put(coll.getName(), positions);
+          } catch (Exception e) {
+            log.warn("Exception computing positions for " + coll.getName() + "/" + shard, e);
+            return;
           }
         });
       });
@@ -106,8 +140,27 @@ public class CollectionsRepairEventListener implements ClusterSingleton, Cluster
       return;
     }
 
-    // compute new placements for all replicas from lost nodes
-    // send MOVEREPLICA admin requests for each lost replica
+    // 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);
+      });
+    });
+    addReplicas.forEach(addReplica -> {
+      try {
+        solrClient.request(addReplica);
+      } catch (Exception e) {
+        log.warn("Exception calling ADDREPLICA " + addReplica.getParams().toQueryString(), e);
+      }
+    });
+
+    // ... and DELETERPLICA for lost ones?
   }
 
   private void handleReplicasDown(ReplicasDownEvent event) {
diff --git a/solr/core/src/test/org/apache/solr/cluster/events/AllEventsListener.java b/solr/core/src/test/org/apache/solr/cluster/events/AllEventsListener.java
new file mode 100644
index 0000000..a8eae86
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/cluster/events/AllEventsListener.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cluster.events;
+
+import org.junit.Assert;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+/**
+ *
+ */
+public class AllEventsListener implements ClusterEventListener {
+  CountDownLatch eventLatch = new CountDownLatch(1);
+  ClusterEvent.EventType expectedType;
+  Map<ClusterEvent.EventType, List<ClusterEvent>> events = new HashMap<>();
+
+  @Override
+  public void onEvent(ClusterEvent event) {
+    events.computeIfAbsent(event.getType(), type -> new ArrayList<>()).add(event);
+    if (event.getType() == expectedType) {
+      eventLatch.countDown();
+    }
+  }
+
+  public void setExpectedType(ClusterEvent.EventType expectedType) {
+    this.expectedType = expectedType;
+    eventLatch = new CountDownLatch(1);
+  }
+
+  public void waitForExpectedEvent(int timeoutSeconds) throws InterruptedException {
+    boolean await = eventLatch.await(timeoutSeconds, TimeUnit.SECONDS);
+    if (!await) {
+      Assert.fail("Timed out waiting for expected event " + expectedType);
+    }
+  }
+}
diff --git a/solr/core/src/test/org/apache/solr/cluster/events/ClusterEventProducerTest.java b/solr/core/src/test/org/apache/solr/cluster/events/ClusterEventProducerTest.java
index 0e39e8f..12079b2 100644
--- a/solr/core/src/test/org/apache/solr/cluster/events/ClusterEventProducerTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/events/ClusterEventProducerTest.java
@@ -1,3 +1,20 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 package org.apache.solr.cluster.events;
 
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
@@ -8,52 +25,15 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
 
 /**
  *
  */
 public class ClusterEventProducerTest extends SolrCloudTestCase {
 
-  public static class AllEventsListener implements ClusterEventListener {
-    CountDownLatch eventLatch = new CountDownLatch(1);
-    ClusterEvent.EventType expectedType;
-    Map<ClusterEvent.EventType, List<ClusterEvent>> events = new HashMap<>();
-
-    @Override
-    public Set<ClusterEvent.EventType> getEventTypes() {
-      return new HashSet<>(Arrays.asList(ClusterEvent.EventType.values()));
-    }
-
-    @Override
-    public void onEvent(ClusterEvent event) {
-      events.computeIfAbsent(event.getType(), type -> new ArrayList<>()).add(event);
-      if (event.getType() == expectedType) {
-        eventLatch.countDown();
-      }
-    }
-
-    public void setExpectedType(ClusterEvent.EventType expectedType) {
-      this.expectedType = expectedType;
-      eventLatch = new CountDownLatch(1);
-    }
-
-    public void waitForExpectedEvent(int timeoutSeconds) throws InterruptedException {
-      boolean await = eventLatch.await(timeoutSeconds, TimeUnit.SECONDS);
-      if (!await) {
-        fail("Timed out waiting for expected event " + expectedType);
-      }
-    }
-  }
-
   private static AllEventsListener eventsListener = new AllEventsListener();
 
   @BeforeClass
@@ -96,7 +76,7 @@ public class ClusterEventProducerTest extends SolrCloudTestCase {
     ClusterEvent event = events.get(0);
     assertEquals("should be NODES_DOWN event type", ClusterEvent.EventType.NODES_DOWN, event.getType());
     NodesDownEvent nodesDown = (NodesDownEvent) event;
-    assertEquals("should be node " + nodeName, nodeName, nodesDown.getNodeNames().iterator().next());
+    assertEquals("should be node " + nodeName, nodeName, nodesDown.getNodeNames().next());
 
     // NODES_UP
     eventsListener.setExpectedType(ClusterEvent.EventType.NODES_UP);
@@ -109,7 +89,7 @@ public class ClusterEventProducerTest extends SolrCloudTestCase {
     event = events.get(0);
     assertEquals("should be NODES_UP event type", ClusterEvent.EventType.NODES_UP, event.getType());
     NodesUpEvent nodesUp = (NodesUpEvent) event;
-    assertEquals("should be node " + newNode.getNodeName(), newNode.getNodeName(), nodesUp.getNodeNames().iterator().next());
+    assertEquals("should be node " + newNode.getNodeName(), newNode.getNodeName(), nodesUp.getNodeNames().next());
 
     // COLLECTIONS_ADDED
     eventsListener.setExpectedType(ClusterEvent.EventType.COLLECTIONS_ADDED);
@@ -124,7 +104,7 @@ public class ClusterEventProducerTest extends SolrCloudTestCase {
     event = events.get(0);
     assertEquals("should be COLLECTIONS_ADDED event type", ClusterEvent.EventType.COLLECTIONS_ADDED, event.getType());
     CollectionsAddedEvent collectionsAdded = (CollectionsAddedEvent) event;
-    assertEquals("should be collection " + collection, collection, collectionsAdded.getCollectionNames().iterator().next());
+    assertEquals("should be collection " + collection, collection, collectionsAdded.getCollectionNames().next());
 
     // COLLECTIONS_REMOVED
     eventsListener.setExpectedType(ClusterEvent.EventType.COLLECTIONS_REMOVED);
@@ -137,7 +117,7 @@ public class ClusterEventProducerTest extends SolrCloudTestCase {
     event = events.get(0);
     assertEquals("should be COLLECTIONS_REMOVED event type", ClusterEvent.EventType.COLLECTIONS_REMOVED, event.getType());
     CollectionsRemovedEvent collectionsRemoved = (CollectionsRemovedEvent) event;
-    assertEquals("should be collection " + collection, collection, collectionsRemoved.getCollectionNames().iterator().next());
+    assertEquals("should be collection " + collection, collection, collectionsRemoved.getCollectionNames().next());
 
     // CLUSTER_CONFIG_CHANGED
     eventsListener.setExpectedType(ClusterEvent.EventType.CLUSTER_PROPERTIES_CHANGED);
@@ -152,7 +132,6 @@ public class ClusterEventProducerTest extends SolrCloudTestCase {
     assertEquals("should be CLUSTER_CONFIG_CHANGED event type", ClusterEvent.EventType.CLUSTER_PROPERTIES_CHANGED, event.getType());
     ClusterPropertiesChangedEvent propertiesChanged = (ClusterPropertiesChangedEvent) event;
     Map<String, Object> newProps = propertiesChanged.getNewClusterProperties();
-    assertEquals("old props should be the same", oldProps, propertiesChanged.getOldClusterProperties());
     assertEquals("new properties wrong value of the 'ext.foo' property: " + newProps,
         "bar", newProps.get("ext.foo"));
 
@@ -166,7 +145,6 @@ public class ClusterEventProducerTest extends SolrCloudTestCase {
     event = events.get(1);
     assertEquals("should be CLUSTER_CONFIG_CHANGED event type", ClusterEvent.EventType.CLUSTER_PROPERTIES_CHANGED, event.getType());
     propertiesChanged = (ClusterPropertiesChangedEvent) event;
-    assertEquals("old props should be the same as previous new props", newProps, propertiesChanged.getOldClusterProperties());
     assertEquals("new properties should not have 'ext.foo' property: " + propertiesChanged.getNewClusterProperties(),
         null, propertiesChanged.getNewClusterProperties().get("ext.foo"));
 
diff --git a/solr/core/src/test/org/apache/solr/cluster/events/impl/CollectionsRepairEventListenerTest.java b/solr/core/src/test/org/apache/solr/cluster/events/impl/CollectionsRepairEventListenerTest.java
new file mode 100644
index 0000000..ae68167
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/cluster/events/impl/CollectionsRepairEventListenerTest.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cluster.events.impl;
+
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.cluster.events.AllEventsListener;
+import org.apache.solr.cluster.events.ClusterEvent;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class CollectionsRepairEventListenerTest extends SolrCloudTestCase {
+
+  private static AllEventsListener eventsListener = new AllEventsListener();
+
+  private static int NUM_NODES = 3;
+
+  @BeforeClass
+  public static void setupCluster() throws Exception {
+    configureCluster(NUM_NODES)
+        .addConfig("conf", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
+        .configure();
+    cluster.getOpenOverseer().getCoreContainer().getClusterEventProducer()
+        .registerListener(eventsListener, ClusterEvent.EventType.values());
+  }
+
+  @Before
+  public void setUp() throws Exception  {
+    super.setUp();
+    cluster.deleteAllCollections();
+  }
+
+  @Test
+  public void testCollectionRepair() throws Exception {
+
+  }
+}