SOLR-14924: Some ReplicationHandler metrics are reported using incorrect types.

solr/CHANGES.txt

@@ -263,6 +263,8 @@ Bug Fixes
 * SOLR-14503: Use specified waitForZk value as connection timeout for zookeeper in SolrDispatcherFilter.
   Also, consume specified SOLR_WAIT_FOR_ZK in bin/solr.cmd (Colvin Cowie via Munendra S N)
 
+* SOLR-14924: Some ReplicationHandler metrics are reported using incorrect types. (ab)
+
 Other Changes
 ---------------------
 

solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java

@@ -48,6 +48,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.BiConsumer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.zip.Adler32;
@@ -68,7 +69,6 @@ import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.RateLimiter;
-import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.params.CommonParams;
@@ -912,15 +912,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
           map.put("downloadSpeed", val / elapsed);
         }
         Properties props = loadReplicationProperties();
-        addVal(map, IndexFetcher.PREVIOUS_CYCLE_TIME_TAKEN, props, Long.class);
+        addReplicationProperties(map::putNoEx, props);
-        addVal(map, IndexFetcher.INDEX_REPLICATED_AT, props, Date.class);
-        addVal(map, IndexFetcher.CONF_FILES_REPLICATED_AT, props, Date.class);
-        addVal(map, IndexFetcher.REPLICATION_FAILED_AT, props, Date.class);
-        addVal(map, IndexFetcher.TIMES_FAILED, props, Integer.class);
-        addVal(map, IndexFetcher.TIMES_INDEX_REPLICATED, props, Integer.class);
-        addVal(map, IndexFetcher.LAST_CYCLE_BYTES_DOWNLOADED, props, Long.class);
-        addVal(map, IndexFetcher.TIMES_CONFIG_REPLICATED, props, Integer.class);
-        addVal(map, IndexFetcher.CONF_FILES_REPLICATED, props, String.class);
       }
     });
     solrMetricsContext.gauge(fetcherMap, true, "fetcher", getCategory().toString(), scope);
@@ -989,18 +981,7 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
       } else if (isPollingDisabled()) {
         follower.add(NEXT_EXECUTION_AT, "Polling disabled");
       }
-      addVal(follower, IndexFetcher.INDEX_REPLICATED_AT, props, Date.class);
+      addReplicationProperties(follower::add, props);
-      addVal(follower, IndexFetcher.INDEX_REPLICATED_AT_LIST, props, List.class);
-      addVal(follower, IndexFetcher.REPLICATION_FAILED_AT_LIST, props, List.class);
-      addVal(follower, IndexFetcher.TIMES_INDEX_REPLICATED, props, Integer.class);
-      addVal(follower, IndexFetcher.CONF_FILES_REPLICATED, props, Integer.class);
-      addVal(follower, IndexFetcher.TIMES_CONFIG_REPLICATED, props, Integer.class);
-      addVal(follower, IndexFetcher.CONF_FILES_REPLICATED_AT, props, Integer.class);
-      addVal(follower, IndexFetcher.LAST_CYCLE_BYTES_DOWNLOADED, props, Long.class);
-      addVal(follower, IndexFetcher.TIMES_FAILED, props, Integer.class);
-      addVal(follower, IndexFetcher.REPLICATION_FAILED_AT, props, Date.class);
-      addVal(follower, IndexFetcher.PREVIOUS_CYCLE_TIME_TAKEN, props, Long.class);
-      addVal(follower, IndexFetcher.CLEARED_LOCAL_IDX, props, Long.class);
 
       follower.add("currentDate", new Date().toString());
       follower.add("isPollingDisabled", String.valueOf(isPollingDisabled()));
@@ -1104,17 +1085,25 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
     return details;
   }
 
-  private void addVal(NamedList<Object> nl, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) {
+  private void addReplicationProperties(BiConsumer<String, Object> consumer, Properties props) {
-    Object val = formatVal(key, props, clzz);
+    addVal(consumer, IndexFetcher.INDEX_REPLICATED_AT, props, Date.class);
-    if (val != null) {
+    addVal(consumer, IndexFetcher.INDEX_REPLICATED_AT_LIST, props, List.class);
-      nl.add(key, val);
+    addVal(consumer, IndexFetcher.REPLICATION_FAILED_AT_LIST, props, List.class);
-    }
+    addVal(consumer, IndexFetcher.TIMES_INDEX_REPLICATED, props, Integer.class);
+    addVal(consumer, IndexFetcher.CONF_FILES_REPLICATED, props, String.class);
+    addVal(consumer, IndexFetcher.TIMES_CONFIG_REPLICATED, props, Integer.class);
+    addVal(consumer, IndexFetcher.CONF_FILES_REPLICATED_AT, props, Date.class);
+    addVal(consumer, IndexFetcher.LAST_CYCLE_BYTES_DOWNLOADED, props, Long.class);
+    addVal(consumer, IndexFetcher.TIMES_FAILED, props, Integer.class);
+    addVal(consumer, IndexFetcher.REPLICATION_FAILED_AT, props, Date.class);
+    addVal(consumer, IndexFetcher.PREVIOUS_CYCLE_TIME_TAKEN, props, Long.class);
+    addVal(consumer, IndexFetcher.CLEARED_LOCAL_IDX, props, Boolean.class);
   }
 
-  private void addVal(MapWriter.EntryWriter ew, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) {
+  private void addVal(BiConsumer<String, Object> consumer, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) {
     Object val = formatVal(key, props, clzz);
     if (val != null) {
-      ew.putNoEx(key, val);
+      consumer.accept(key, val);
     }
   }
 
@@ -1135,6 +1124,22 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
         l.add(new Date(Long.parseLong(s1)).toString());
       }
       return l;
+    } else if (clzz == Long.class) {
+      try {
+        Long l = Long.parseLong(s);
+        return l;
+      } catch (NumberFormatException e) {
+        return null;
+      }
+    } else if (clzz == Integer.class) {
+      try {
+        Integer i = Integer.parseInt(s);
+        return i;
+      } catch (NumberFormatException e) {
+        return null;
+      }
+    } else if (clzz == Boolean.class) {
+      return Boolean.parseBoolean(s);
     } else {
       return s;
     }

solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java

@@ -24,8 +24,11 @@ import java.util.Collections;
 import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
+import com.codahale.metrics.Metric;
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
@@ -39,6 +42,8 @@ import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricManager;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -48,6 +53,7 @@ public class ReplaceNodeTest extends SolrCloudTestCase {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   @BeforeClass
   public static void setupCluster() throws Exception {
+    System.setProperty("metricsEnabled", "true");
     configureCluster(6)
         .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-dynamic").resolve("conf"))
         .configure();
@@ -170,6 +176,39 @@ public class ReplaceNodeTest extends SolrCloudTestCase {
         assertFalse(r.toString(), Replica.State.ACTIVE.equals(r.getState()));
       }
     }
+
+    // check replication metrics on this jetty - see SOLR-14924
+    for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
+      if (jetty.getCoreContainer() == null) {
+        continue;
+      }
+      SolrMetricManager metricManager = jetty.getCoreContainer().getMetricManager();
+      String registryName = null;
+      for (String name : metricManager.registryNames()) {
+        if (name.startsWith("solr.core.")) {
+          registryName = name;
+        }
+      }
+      Map<String, Metric> metrics = metricManager.registry(registryName).getMetrics();
+      if (!metrics.containsKey("REPLICATION./replication.fetcher")) {
+        continue;
+      }
+      @SuppressWarnings("unchecked")
+      MetricsMap fetcherGauge = (MetricsMap) ((SolrMetricManager.GaugeWrapper<?>) metrics.get("REPLICATION./replication.fetcher")).getGauge();
+      assertNotNull("no IndexFetcher gauge in metrics", fetcherGauge);
+      Map<String, Object> value = fetcherGauge.getValue();
+      if (value.isEmpty()) {
+        continue;
+      }
+      assertNotNull("isReplicating missing: " + value, value.get("isReplicating"));
+      assertTrue("isReplicating should be a boolean: " + value, value.get("isReplicating") instanceof Boolean);
+      if (value.get("indexReplicatedAt") == null) {
+        continue;
+      }
+      assertNotNull("timesIndexReplicated missing: " + value, value.get("timesIndexReplicated"));
+      assertTrue("timesIndexReplicated should be a number: " + value, value.get("timesIndexReplicated") instanceof Number);
+    }
+
   }
 
   public static  CollectionAdminRequest.AsyncCollectionAdminRequest createReplaceNodeRequest(String sourceNode, String targetNode, Boolean parallel) {