Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17043: Remove SolrClient path pattern matching #3238

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,8 @@ public static void checkDiskSpace(
.add("key", indexSizeMetricName)
.add("key", freeDiskSpaceMetricName);
SolrResponse rsp =
new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/metrics", params)
new GenericSolrRequest(
SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
.process(cloudManager.getSolrClient());

Number size = (Number) rsp.getResponse().findRecursive("metrics", indexSizeMetricName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import org.apache.solr.api.Api;
import org.apache.solr.api.ApiBag;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.io.stream.expr.Expressible;
Expand Down Expand Up @@ -968,7 +967,7 @@ private static class PerReplicaCallable extends CollectionRequiringSolrRequest<S
int maxWait;

PerReplicaCallable(Replica replica, String prop, int expectedZkVersion, int maxWait) {
super(METHOD.GET, "/config/" + ZNODEVER);
super(METHOD.GET, "/config/" + ZNODEVER, SolrRequestType.ADMIN);
this.replica = replica;
this.expectedZkVersion = expectedZkVersion;
this.prop = prop;
Expand Down Expand Up @@ -1032,11 +1031,6 @@ public Boolean call() throws Exception {
protected SolrResponse createResponse(SolrClient client) {
return null;
}

@Override
public String getRequestType() {
return SolrRequest.SolrRequestType.ADMIN.toString();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.Version;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.impl.HttpSolrClient;
import org.apache.solr.client.solrj.request.CollectionRequiringSolrRequest;
Expand Down Expand Up @@ -357,7 +356,7 @@ private static class GetZkSchemaVersionCallable

GetZkSchemaVersionCallable(
String baseUrl, String coreName, int expectedZkVersion, ZkController zkController) {
super(METHOD.GET, "/schema/zkversion");
super(METHOD.GET, "/schema/zkversion", SolrRequestType.ADMIN);
this.zkController = zkController;
this.baseUrl = baseUrl;
this.coreName = coreName;
Expand Down Expand Up @@ -413,11 +412,6 @@ public Integer call() throws Exception {
protected SolrResponse createResponse(SolrClient client) {
return null;
}

@Override
public String getRequestType() {
return SolrRequest.SolrRequestType.ADMIN.toString();
}
}

public static class FieldExistsException extends SolrException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void testPathIsAddedToContext() throws IOException, SolrServerException {
private static class SystemInfoRequest extends SolrRequest<QueryResponse> {

public SystemInfoRequest() {
super(METHOD.GET, "/admin/info/system");
super(METHOD.GET, "/admin/info/system", SolrRequest.SolrRequestType.ADMIN);
}

@Override
Expand All @@ -60,10 +60,5 @@ public SolrParams getParams() {
protected QueryResponse createResponse(final SolrClient client) {
return new QueryResponse();
}

@Override
public String getRequestType() {
return SolrRequest.SolrRequestType.ADMIN.toString();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ public void testAggregateNodeLevelMetrics() throws SolrServerException, IOExcept
cloudClient.query(collection2, solrQuery);

NamedList<Object> response =
cloudClient.request(new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/metrics"));
cloudClient.request(
new GenericSolrRequest(
SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN));

NamedList<Object> metrics = (NamedList<Object>) response.get("metrics");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ public void proxySystemInfoHandlerAllNodes() throws IOException, SolrServerExcep
public void proxyMetricsHandlerAllNodes() throws IOException, SolrServerException {
MapSolrParams params = new MapSolrParams(Collections.singletonMap("nodes", "all"));
GenericSolrRequest req =
new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/metrics", params);
new GenericSolrRequest(
SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
SimpleSolrResponse rsp = req.process(solrClient, null);
NamedList<Object> nl = rsp.getResponse();
assertEquals(3, nl.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public void testHealthCheckHandler() throws Exception {
// as compared with testHealthCheckHandlerWithCloudClient
// (Not sure if that's actually a good thing -- but it's how the existing test worked)
final var genericHealthcheck =
new GenericSolrRequest(SolrRequest.METHOD.GET, HEALTH_CHECK_HANDLER_PATH);
new GenericSolrRequest(
SolrRequest.METHOD.GET, HEALTH_CHECK_HANDLER_PATH, SolrRequest.SolrRequestType.ADMIN);
assertEquals(
CommonParams.OK,
genericHealthcheck.process(cluster.getSolrClient()).getResponse().get(CommonParams.STATUS));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@
import org.apache.lucene.tests.util.LuceneTestCase;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest.METHOD;
import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
import org.apache.solr.client.solrj.impl.NoOpResponseParser;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.metrics.SolrMetricManager;
Expand Down Expand Up @@ -79,9 +81,8 @@ public static void clearMetricsRegistries() {
@Test
public void testPrometheusStructureOutput() throws Exception {
ModifiableSolrParams params = new ModifiableSolrParams();
params.set("qt", "/admin/metrics");
params.set("wt", "prometheus");
QueryRequest req = new QueryRequest(params);
var req = new GenericSolrRequest(METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN, params);
req.setResponseParser(new NoOpResponseParser("prometheus"));

try (SolrClient adminClient = getHttpSolrClient(solrClientTestRule.getBaseUrl())) {
Expand Down Expand Up @@ -135,9 +136,8 @@ public void testPrometheusDummyOutput() throws Exception {
String expectedJvm = "solr_metrics_jvm_gc{item=\"dummyMetrics\"} 0.0";

ModifiableSolrParams params = new ModifiableSolrParams();
params.set("qt", "/admin/metrics");
params.set("wt", "prometheus");
QueryRequest req = new QueryRequest(params);
var req = new GenericSolrRequest(METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN, params);
req.setResponseParser(new NoOpResponseParser("prometheus"));

try (SolrClient adminClient = getHttpSolrClient(solrClientTestRule.getBaseUrl())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,14 @@ public void testBasicAuth() throws Exception {
.build();
} else {
GenericSolrRequest genericSolrRequest =
new GenericSolrRequest(SolrRequest.METHOD.POST, authcPrefix);
new GenericSolrRequest(
SolrRequest.METHOD.POST, authcPrefix, SolrRequest.SolrRequestType.ADMIN);
genericSolrRequest.setContentWriter(
new StringPayloadContentWriter(command, CommonParams.JSON_MIME));
genericReq = genericSolrRequest;
}

// avoid bad connection races due to shutdown
// avoid bad connection races due to shutdownn
final var httpClient = ((CloudLegacySolrClient) cluster.getSolrClient()).getHttpClient();
httpClient.getConnectionManager().closeExpiredConnections();
httpClient.getConnectionManager().closeIdleConnections(1, TimeUnit.MILLISECONDS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Map;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
Expand Down Expand Up @@ -129,7 +130,8 @@ public void test() throws IOException, SolrServerException {
public void testAdminApi() throws Exception {
CloudSolrClient cloudClient = cluster.getSolrClient();

cloudClient.request(new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/metrics"));
cloudClient.request(
new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/metrics", SolrRequestType.ADMIN));
var finishedSpans = getAndClearSpans();
assertEquals("get:/admin/metrics", finishedSpans.get(0).getName());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.solr.prometheus.scraper;

import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.prometheus.client.Collector;
Expand All @@ -35,10 +37,12 @@
import net.thisptr.jackson.jq.JsonQuery;
import net.thisptr.jackson.jq.exception.JsonQueryException;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest.METHOD;
import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.Http2SolrClient;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.prometheus.collector.MetricSamples;
import org.apache.solr.prometheus.exporter.MetricsQuery;
Expand Down Expand Up @@ -130,29 +134,35 @@ protected MetricSamples request(SolrClient client, MetricsQuery query) throws IO
zkHostLabelValue = ((CloudSolrClient) client).getClusterStateProvider().getQuorumHosts();
}

QueryRequest queryRequest = new QueryRequest(query.getParameters());
queryRequest.setPath(query.getPath());

NamedList<Object> queryResponse;
SolrRequestType requestType = SolrRequestType.QUERY;
boolean requiresCollection = true;
if (ADMIN_PATHS.contains(query.getPath())) {
requestType = SolrRequestType.ADMIN;
requiresCollection = false;
}
var request =
new GenericSolrRequest(METHOD.GET, query.getPath(), requestType, query.getParameters());
request.setRequiresCollection(requiresCollection);
NamedList<Object> response;
try {
if (query.getCollection().isEmpty() && query.getCore().isEmpty()) {
queryResponse = client.request(queryRequest);
response = client.request(request);
} else if (query.getCore().isPresent()) {
queryResponse = client.request(queryRequest, query.getCore().get());
response = client.request(request, query.getCore().get());
} else if (query.getCollection().isPresent()) {
queryResponse = client.request(queryRequest, query.getCollection().get());
response = client.request(request, query.getCollection().get());
} else {
throw new AssertionError("Invalid configuration");
}
if (queryResponse == null) { // ideally we'd make this impossible
if (response == null) { // ideally we'd make this impossible
throw new RuntimeException("no response from server");
}
} catch (SolrServerException | IOException e) {
log.error("failed to request: {}", queryRequest.getPath(), e);
log.error("failed to request: {}", request.getPath(), e);
return samples;
}

JsonNode jsonNode = OBJECT_MAPPER.readTree((String) queryResponse.get("response"));
JsonNode jsonNode = OBJECT_MAPPER.readTree((String) response.get("response"));

for (JsonQuery jsonQuery : query.getJsonQueries()) {
try {
Expand Down
21 changes: 18 additions & 3 deletions solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;
Expand Down Expand Up @@ -90,6 +91,7 @@ public enum SolrClientContext {

private METHOD method = METHOD.GET;
private String path = null;
private SolrRequestType requestType = SolrRequestType.UNSPECIFIED;
private Map<String, String> headers;
private List<String> preferredNodes;

Expand Down Expand Up @@ -127,9 +129,10 @@ public String getBasicAuthPassword() {
// ---------------------------------------------------------
// ---------------------------------------------------------

public SolrRequest(METHOD m, String path) {
public SolrRequest(METHOD m, String path, SolrRequestType requestType) {
this.method = m;
this.path = path;
this.requestType = requestType;
}

// ---------------------------------------------------------
Expand Down Expand Up @@ -185,8 +188,20 @@ public void setQueryParams(Set<String> queryParams) {
this.queryParams = queryParams;
}

/** This method defines the type of this Solr request. */
public abstract String getRequestType();
/**
* The type of this Solr request.
*
* <p>Pattern matches {@link SolrRequest#getPath} to identify ADMIN requests and other special
* cases. Overriding this method may affect request routing within various clients (i.e. {@link
* CloudSolrClient}).
*/
public SolrRequestType getRequestType() {
return requestType;
}

public void setRequestType(SolrRequestType requestType) {
this.requestType = requestType;
}

/**
* The parameters for this request; never null. The runtime type may be mutable but modifications
Expand Down
Loading