You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ds...@apache.org on 2025/02/19 03:58:39 UTC
(solr) branch branch_9_8 updated (4098e86795b -> cc2112f93d6)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a change to branch branch_9_8
in repository https://gitbox.apache.org/repos/asf/solr.git
from 4098e86795b SOLR-17667: In TestLBHttp2SolrClient only listen on the client DEBUG logging
new 6382b70c52a SOLR-17649: Fix JSON faceting on multiValued number types (#3158)
new 11075f7259b SOLR-17221: Fix Http2SolrClient merging case sensitive solr params (#3028)
new cc2112f93d6 SOLR-17587: wt=prometheus fix duplicate TYPE information (#3006)
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
solr/CHANGES.txt | 9 ++++
.../apache/solr/handler/admin/MetricsHandler.java | 31 +++++++++++++
.../core/PrometheusCoreFormatterInfo.java | 4 --
.../prometheus/core/SolrCoreCacheMetric.java | 5 +--
.../prometheus/core/SolrCoreHandlerMetric.java | 5 +--
.../prometheus/core/SolrCoreHighlighterMetric.java | 5 +--
.../prometheus/core/SolrCoreIndexMetric.java | 5 +--
.../metrics/prometheus/core/SolrCoreMetric.java | 46 +++++++++++--------
.../prometheus/core/SolrCoreSearcherMetric.java | 5 +--
.../prometheus/core/SolrCoreTlogMetric.java | 5 +--
.../core/SolrPrometheusCoreFormatter.java | 20 ++++-----
.../solr/response/PrometheusResponseWriter.java | 21 +++------
.../org/apache/solr/search/facet/FacetField.java | 22 +++++----
.../search/facet/FacetFieldProcessorByArrayDV.java | 2 +-
.../org/apache/solr/search/facet/FieldUtil.java | 52 ++++++++++++++++------
.../solr/collection1/conf/schema_latest.xml | 2 +
.../solr/handler/admin/MetricsHandlerTest.java | 6 +--
.../apache/solr/metrics/SolrCoreMetricTest.java | 25 ++++++-----
.../solr/metrics/SolrPrometheusFormatterTest.java | 41 ++++++++++-------
.../response/TestPrometheusResponseWriter.java | 19 +++++++-
.../apache/solr/response/TestRawTransformer.java | 3 +-
.../apache/solr/search/facet/TestJsonFacets.java | 29 ++++++++++++
.../solr/client/solrj/impl/Http2SolrClient.java | 20 +++------
.../client/solrj/impl/Http2SolrClientTest.java | 2 +
.../client/solrj/impl/HttpJdkSolrClientTest.java | 2 +
.../client/solrj/impl/HttpSolrClientTestBase.java | 36 ++++++++++++++-
26 files changed, 283 insertions(+), 139 deletions(-)
(solr) 03/03: SOLR-17587: wt=prometheus fix duplicate TYPE information (#3006)
Posted by ds...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9_8
in repository https://gitbox.apache.org/repos/asf/solr.git
commit cc2112f93d65eef29881c1efae76db3cbef85cbf
Author: Matthew Biscocho <54...@users.noreply.github.com>
AuthorDate: Thu Jan 9 15:03:05 2025 -0500
SOLR-17587: wt=prometheus fix duplicate TYPE information (#3006)
Metrics: Prometheus response writer fix for non-compliant exposition format containing duplicate TYPE lines
9x backport of main:
* SOLR-17587: wt=prometheus fix duplicate TYPE information (#2902)
---
solr/CHANGES.txt | 3 ++
.../apache/solr/handler/admin/MetricsHandler.java | 31 +++++++++++++++
.../core/PrometheusCoreFormatterInfo.java | 4 --
.../prometheus/core/SolrCoreCacheMetric.java | 5 +--
.../prometheus/core/SolrCoreHandlerMetric.java | 5 +--
.../prometheus/core/SolrCoreHighlighterMetric.java | 5 +--
.../prometheus/core/SolrCoreIndexMetric.java | 5 +--
.../metrics/prometheus/core/SolrCoreMetric.java | 46 +++++++++++++---------
.../prometheus/core/SolrCoreSearcherMetric.java | 5 +--
.../prometheus/core/SolrCoreTlogMetric.java | 5 +--
.../core/SolrPrometheusCoreFormatter.java | 20 ++++------
.../solr/response/PrometheusResponseWriter.java | 21 +++-------
.../solr/handler/admin/MetricsHandlerTest.java | 6 +--
.../apache/solr/metrics/SolrCoreMetricTest.java | 25 ++++++------
.../solr/metrics/SolrPrometheusFormatterTest.java | 41 ++++++++++++-------
.../response/TestPrometheusResponseWriter.java | 19 ++++++++-
16 files changed, 146 insertions(+), 100 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index de389c38463..8f65e55bd3d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -7,6 +7,9 @@ https://github.com/apache/solr/blob/main/solr/solr-ref-guide/modules/upgrade-not
================== 9.8.1 ==================
Bug Fixes
---------------------
+* SOLR-17587: Metrics: wt=prometheus fix for non-compliant exposition format containing duplicate TYPE lines.
+ (Matthew Biscocho via David Smiley)
+
* SOLR-17637: LBHttp2SolrClient can fail to complete async requests in certain error scenarios.
This can cause the HttpShardHandler to indefinitely wait on a completed response that will never come. (Houston Putman)
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
index 73da8a818e4..4bba3414be9 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHandler.java
@@ -186,10 +186,20 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
List<MetricType> metricTypes = parseMetricTypes(params);
List<MetricFilter> metricFilters =
metricTypes.stream().map(MetricType::asMetricFilter).collect(Collectors.toList());
+
Set<String> requestedRegistries = parseRegistries(params);
+ MetricRegistry mergedCoreRegistries = new MetricRegistry();
for (String registryName : requestedRegistries) {
MetricRegistry dropwizardRegistry = metricManager.registry(registryName);
+
+ // Merge all core registries into a single registry and
+ // append the core name to the metric to avoid duplicate metrics name
+ if (registryName.startsWith("solr.core")) {
+ mergedCoreRegistries.registerAll(getCoreNameFromRegistry(registryName), dropwizardRegistry);
+ continue;
+ }
+
PrometheusResponseWriter.toPrometheus(
dropwizardRegistry,
registryName,
@@ -203,6 +213,22 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
response.add(registryName, registry);
});
}
+
+ if (!mergedCoreRegistries.getMetrics().isEmpty()) {
+ PrometheusResponseWriter.toPrometheus(
+ mergedCoreRegistries,
+ "solr.core",
+ metricFilters,
+ mustMatchFilter,
+ propertyFilter,
+ false,
+ false,
+ true,
+ (registry) -> {
+ response.add("solr.core", registry);
+ });
+ }
+
return response;
}
@@ -523,6 +549,11 @@ public class MetricsHandler extends RequestHandlerBase implements PermissionName
return metricTypes;
}
+ private String getCoreNameFromRegistry(String registryName) {
+ String coreName = registryName.substring(registryName.indexOf('.') + 1);
+ return coreName.replace(".", "_");
+ }
+
@Override
public String getDescription() {
return "A handler to return all the metrics gathered by Solr";
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/PrometheusCoreFormatterInfo.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/PrometheusCoreFormatterInfo.java
index baa7be5898b..b80b406d356 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/PrometheusCoreFormatterInfo.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/PrometheusCoreFormatterInfo.java
@@ -16,8 +16,6 @@
*/
package org.apache.solr.metrics.prometheus.core;
-import java.util.regex.Pattern;
-
public interface PrometheusCoreFormatterInfo {
/** Category of prefix Solr Core dropwizard handler metric names */
enum CoreCategory {
@@ -32,6 +30,4 @@ public interface PrometheusCoreFormatterInfo {
INDEX,
CORE
}
-
- Pattern CLOUD_CORE_PATTERN = Pattern.compile("^core_(.*)_(shard[0-9]+)_(replica_.[0-9]+)$");
}
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreCacheMetric.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreCacheMetric.java
index e8352d3575c..d4d9627c222 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreCacheMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreCacheMetric.java
@@ -24,9 +24,8 @@ import org.apache.solr.metrics.prometheus.SolrPrometheusFormatter;
public class SolrCoreCacheMetric extends SolrCoreMetric {
public static final String CORE_CACHE_SEARCHER_METRICS = "solr_metrics_core_cache";
- public SolrCoreCacheMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, coreName, metricName, cloudMode);
+ public SolrCoreCacheMetric(Metric dropwizardMetric, String metricName) {
+ super(dropwizardMetric, metricName);
}
/*
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHandlerMetric.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHandlerMetric.java
index af9641729a9..9273e825759 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHandlerMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHandlerMetric.java
@@ -30,9 +30,8 @@ public class SolrCoreHandlerMetric extends SolrCoreMetric {
public static final String CORE_REQUESTS_TOTAL_TIME = "solr_metrics_core_requests_time";
public static final String CORE_REQUEST_TIMES = "solr_metrics_core_average_request_time";
- public SolrCoreHandlerMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, coreName, metricName, cloudMode);
+ public SolrCoreHandlerMetric(Metric dropwizardMetric, String metricName) {
+ super(dropwizardMetric, metricName);
}
/*
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHighlighterMetric.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHighlighterMetric.java
index 52bc1292c25..d3da3d3426f 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHighlighterMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreHighlighterMetric.java
@@ -24,9 +24,8 @@ import org.apache.solr.metrics.prometheus.SolrPrometheusFormatter;
public class SolrCoreHighlighterMetric extends SolrCoreMetric {
public static final String CORE_HIGHLIGHER_METRICS = "solr_metrics_core_highlighter_requests";
- public SolrCoreHighlighterMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, coreName, metricName, cloudMode);
+ public SolrCoreHighlighterMetric(Metric dropwizardMetric, String metricName) {
+ super(dropwizardMetric, metricName);
}
/*
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreIndexMetric.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreIndexMetric.java
index 048fc8d2ca2..e45e1e955bd 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreIndexMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreIndexMetric.java
@@ -24,9 +24,8 @@ import org.apache.solr.metrics.prometheus.SolrPrometheusFormatter;
public class SolrCoreIndexMetric extends SolrCoreMetric {
public static final String CORE_INDEX_METRICS = "solr_metrics_core_index_size_bytes";
- public SolrCoreIndexMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, coreName, metricName, cloudMode);
+ public SolrCoreIndexMetric(Metric dropwizardMetric, String metricName) {
+ super(dropwizardMetric, metricName);
}
/*
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java
index 5010425a023..cb283ea7563 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreMetric.java
@@ -16,37 +16,45 @@
*/
package org.apache.solr.metrics.prometheus.core;
-import static org.apache.solr.metrics.prometheus.core.PrometheusCoreFormatterInfo.CLOUD_CORE_PATTERN;
-
import com.codahale.metrics.Metric;
+import java.util.List;
import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.apache.solr.common.SolrException;
import org.apache.solr.metrics.prometheus.SolrMetric;
/** Base class is a wrapper to export a solr.core {@link com.codahale.metrics.Metric} */
public abstract class SolrCoreMetric extends SolrMetric {
- public String coreName;
- public SolrCoreMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, metricName);
- this.coreName = coreName;
- labels.put("core", coreName);
- if (cloudMode) {
- appendCloudModeLabels();
- }
- }
+ public static Pattern CLOUD_CORE_PATTERN =
+ Pattern.compile(
+ "(?<core>^core_(?<collection>.*)_(?<shard>shard[0-9]+)_(?<replica>replica_.[0-9]+)).(.*)$");
+ public static Pattern STANDALONE_CORE_PATTERN = Pattern.compile("^core_(?<core>.*?)\\.(.*)$");
+ public static List<String> CLOUD_LABEL_KEYS = List.of("core", "collection", "shard", "replica");
+ public static List<String> STANDALONE_LABEL_KEYS = List.of("core");
+ public String coreName;
- private void appendCloudModeLabels() {
- Matcher m = CLOUD_CORE_PATTERN.matcher(coreName);
- if (m.find()) {
- labels.put("collection", m.group(1));
- labels.put("shard", m.group(2));
- labels.put("replica", m.group(3));
+ public SolrCoreMetric(Metric dropwizardMetric, String metricName) {
+ // Remove Core Name prefix from metric as it is no longer needed for tag parsing from this point
+ super(dropwizardMetric, metricName.substring(metricName.indexOf(".") + 1));
+ Matcher cloudPattern = CLOUD_CORE_PATTERN.matcher(metricName);
+ Matcher standalonePattern = STANDALONE_CORE_PATTERN.matcher(metricName);
+ if (cloudPattern.find()) {
+ coreName = cloudPattern.group("core");
+ CLOUD_LABEL_KEYS.forEach(
+ (key) -> {
+ labels.put(key, cloudPattern.group(key));
+ });
+ } else if (standalonePattern.find()) {
+ coreName = standalonePattern.group("core");
+ STANDALONE_LABEL_KEYS.forEach(
+ (key) -> {
+ labels.put(key, standalonePattern.group(key));
+ });
} else {
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR,
- "Core name does not match pattern for parsing " + coreName);
+ "Core name does not match pattern for parsing in metric " + metricName);
}
}
}
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreSearcherMetric.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreSearcherMetric.java
index 93071f9b8ad..7ca694d311d 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreSearcherMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreSearcherMetric.java
@@ -29,9 +29,8 @@ public class SolrCoreSearcherMetric extends SolrCoreMetric {
public static final String CORE_SEARCHER_METRICS = "solr_metrics_core_searcher_documents";
public static final String CORE_SEARCHER_TIMES = "solr_metrics_core_average_searcher_warmup_time";
- public SolrCoreSearcherMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, coreName, metricName, cloudMode);
+ public SolrCoreSearcherMetric(Metric dropwizardMetric, String metricName) {
+ super(dropwizardMetric, metricName);
}
/*
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreTlogMetric.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreTlogMetric.java
index 29b13e1caee..7286c99d986 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreTlogMetric.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrCoreTlogMetric.java
@@ -24,9 +24,8 @@ import org.apache.solr.metrics.prometheus.SolrPrometheusFormatter;
public class SolrCoreTlogMetric extends SolrCoreMetric {
public static final String CORE_TLOG_METRICS = "solr_metrics_core_tlog";
- public SolrCoreTlogMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, coreName, metricName, cloudMode);
+ public SolrCoreTlogMetric(Metric dropwizardMetric, String metricName) {
+ super(dropwizardMetric, metricName);
}
/*
diff --git a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrPrometheusCoreFormatter.java b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrPrometheusCoreFormatter.java
index 6b2a49154f3..dd3ed025ddf 100644
--- a/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrPrometheusCoreFormatter.java
+++ b/solr/core/src/java/org/apache/solr/metrics/prometheus/core/SolrPrometheusCoreFormatter.java
@@ -28,13 +28,9 @@ import org.apache.solr.metrics.prometheus.SolrPrometheusFormatter;
*/
public class SolrPrometheusCoreFormatter extends SolrPrometheusFormatter
implements PrometheusCoreFormatterInfo {
- public final String coreName;
- public final boolean cloudMode;
- public SolrPrometheusCoreFormatter(String coreName, boolean cloudMode) {
+ public SolrPrometheusCoreFormatter() {
super();
- this.coreName = coreName;
- this.cloudMode = cloudMode;
}
@Override
@@ -45,7 +41,7 @@ public class SolrPrometheusCoreFormatter extends SolrPrometheusFormatter
@Override
public SolrMetric categorizeMetric(Metric dropwizardMetric, String metricName) {
- String metricCategory = metricName.split("\\.", 2)[0];
+ String metricCategory = metricName.split("\\.", 3)[1];
if (!Enums.getIfPresent(PrometheusCoreFormatterInfo.CoreCategory.class, metricCategory)
.isPresent()) {
return new SolrNoOpMetric();
@@ -55,17 +51,17 @@ public class SolrPrometheusCoreFormatter extends SolrPrometheusFormatter
case QUERY:
case UPDATE:
case REPLICATION:
- return new SolrCoreHandlerMetric(dropwizardMetric, coreName, metricName, cloudMode);
+ return new SolrCoreHandlerMetric(dropwizardMetric, metricName);
case TLOG:
- return new SolrCoreTlogMetric(dropwizardMetric, coreName, metricName, cloudMode);
+ return new SolrCoreTlogMetric(dropwizardMetric, metricName);
case CACHE:
- return new SolrCoreCacheMetric(dropwizardMetric, coreName, metricName, cloudMode);
+ return new SolrCoreCacheMetric(dropwizardMetric, metricName);
case SEARCHER:
- return new SolrCoreSearcherMetric(dropwizardMetric, coreName, metricName, cloudMode);
+ return new SolrCoreSearcherMetric(dropwizardMetric, metricName);
case HIGHLIGHTER:
- return new SolrCoreHighlighterMetric(dropwizardMetric, coreName, metricName, cloudMode);
+ return new SolrCoreHighlighterMetric(dropwizardMetric, metricName);
case INDEX:
- return new SolrCoreIndexMetric(dropwizardMetric, coreName, metricName, cloudMode);
+ return new SolrCoreIndexMetric(dropwizardMetric, metricName);
case CORE:
default:
return new SolrNoOpMetric();
diff --git a/solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java b/solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java
index 5c6210603da..8689751a514 100644
--- a/solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java
+++ b/solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java
@@ -25,12 +25,11 @@ import io.prometheus.metrics.expositionformats.PrometheusTextFormatWriter;
import java.io.IOException;
import java.io.OutputStream;
import java.lang.invoke.MethodHandles;
-import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Predicate;
-import java.util.stream.Collectors;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.metrics.AggregateMetric;
import org.apache.solr.metrics.prometheus.SolrPrometheusFormatter;
@@ -115,8 +114,10 @@ public class PrometheusResponseWriter extends RawResponseWriter {
Metric dropwizardMetric = dropwizardMetrics.get(metricName);
formatter.exportDropwizardMetric(dropwizardMetric, metricName);
} catch (Exception e) {
- // Do not fail entirely for metrics exporting. Log and try to export next metric
- log.warn("Error occurred exporting Dropwizard Metric to Prometheus", e);
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Error occurred exporting Dropwizard Metric to Prometheus",
+ e);
}
});
@@ -124,21 +125,11 @@ public class PrometheusResponseWriter extends RawResponseWriter {
}
public static SolrPrometheusFormatter getFormatterType(String registryName) {
- String coreName;
- boolean cloudMode = false;
String[] parsedRegistry = registryName.split("\\.");
switch (parsedRegistry[1]) {
case "core":
- if (parsedRegistry.length == 3) {
- coreName = parsedRegistry[2];
- } else if (parsedRegistry.length == 5) {
- coreName = Arrays.stream(parsedRegistry).skip(1).collect(Collectors.joining("_"));
- cloudMode = true;
- } else {
- coreName = registryName;
- }
- return new SolrPrometheusCoreFormatter(coreName, cloudMode);
+ return new SolrPrometheusCoreFormatter();
case "jvm":
return new SolrPrometheusJvmFormatter();
case "jetty":
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java
index a90e059e03f..4c3709f5ae5 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java
@@ -725,8 +725,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
NamedList<?> values = resp.getValues();
assertNotNull(values.get("metrics"));
values = (NamedList<?>) values.get("metrics");
- SolrPrometheusFormatter formatter =
- (SolrPrometheusFormatter) values.get("solr.core.collection1");
+ SolrPrometheusFormatter formatter = (SolrPrometheusFormatter) values.get("solr.core");
assertNotNull(formatter);
MetricSnapshots actualSnapshots = formatter.collect();
assertNotNull(actualSnapshots);
@@ -1009,8 +1008,7 @@ public class MetricsHandlerTest extends SolrTestCaseJ4 {
NamedList<?> values = resp.getValues();
assertNotNull(values.get("metrics"));
values = (NamedList<?>) values.get("metrics");
- SolrPrometheusFormatter formatter =
- (SolrPrometheusFormatter) values.get("solr.core.collection1");
+ SolrPrometheusFormatter formatter = (SolrPrometheusFormatter) values.get("solr.core");
assertNotNull(formatter);
MetricSnapshots actualSnapshots = formatter.collect();
assertNotNull(actualSnapshots);
diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricTest.java
index 19a9fce2cbe..9ee34056e5e 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricTest.java
@@ -26,12 +26,13 @@ import org.junit.Test;
public class SolrCoreMetricTest extends SolrTestCaseJ4 {
@Test
- public void testStandaloneDefaultLabels() throws InterruptedException {
- String expectedCoreName = "test_core";
+ public void testStandaloneDefaultLabels() {
+ String dropwizardMetricName = "core_Core_Name.test_core_metric";
+
+ String expectedCoreName = "Core_Name";
String expectedMetricName = "test_core_metric";
Labels expectedLabels = Labels.of("core", expectedCoreName);
- TestSolrCoreMetric testSolrCoreMetric =
- new TestSolrCoreMetric(null, expectedCoreName, expectedMetricName, false);
+ TestSolrCoreMetric testSolrCoreMetric = new TestSolrCoreMetric(null, dropwizardMetricName);
assertEquals(expectedCoreName, testSolrCoreMetric.coreName);
assertEquals(expectedMetricName, testSolrCoreMetric.metricName);
@@ -39,10 +40,12 @@ public class SolrCoreMetricTest extends SolrTestCaseJ4 {
}
@Test
- public void testCloudDefaultLabels() throws InterruptedException {
- String expectedCoreName = "core_test_core_shard1_replica_n1";
+ public void testCloudDefaultLabels() {
+ String dropwizardMetricName = "core_Collection_Name_shard1_replica_n1.test_core_metric";
+
+ String expectedCoreName = "core_Collection_Name_shard1_replica_n1";
String expectedMetricName = "test_core_metric";
- String expectedCollectionName = "test_core";
+ String expectedCollectionName = "Collection_Name";
String expectedShardName = "shard1";
String expectedReplicaName = "replica_n1";
@@ -56,8 +59,7 @@ public class SolrCoreMetricTest extends SolrTestCaseJ4 {
expectedShardName,
"replica",
expectedReplicaName);
- TestSolrCoreMetric testSolrCoreMetric =
- new TestSolrCoreMetric(null, expectedCoreName, expectedMetricName, true);
+ TestSolrCoreMetric testSolrCoreMetric = new TestSolrCoreMetric(null, dropwizardMetricName);
assertEquals(expectedCoreName, testSolrCoreMetric.coreName);
assertEquals(expectedMetricName, testSolrCoreMetric.metricName);
@@ -65,9 +67,8 @@ public class SolrCoreMetricTest extends SolrTestCaseJ4 {
}
static class TestSolrCoreMetric extends SolrCoreMetric {
- public TestSolrCoreMetric(
- Metric dropwizardMetric, String coreName, String metricName, boolean cloudMode) {
- super(dropwizardMetric, coreName, metricName, cloudMode);
+ public TestSolrCoreMetric(Metric dropwizardMetric, String metricName) {
+ super(dropwizardMetric, metricName);
}
@Override
diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrPrometheusFormatterTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrPrometheusFormatterTest.java
index 5c62910b43f..76077385800 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrPrometheusFormatterTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrPrometheusFormatterTest.java
@@ -16,7 +16,8 @@
*/
package org.apache.solr.metrics;
-import static org.apache.solr.metrics.prometheus.core.PrometheusCoreFormatterInfo.CLOUD_CORE_PATTERN;
+import static org.apache.solr.metrics.prometheus.core.SolrCoreMetric.CLOUD_CORE_PATTERN;
+import static org.apache.solr.metrics.prometheus.core.SolrCoreMetric.STANDALONE_CORE_PATTERN;
import com.codahale.metrics.Counter;
import com.codahale.metrics.Gauge;
@@ -146,32 +147,42 @@ public class SolrPrometheusFormatterTest extends SolrTestCaseJ4 {
@Test
public void testCloudCorePattern() {
- String coreName = "core_test-core_shard2_replica_t123";
- Matcher m = CLOUD_CORE_PATTERN.matcher(coreName);
+ String metricName = "core_test-core_shard2_replica_t123.TEST./foobar/endpoint";
+ Matcher m = CLOUD_CORE_PATTERN.matcher(metricName);
assertTrue(m.find());
- assertEquals("test-core", m.group(1));
- assertEquals("shard2", m.group(2));
- assertEquals("replica_t123", m.group(3));
+ assertEquals("core_test-core_shard2_replica_t123", m.group("core"));
+ assertEquals("test-core", m.group("collection"));
+ assertEquals("shard2", m.group("shard"));
+ assertEquals("replica_t123", m.group("replica"));
- coreName = "core_foo_bar_shard24_replica_p8";
- m = CLOUD_CORE_PATTERN.matcher(coreName);
+ metricName = "core_foo_bar_shard24_replica_p8.QUERY.random.metric-name";
+ m = CLOUD_CORE_PATTERN.matcher(metricName);
assertTrue(m.matches());
- assertEquals("foo_bar", m.group(1));
- assertEquals("shard24", m.group(2));
- assertEquals("replica_p8", m.group(3));
+ assertEquals("core_foo_bar_shard24_replica_p8", m.group("core"));
+ assertEquals("foo_bar", m.group("collection"));
+ assertEquals("shard24", m.group("shard"));
+ assertEquals("replica_p8", m.group("replica"));
}
@Test
public void testBadCloudCorePattern() {
- String badCoreName = "core_solrtest_shard100_replica_xyz23";
- Matcher m = CLOUD_CORE_PATTERN.matcher(badCoreName);
+ String badMetricName = "core_solrtest_shard100_replica_xyz23.TEST./foobar/endpoint";
+ Matcher m = CLOUD_CORE_PATTERN.matcher(badMetricName);
assertFalse(m.matches());
- badCoreName = "core_solrtest_shards100_replica_x23";
- m = CLOUD_CORE_PATTERN.matcher(badCoreName);
+ badMetricName = "core_solrtest_shards100_replica_x23.QUERY.random.metric-name";
+ m = CLOUD_CORE_PATTERN.matcher(badMetricName);
assertFalse(m.matches());
}
+ @Test
+ public void testStandaloneCorePattern() {
+ String metricName = "core_test-core.TEST./foobar/endpoint";
+ Matcher m = STANDALONE_CORE_PATTERN.matcher(metricName);
+ assertTrue(m.find());
+ assertEquals("test-core", m.group(1));
+ }
+
static class TestSolrPrometheusFormatter extends SolrPrometheusFormatter {
@Override
public void exportDropwizardMetric(Metric dropwizardMetric, String metricName) {}
diff --git a/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java b/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java
index 0e25201cce0..7f15a6ae894 100644
--- a/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java
+++ b/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java
@@ -23,7 +23,9 @@ import com.codahale.metrics.SettableGauge;
import com.codahale.metrics.SharedMetricRegistries;
import java.lang.invoke.MethodHandles;
import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.stream.Collectors;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.solr.SolrTestCaseJ4;
@@ -83,8 +85,23 @@ public class TestPrometheusResponseWriter extends SolrTestCaseJ4 {
NamedList<Object> res = adminClient.request(req);
assertNotNull("null response from server", res);
String output = (String) res.get("response");
+
+ Set<String> seenTypeInfo = new HashSet<>();
+
List<String> filteredResponse =
- output.lines().filter(line -> !line.startsWith("#")).collect(Collectors.toList());
+ output
+ .lines()
+ .filter(
+ line -> {
+ if (!line.startsWith("#")) {
+ return true;
+ }
+ assertTrue(
+ "Prometheus exposition format cannot have duplicate TYPE information",
+ seenTypeInfo.add(line));
+ return false;
+ })
+ .collect(Collectors.toList());
filteredResponse.forEach(
(actualMetric) -> {
String actualValue;
(solr) 02/03: SOLR-17221: Fix Http2SolrClient merging case sensitive solr params (#3028)
Posted by ds...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9_8
in repository https://gitbox.apache.org/repos/asf/solr.git
commit 11075f7259b00da0614eb3236a33f712303395c7
Author: Yue Yu <82...@users.noreply.github.com>
AuthorDate: Thu Jan 16 00:55:21 2025 -0600
SOLR-17221: Fix Http2SolrClient merging case sensitive solr params (#3028)
If multiple query param keys are sent that only vary by case, they were wrongly merged when doing distributed search (sharded collections). This could likely occur for fielded parameters such as f.CASE_SENSITIVE_FIELD.facet.limit=50
Also: compose the request content directly using SolrParams.toQueryString(). This will avoid to use Jetty Fields as an intermediary mapping
---------
Co-authored-by: David Smiley <ds...@apache.org>
(cherry picked from commit 82083ea13ad2b2114d4311439be2f90ed868fac1)
---
solr/CHANGES.txt | 4 +++
.../solr/client/solrj/impl/Http2SolrClient.java | 20 ++++--------
.../client/solrj/impl/Http2SolrClientTest.java | 2 ++
.../client/solrj/impl/HttpJdkSolrClientTest.java | 2 ++
.../client/solrj/impl/HttpSolrClientTestBase.java | 36 ++++++++++++++++++++--
5 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 8989430fb30..de389c38463 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -18,6 +18,10 @@ Bug Fixes
* SOLR-17649: Fix a regression of faceting on multi-valued EnumFieldType, introduced by an earlier 9.x version. (Thomas Wöckinger, David Smiley)
+* SOLR-17221: If multiple query param keys are sent that only vary by case, they were wrongly merged when
+ doing distributed search (sharded collections). This could likely occur for fielded parameters such as
+ f.CASE_SENSITIVE_FIELD.facet.limit=50. (Yue Yu, David Smiley)
+
Dependency Upgrades
---------------------
(No changes)
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index cf8415ee64c..2f4ea6509a1 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -75,7 +75,6 @@ import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
-import org.eclipse.jetty.client.util.FormRequestContent;
import org.eclipse.jetty.client.util.InputStreamRequestContent;
import org.eclipse.jetty.client.util.InputStreamResponseListener;
import org.eclipse.jetty.client.util.MultiPartRequestContent;
@@ -90,7 +89,6 @@ import org.eclipse.jetty.http2.client.HTTP2Client;
import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2;
import org.eclipse.jetty.io.ClientConnector;
import org.eclipse.jetty.util.BlockingArrayQueue;
-import org.eclipse.jetty.util.Fields;
import org.eclipse.jetty.util.HttpCookieStore;
import org.eclipse.jetty.util.ssl.KeyStoreScanner;
import org.eclipse.jetty.util.ssl.SslContextFactory;
@@ -817,18 +815,12 @@ public class Http2SolrClient extends HttpSolrClientBase {
}
} else {
// application/x-www-form-urlencoded
- Fields fields = new Fields();
- Iterator<String> iter = wparams.getParameterNamesIterator();
- while (iter.hasNext()) {
- String key = iter.next();
- String[] vals = wparams.getParams(key);
- if (vals != null) {
- for (String val : vals) {
- fields.add(key, val);
- }
- }
- }
- req.body(new FormRequestContent(fields, FALLBACK_CHARSET));
+ String queryString = wparams.toQueryString();
+ // remove the leading "?" if there is any
+ queryString = queryString.startsWith("?") ? queryString.substring(1) : queryString;
+ req.body(
+ new StringRequestContent(
+ "application/x-www-form-urlencoded", queryString, FALLBACK_CHARSET));
}
return req;
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
index 0748b133d6a..15cfc0432e1 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
@@ -147,6 +147,8 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase {
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
SolrQuery q = new SolrQuery("foo");
q.setParam("a", MUST_ENCODE);
+ q.setParam("case_sensitive_param", "lowercase");
+ q.setParam("CASE_SENSITIVE_PARAM", "uppercase");
Http2SolrClient.Builder b =
new Http2SolrClient.Builder(url).withDefaultCollection(DEFAULT_CORE);
if (rp != null) {
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
index 07a202ccc65..701a8a0dcfc 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
@@ -179,6 +179,8 @@ public class HttpJdkSolrClientTest extends HttpSolrClientTestBase {
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
SolrQuery q = new SolrQuery("foo");
q.setParam("a", MUST_ENCODE);
+ q.setParam("case_sensitive_param", "lowercase");
+ q.setParam("CASE_SENSITIVE_PARAM", "uppercase");
HttpJdkSolrClient.Builder b = builder(url);
if (rp != null) {
b.withResponseParser(rp);
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
index 919af4ef266..5b9a352623e 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
@@ -116,6 +116,11 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
// param encoding
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ // case sensitive param
+ assertEquals(1, DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase", DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1, DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase", DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
}
public void testQueryPost() throws Exception {
@@ -128,10 +133,14 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1, DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase", DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1, DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase", DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded", DebugServlet.headers.get("content-type"));
// this validates that URI encoding has been applied - the content-length is smaller if not
- assertEquals("41", DebugServlet.headers.get("content-length"));
+ assertEquals("103", DebugServlet.headers.get("content-length"));
}
public void testQueryPut() throws Exception {
@@ -144,9 +153,13 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1, DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase", DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1, DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase", DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded", DebugServlet.headers.get("content-type"));
- assertEquals("41", DebugServlet.headers.get("content-length"));
+ assertEquals("103", DebugServlet.headers.get("content-length"));
}
public void testQueryXmlGet() throws Exception {
@@ -159,6 +172,10 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1, DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase", DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1, DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase", DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
}
@@ -172,6 +189,10 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1, DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase", DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1, DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase", DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded", DebugServlet.headers.get("content-type"));
}
@@ -186,6 +207,10 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1, DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase", DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1, DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase", DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded", DebugServlet.headers.get("content-type"));
}
@@ -264,6 +289,9 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
req.add(doc);
// non-ASCII characters and curly quotes should be URI-encoded
req.setParam("a", MUST_ENCODE);
+ // params should be case sensitive
+ req.setParam("case_sensitive_param", "lowercase");
+ req.setParam("CASE_SENSITIVE_PARAM", "uppercase");
try {
client.request(req);
@@ -281,6 +309,10 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals(contentType, DebugServlet.headers.get("content-type"));
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1, DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase", DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1, DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase", DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
if (wt == WT.XML) {
String requestBody = new String(DebugServlet.requestBody, StandardCharsets.UTF_8);
(solr) 01/03: SOLR-17649: Fix JSON faceting on multiValued number types (#3158)
Posted by ds...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9_8
in repository https://gitbox.apache.org/repos/asf/solr.git
commit 6382b70c52aa47a0c4877061b34073b20497e1d1
Author: Thomas Wöckinger <th...@users.noreply.github.com>
AuthorDate: Wed Feb 12 23:09:28 2025 +0100
SOLR-17649: Fix JSON faceting on multiValued number types (#3158)
* Add DV type checks to alert to a problem instead of silently returning nothing
* Clarify isNumeric
---------
Co-authored-by: Thomas Wöckinger <tw...@silbergrau.com>
Co-authored-by: David Smiley <ds...@apache.org>
(cherry picked from commit f66aa9ed07fa76b5db45186073a9b2ac20c01fd4)
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/search/facet/FacetField.java | 22 +++++----
.../search/facet/FacetFieldProcessorByArrayDV.java | 2 +-
.../org/apache/solr/search/facet/FieldUtil.java | 52 ++++++++++++++++------
.../solr/collection1/conf/schema_latest.xml | 2 +
.../apache/solr/response/TestRawTransformer.java | 3 +-
.../apache/solr/search/facet/TestJsonFacets.java | 29 ++++++++++++
7 files changed, 89 insertions(+), 23 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index e0b8509e0d5..8989430fb30 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -16,6 +16,8 @@ Bug Fixes
* SOLR-17652: Fix a bug that could cause long leader elections to leave PULL replicas in DOWN state forever. (hossman)
+* SOLR-17649: Fix a regression of faceting on multi-valued EnumFieldType, introduced by an earlier 9.x version. (Thomas Wöckinger, David Smiley)
+
Dependency Upgrades
---------------------
(No changes)
diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetField.java b/solr/core/src/java/org/apache/solr/search/facet/FacetField.java
index 77e2e373b3b..c4b0fdac2a3 100644
--- a/solr/core/src/java/org/apache/solr/search/facet/FacetField.java
+++ b/solr/core/src/java/org/apache/solr/search/facet/FacetField.java
@@ -18,9 +18,9 @@ package org.apache.solr.search.facet;
import java.util.HashMap;
import java.util.Map;
+import org.apache.lucene.index.DocValuesType;
import org.apache.solr.common.SolrException;
import org.apache.solr.schema.FieldType;
-import org.apache.solr.schema.NumberType;
import org.apache.solr.schema.SchemaField;
public class FacetField extends FacetRequestSorted {
@@ -95,9 +95,9 @@ public class FacetField extends FacetRequestSorted {
return new FacetFieldProcessorByArrayDV(fcontext, this, sf);
}
- NumberType ntype = ft.getNumberType();
+ final boolean isNumber = ft.getNumberType() != null;
// ensure we can support the requested options for numeric faceting:
- if (ntype != null) {
+ if (isNumber) {
if (prefix != null) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
@@ -139,11 +139,11 @@ public class FacetField extends FacetRequestSorted {
// FieldType.getDocValuesType()
if (!multiToken) {
- if (mincount > 0 && prefix == null && (ntype != null || method == FacetMethod.DVHASH)) {
+ if (mincount > 0 && prefix == null && (isNumber || method == FacetMethod.DVHASH)) {
// TODO can we auto-pick for strings when term cardinality is much greater than DocSet
// cardinality? or if we don't know cardinality but DocSet size is very small
return new FacetFieldProcessorByHashDV(fcontext, this, sf);
- } else if (ntype == null) {
+ } else if (isNumber == false) {
// single valued string...
return new FacetFieldProcessorByArrayDV(fcontext, this, sf);
} else {
@@ -152,12 +152,13 @@ public class FacetField extends FacetRequestSorted {
}
}
- if (sf.hasDocValues() && sf.getType().isPointField()) {
+ // multi-valued after this point
+
+ if (sf.hasDocValues() && isNumber && dvType(fcontext, field) != DocValuesType.SORTED_SET) {
+ // doesn't support SORTED_SET DV (it's a TODO)
return new FacetFieldProcessorByHashDV(fcontext, this, sf);
}
- // multi-valued after this point
-
if (sf.hasDocValues() || method == FacetMethod.DV || !sf.isUninvertible()) {
// single and multi-valued string docValues
return new FacetFieldProcessorByArrayDV(fcontext, this, sf);
@@ -167,6 +168,11 @@ public class FacetField extends FacetRequestSorted {
return new FacetFieldProcessorByArrayUIF(fcontext, this, sf);
}
+ private static DocValuesType dvType(FacetContext fcontext, String field) {
+ var fieldInfo = fcontext.searcher.getFieldInfos().fieldInfo(field);
+ return fieldInfo == null ? null : fieldInfo.getDocValuesType();
+ }
+
@Override
public FacetMerger createFacetMerger(Object prototype) {
return new FacetFieldMerger(this);
diff --git a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByArrayDV.java b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByArrayDV.java
index cb7d5492a91..27fcc38646c 100644
--- a/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByArrayDV.java
+++ b/solr/core/src/java/org/apache/solr/search/facet/FacetFieldProcessorByArrayDV.java
@@ -38,7 +38,7 @@ import org.apache.solr.search.facet.SweepCountAware.SegCountGlobal;
import org.apache.solr.search.facet.SweepCountAware.SegCountPerSeg;
import org.apache.solr.uninverting.FieldCacheImpl;
-/** Grabs values from {@link DocValues}. */
+/** Grabs values from {@link SortedDocValues} or {@link SortedDocValues}. */
class FacetFieldProcessorByArrayDV extends FacetFieldProcessorByArray {
static boolean unwrap_singleValued_multiDv = true; // only set to false for test coverage
diff --git a/solr/core/src/java/org/apache/solr/search/facet/FieldUtil.java b/solr/core/src/java/org/apache/solr/search/facet/FieldUtil.java
index 07abc6c6ab1..6baac331c92 100644
--- a/solr/core/src/java/org/apache/solr/search/facet/FieldUtil.java
+++ b/solr/core/src/java/org/apache/solr/search/facet/FieldUtil.java
@@ -18,8 +18,12 @@ package org.apache.solr.search.facet;
import java.io.IOException;
import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.DocValuesType;
+import org.apache.lucene.index.FieldInfo;
+import org.apache.lucene.index.LeafReader;
import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.index.SortedDocValues;
+import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.index.SortedSetDocValues;
import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.util.BytesRef;
@@ -44,27 +48,49 @@ public class FieldUtil {
public static SortedDocValues getSortedDocValues(
QueryContext context, SchemaField field, QParser qparser) throws IOException {
- SortedDocValues si =
- context.searcher().getSlowAtomicReader().getSortedDocValues(field.getName());
- // if (!field.hasDocValues() && (field.getType() instanceof StrField || field.getType()
- // instanceof TextField)) {
- // }
-
- return si == null ? DocValues.emptySorted() : si;
+ var reader = context.searcher().getSlowAtomicReader();
+ var dv = reader.getSortedDocValues(field.getName());
+ checkDvType(dv, field, reader);
+ return dv == null ? DocValues.emptySorted() : dv;
}
public static SortedSetDocValues getSortedSetDocValues(
QueryContext context, SchemaField field, QParser qparser) throws IOException {
- SortedSetDocValues si =
- context.searcher().getSlowAtomicReader().getSortedSetDocValues(field.getName());
- return si == null ? DocValues.emptySortedSet() : si;
+ var reader = context.searcher().getSlowAtomicReader();
+ var dv = reader.getSortedSetDocValues(field.getName());
+ checkDvType(dv, field, reader);
+ return dv == null ? DocValues.emptySortedSet() : dv;
}
public static NumericDocValues getNumericDocValues(
QueryContext context, SchemaField field, QParser qparser) throws IOException {
- SolrIndexSearcher searcher = context.searcher();
- NumericDocValues si = searcher.getSlowAtomicReader().getNumericDocValues(field.getName());
- return si == null ? DocValues.emptyNumeric() : si;
+ var reader = context.searcher().getSlowAtomicReader();
+ var dv = reader.getNumericDocValues(field.getName());
+ checkDvType(dv, field, reader);
+ return dv == null ? DocValues.emptyNumeric() : dv;
+ }
+
+ private static void checkDvType(Object dv, SchemaField field, LeafReader reader) {
+ if (dv == null) {
+ return;
+ }
+ FieldInfo fieldInfo = reader.getFieldInfos().fieldInfo(field.getName());
+ if (fieldInfo == null) {
+ return;
+ }
+ var dvType = fieldInfo.getDocValuesType();
+ if (dvType == DocValuesType.NONE) {
+ return;
+ } else if (dvType == DocValuesType.SORTED) {
+ if (dv instanceof SortedDocValues) return;
+ } else if (dvType == DocValuesType.SORTED_SET) {
+ if (dv instanceof SortedSetDocValues) return;
+ } else if (dvType == DocValuesType.NUMERIC) {
+ if (dv instanceof NumericDocValues) return;
+ } else if (dvType == DocValuesType.SORTED_NUMERIC) {
+ if (dv instanceof SortedNumericDocValues) return;
+ }
+ throw new IllegalStateException("Unexpected DocValues type " + dvType + " for field " + field);
}
/**
diff --git a/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml b/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml
index 7d1e09faf28..87053fbd6ce 100644
--- a/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml
+++ b/solr/core/src/test-files/solr/collection1/conf/schema_latest.xml
@@ -729,6 +729,8 @@
<fieldType name="currency" class="solr.CurrencyField" precisionStep="8" defaultCurrency="USD"
currencyConfig="currency.xml"/>
+ <fieldType name="severityType" class="solr.EnumFieldType" enumsConfig="enumsConfig.xml" enumName="severity"/>
+ <field name="severity_mv" type="severityType" indexed="true" stored="true" multiValued="true" docValues="true" />
<!-- some examples for different languages (generally ordered by ISO code) -->
<!-- REMOVED. these reference things not in the test config, like lang/stopwords_en.txt -->
diff --git a/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java b/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
index c2208d5204b..0db7abcf39b 100644
--- a/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
+++ b/solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
@@ -90,7 +90,8 @@ public class TestRawTransformer extends SolrCloudTestCase {
"stopwords.txt",
"synonyms.txt",
"protwords.txt",
- "currency.xml"
+ "currency.xml",
+ "enumsConfig.xml"
}) {
Files.copy(Path.of(src_dir, file), confDir.resolve(file));
}
diff --git a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
index 66673b77f57..9ca008dbe5d 100644
--- a/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
+++ b/solr/core/src/test/org/apache/solr/search/facet/TestJsonFacets.java
@@ -4908,6 +4908,35 @@ public class TestJsonFacets extends SolrTestCaseHS {
+ "}");
}
+ @Test
+ public void testMultivalueEnumTypes() throws Exception {
+ final Client client = Client.localClient();
+
+ final SolrParams p = params("rows", "0");
+
+ client.deleteByQuery("*:*", null);
+
+ List<SolrInputDocument> docsToAdd = new ArrayList<>(6);
+ docsToAdd.add(sdoc("id", "1", "severity_mv", "Not Available", "severity_mv", "Low"));
+ docsToAdd.add(sdoc("id", "2", "severity_mv", "Low", "severity_mv", "Medium"));
+ docsToAdd.add(sdoc("id", "3", "severity_mv", "Medium", "severity_mv", "High"));
+ docsToAdd.add(sdoc("id", "4", "severity_mv", "High", "severity_mv", "Not Available"));
+ docsToAdd.add(sdoc("id", "5", "severity_mv", "Not Available", "severity_mv", "Low"));
+ docsToAdd.add(sdoc("id", "6", "severity_mv", "Low", "severity_mv", "Medium"));
+
+ Collections.shuffle(docsToAdd, random());
+ for (SolrInputDocument doc : docsToAdd) {
+ client.add(doc, null);
+ }
+ client.commit();
+
+ client.testJQ(
+ params(p, "q", "*:*", "json.facet", "{f:{type:terms, method:enum, field:severity_mv}}"),
+ "facets=={ count:6,"
+ + "f:{ buckets:[{val:Low,count:4},{val:'Not Available',count:3},{val:Medium,count:3},{val:High,count:2}] }"
+ + "}");
+ }
+
@Test
public void testFacetValueTypes() throws Exception {
doFacetValueTypeValidation(Client.localClient());