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/02/27 12:39:23 UTC
[lucene-solr] branch branch_8x updated: SOLR-14252:
NullPointerException in AggregateMetric.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new ad9b2d2 SOLR-14252: NullPointerException in AggregateMetric.
ad9b2d2 is described below
commit ad9b2d2e1994f6cc868444eed9c3438408ef6078
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Thu Feb 27 12:58:57 2020 +0100
SOLR-14252: NullPointerException in AggregateMetric.
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/metrics/AggregateMetric.java | 6 ++
.../apache/solr/util/stats/MetricUtilsTest.java | 70 ++++++++++++++++------
solr/solr-ref-guide/src/metrics-reporting.adoc | 2 +-
4 files changed, 61 insertions(+), 19 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 2b9b159..f149002 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -152,6 +152,8 @@ Bug Fixes
* SOLR-14250: Do not log error when trying to consume non-existing input stream due to Expect: 100-continue (janhoy)
+* SOLR-14252: Avoid NullPointerException in AggregateMetric. (Andy Webb via ab)
+
Other Changes
---------------------
diff --git a/solr/core/src/java/org/apache/solr/metrics/AggregateMetric.java b/solr/core/src/java/org/apache/solr/metrics/AggregateMetric.java
index babc99d..3c7df0c 100644
--- a/solr/core/src/java/org/apache/solr/metrics/AggregateMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/AggregateMetric.java
@@ -107,6 +107,9 @@ public class AggregateMetric implements Metric {
res = n.doubleValue();
}
}
+ if (res == null) {
+ return 0;
+ }
return res;
}
@@ -128,6 +131,9 @@ public class AggregateMetric implements Metric {
res = n.doubleValue();
}
}
+ if (res == null) {
+ return 0;
+ }
return res;
}
diff --git a/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java b/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java
index fa9ab51..4c2f966 100644
--- a/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java
+++ b/solr/core/src/test/org/apache/solr/util/stats/MetricUtilsTest.java
@@ -79,11 +79,23 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
meter.mark();
Histogram histogram = registry.histogram("histogram");
histogram.update(10);
- AggregateMetric am = new AggregateMetric();
- registry.register("aggregate", am);
- am.set("foo", 10);
- am.set("bar", 1);
- am.set("bar", 2);
+
+ // SOLR-14252: check that negative values are supported correctly
+ // NB a-d represent the same metric from multiple nodes
+ AggregateMetric am1 = new AggregateMetric();
+ registry.register("aggregate1", am1);
+ am1.set("a", -10);
+ am1.set("b", 1);
+ am1.set("b", -2);
+ am1.set("c", -3);
+ am1.set("d", -5);
+
+ // SOLR-14252: check that aggregation of non-Number metrics don't trigger NullPointerException
+ AggregateMetric am2 = new AggregateMetric();
+ registry.register("aggregate2", am2);
+ am2.set("a", false);
+ am2.set("b", true);
+
Gauge<String> gauge = () -> "foobar";
registry.register("gauge", gauge);
Gauge<Long> error = () -> {throw new InternalError("Memory Pool not found error");};
@@ -102,17 +114,26 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
assertEquals(1L, v.get("count"));
} else if (k.startsWith("histogram")) {
assertEquals(1L, v.get("count"));
- } else if (k.startsWith("aggregate")) {
- assertEquals(2, v.get("count"));
+ } else if (k.startsWith("aggregate1")) {
+ assertEquals(4, v.get("count"));
Map<String, Object> values = (Map<String, Object>)v.get("values");
assertNotNull(values);
- assertEquals(2, values.size());
- Map<String, Object> update = (Map<String, Object>)values.get("foo");
- assertEquals(10, update.get("value"));
+ assertEquals(4, values.size());
+ Map<String, Object> update = (Map<String, Object>)values.get("a");
+ assertEquals(-10, update.get("value"));
assertEquals(1, update.get("updateCount"));
- update = (Map<String, Object>)values.get("bar");
- assertEquals(2, update.get("value"));
+ update = (Map<String, Object>)values.get("b");
+ assertEquals(-2, update.get("value"));
assertEquals(2, update.get("updateCount"));
+ assertEquals(-10D, v.get("min"));
+ assertEquals(-2D, v.get("max"));
+ assertEquals(-5D, v.get("mean"));
+ } else if (k.startsWith("aggregate2")) {
+ // SOLR-14252: non-Number metric aggregations should return 0 rather than throwing NPE
+ assertEquals(2, v.get("count"));
+ assertEquals(0D, v.get("min"));
+ assertEquals(0D, v.get("max"));
+ assertEquals(0D, v.get("mean"));
} else if (k.startsWith("memory.expected.error")) {
assertNull(v);
}
@@ -139,19 +160,32 @@ public class MetricUtilsTest extends SolrTestCaseJ4 {
assertTrue(o instanceof Map);
Map v = (Map)o;
assertEquals(1L, v.get("count"));
- } else if (k.startsWith("aggregate")) {
+ } else if (k.startsWith("aggregate1")) {
+ assertTrue(o instanceof Map);
+ Map v = (Map)o;
+ assertEquals(4, v.get("count"));
+ Map<String, Object> values = (Map<String, Object>)v.get("values");
+ assertNotNull(values);
+ assertEquals(4, values.size());
+ Map<String, Object> update = (Map<String, Object>)values.get("a");
+ assertEquals(-10, update.get("value"));
+ assertEquals(1, update.get("updateCount"));
+ update = (Map<String, Object>)values.get("b");
+ assertEquals(-2, update.get("value"));
+ assertEquals(2, update.get("updateCount"));
+ } else if (k.startsWith("aggregate2")) {
assertTrue(o instanceof Map);
Map v = (Map)o;
assertEquals(2, v.get("count"));
Map<String, Object> values = (Map<String, Object>)v.get("values");
assertNotNull(values);
assertEquals(2, values.size());
- Map<String, Object> update = (Map<String, Object>)values.get("foo");
- assertEquals(10, update.get("value"));
+ Map<String, Object> update = (Map<String, Object>)values.get("a");
+ assertEquals(false, update.get("value"));
+ assertEquals(1, update.get("updateCount"));
+ update = (Map<String, Object>)values.get("b");
+ assertEquals(true, update.get("value"));
assertEquals(1, update.get("updateCount"));
- update = (Map<String, Object>)values.get("bar");
- assertEquals(2, update.get("value"));
- assertEquals(2, update.get("updateCount"));
} else if (k.startsWith("memory.expected.error")) {
assertNull(o);
} else {
diff --git a/solr/solr-ref-guide/src/metrics-reporting.adoc b/solr/solr-ref-guide/src/metrics-reporting.adoc
index ecefda7..c66017c 100644
--- a/solr/solr-ref-guide/src/metrics-reporting.adoc
+++ b/solr/solr-ref-guide/src/metrics-reporting.adoc
@@ -519,7 +519,7 @@ Example configuration:
</lst>
<lst name="report">
<str name="group">aggregated_shard_leaders</str>
- <str name="registry">solr\.core\.(.*)\.leader</str>
+ <str name="registry">solr\.collection\.(.*)\.leader</str>
<str name="label">leader.$1</str>
<str name="filter">UPDATE\./update/.*</str>
</lst>