diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 99e235109a3..d42675dd810 100644 --- a/solr/CHANGES.txt +++ b/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 --------------------- diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java index 97bed5c0de1..d814ab5a4b0 100644 --- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java +++ b/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); - 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); + addReplicationProperties(map::putNoEx, props); } }); 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); - 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); + addReplicationProperties(follower::add, props); 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 nl, String key, Properties props, @SuppressWarnings({"rawtypes"})Class clzz) { - Object val = formatVal(key, props, clzz); - if (val != null) { - nl.add(key, val); - } + private void addReplicationProperties(BiConsumer consumer, Properties props) { + addVal(consumer, IndexFetcher.INDEX_REPLICATED_AT, props, Date.class); + addVal(consumer, IndexFetcher.INDEX_REPLICATED_AT_LIST, props, List.class); + 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 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; } diff --git a/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java b/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java index 4979dd2d8dd..163cc42cb40 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java +++ b/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 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 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) {