Skip to content

Commit

Permalink
SOLR-17390: EmbeddedSolrServer now considers the ResponseParser (#2774)
Browse files Browse the repository at this point in the history 
And
* Moved HttpSolrCall.getResponseWriter to SolrQueryRequest
* Subtle improvements to make ContentStream work when they might not have
  • Loading branch information
@dsmiley
dsmiley authored Nov 4, 2024
1 parent e849fd0 commit c5c538a
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 77 deletions.
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ Improvements

* SOLR-17528: Introduce -y short option to bin/solr start --no-prompt option. Aligns with bin/solr package tool. (Eric Pugh)

* SOLR-17390: EmbeddedSolrServer now considers the ResponseParser (David Smiley)

Optimizations
---------------------
* SOLR-14985: Solrj CloudSolrClient with Solr URLs had serious performance regressions (since the
Expand Down
162 changes: 98 additions & 64 deletions solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,28 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.function.Supplier;
import org.apache.lucene.search.TotalHits.Relation;
import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.StreamingResponseCallback;
import org.apache.solr.client.solrj.impl.BinaryRequestWriter;
import org.apache.solr.client.solrj.impl.BinaryResponseParser;
import org.apache.solr.client.solrj.impl.InputStreamResponseParser;
import org.apache.solr.client.solrj.request.ContentStreamUpdateRequest;
import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.common.SolrDocument;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.MapSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;
import org.apache.solr.common.util.ContentStreamBase;
Expand All @@ -55,6 +58,7 @@
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.response.BinaryResponseWriter;
import org.apache.solr.response.QueryResponseWriterUtil;
import org.apache.solr.response.ResultContext;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.servlet.SolrRequestParsers;
Expand Down Expand Up @@ -165,13 +169,13 @@ public NamedList<Object> request(SolrRequest<?> request, String coreName)
try {
SolrQueryRequest req =
_parser.buildRequestFrom(
null, request.getParams(), getContentStreams(request), request.getUserPrincipal());
null, getParams(request), getContentStreams(request), request.getUserPrincipal());
req.getContext().put("httpMethod", request.getMethod().name());
req.getContext().put(PATH, path);
SolrQueryResponse resp = new SolrQueryResponse();
handler.handleRequest(req, resp);
checkForExceptions(resp);
return BinaryResponseWriter.getParsedResponse(req, resp);
return writeResponse(request, req, resp);
} catch (IOException | SolrException iox) {
throw iox;
} catch (Exception ex) {
Expand All @@ -196,10 +200,7 @@ public NamedList<Object> request(SolrRequest<?> request, String coreName)
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "No such core: " + coreName);
}

SolrParams params = request.getParams();
if (params == null) {
params = new ModifiableSolrParams();
}
SolrParams params = getParams(request);

// Extract the handler from the path or params
handler = core.getRequestHandler(path);
Expand All @@ -217,62 +218,18 @@ public NamedList<Object> request(SolrRequest<?> request, String coreName)
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "unknown handler: " + path);
}
req =
_parser.buildRequestFrom(
core, params, getContentStreams(request), request.getUserPrincipal());
core.getSolrConfig()
.getRequestParsers()
.buildRequestFrom(
core, params, getContentStreams(request), request.getUserPrincipal());
req.getContext().put(PATH, path);
req.getContext().put("httpMethod", request.getMethod().name());
SolrQueryResponse rsp = new SolrQueryResponse();
SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));

core.execute(handler, req, rsp);
checkForExceptions(rsp);

// Check if this should stream results
if (request.getStreamingResponseCallback() != null) {
try {
final StreamingResponseCallback callback = request.getStreamingResponseCallback();
BinaryResponseWriter.Resolver resolver =
new BinaryResponseWriter.Resolver(req, rsp.getReturnFields()) {
@Override
public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOException {
// write an empty list...
SolrDocumentList docs = new SolrDocumentList();
docs.setNumFound(ctx.getDocList().matches());
docs.setNumFoundExact(ctx.getDocList().hitCountRelation() == Relation.EQUAL_TO);
docs.setStart(ctx.getDocList().offset());
docs.setMaxScore(ctx.getDocList().maxScore());
codec.writeSolrDocumentList(docs);

// This will transform
writeResultsBody(ctx, codec);
}
};

try (var out =
new ByteArrayOutputStream() {
ByteArrayInputStream toInputStream() {
return new ByteArrayInputStream(buf, 0, count);
}
}) {
createJavaBinCodec(callback, resolver)
.setWritableDocFields(resolver)
.marshal(rsp.getValues(), out);

try (ByteArrayInputStream in = out.toInputStream()) {
@SuppressWarnings({"unchecked"})
NamedList<Object> resolved =
(NamedList<Object>) new JavaBinCodec(resolver).unmarshal(in);
return resolved;
}
}
} catch (Exception ex) {
throw new RuntimeException(ex);
}
}

// Now write it out
NamedList<Object> normalized = BinaryResponseWriter.getParsedResponse(req, rsp);
return normalized;
return writeResponse(request, req, rsp);
} catch (IOException | SolrException iox) {
throw iox;
} catch (Exception ex) {
Expand All @@ -285,12 +242,89 @@ ByteArrayInputStream toInputStream() {
}
}

private Set<ContentStream> getContentStreams(SolrRequest<?> request) throws IOException {
if (request.getMethod() == SolrRequest.METHOD.GET) return null;
private static SolrParams getParams(SolrRequest<?> request) {
var params = request.getParams();
var responseParser = request.getResponseParser();
if (responseParser == null) {
responseParser = new BinaryResponseParser();
}
var addParams =
new MapSolrParams(
Map.of(
CommonParams.WT,
responseParser.getWriterType(),
CommonParams.VERSION,
responseParser.getVersion()));
return SolrParams.wrapDefaults(addParams, params);
}

private NamedList<Object> writeResponse(
SolrRequest<?> request, SolrQueryRequest req, SolrQueryResponse rsp) throws IOException {
ResponseParser responseParser = request.getResponseParser();
if (responseParser == null) {
responseParser = new BinaryResponseParser();
}
StreamingResponseCallback callback = request.getStreamingResponseCallback();
// TODO refactor callback to be a special responseParser that we check for
// TODO if responseParser is a special/internal NamedList ResponseParser, just return NL

var byteBuffer =
new ByteArrayOutputStream() {
ByteArrayInputStream toInputStream() {
return new ByteArrayInputStream(buf, 0, count);
}
};

if (callback == null) {
QueryResponseWriterUtil.writeQueryResponse(
byteBuffer, req.getResponseWriter(), req, rsp, null);
} else {
// mostly stream results to the callback; rest goes into the byteBuffer
if (!(responseParser instanceof BinaryResponseParser))
throw new IllegalArgumentException(
"Only javabin is supported when using a streaming response callback");
var resolver =
new BinaryResponseWriter.Resolver(req, rsp.getReturnFields()) {
@Override
public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOException {
// write an empty list...
SolrDocumentList docs = new SolrDocumentList();
docs.setNumFound(ctx.getDocList().matches());
docs.setNumFoundExact(ctx.getDocList().hitCountRelation() == Relation.EQUAL_TO);
docs.setStart(ctx.getDocList().offset());
docs.setMaxScore(ctx.getDocList().maxScore());
codec.writeSolrDocumentList(docs);

// This will transform
writeResultsBody(ctx, codec);
}
};

// invoke callbacks, and writes the rest to byteBuffer
try (var javaBinCodec = createJavaBinCodec(callback, resolver)) {
javaBinCodec.setWritableDocFields(resolver).marshal(rsp.getValues(), byteBuffer);
}
}

if (responseParser instanceof InputStreamResponseParser) {
// SPECIAL CASE
NamedList<Object> namedList = new NamedList<>();
namedList.add("stream", byteBuffer.toInputStream());
namedList.add("responseStatus", 200); // always by this point
return namedList;
}

// note: don't bother using the Reader variant; it often throws UnsupportedOperationException
return responseParser.processResponse(byteBuffer.toInputStream(), null);
}

/** A list of streams, non-null. */
private List<ContentStream> getContentStreams(SolrRequest<?> request) throws IOException {
if (request.getMethod() == SolrRequest.METHOD.GET) return List.of();
if (request instanceof ContentStreamUpdateRequest) {
final ContentStreamUpdateRequest csur = (ContentStreamUpdateRequest) request;
final Collection<ContentStream> cs = csur.getContentStreams();
if (cs != null) return new HashSet<>(cs);
if (cs != null) return new ArrayList<>(cs);
}

final RequestWriter.ContentWriter contentWriter = request.getContentWriter(null);
Expand All @@ -308,7 +342,7 @@ private Set<ContentStream> getContentStreams(SolrRequest<?> request) throws IOEx

final byte[] buf = baos.toByteArray();
if (buf.length > 0) {
return Collections.singleton(
return List.of(
new ContentStreamBase() {

@Override
Expand All @@ -323,7 +357,7 @@ public String getContentType() {
});
}

return null;
return List.of();
}

private JavaBinCodec createJavaBinCodec(
Expand Down
14 changes: 14 additions & 0 deletions solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
import org.apache.solr.response.QueryResponseWriter;
import org.apache.solr.schema.IndexSchema;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.servlet.HttpSolrCall;
Expand Down Expand Up @@ -188,4 +189,17 @@ default CoreContainer getCoreContainer() {
default CloudDescriptor getCloudDescriptor() {
return getCore().getCoreDescriptor().getCloudDescriptor();
}

/** The writer to use for this request, considering {@link CommonParams#WT}. Never null. */
default QueryResponseWriter getResponseWriter() {
// it's weird this method is here instead of SolrQueryResponse, but it's practical/convenient
SolrCore core = getCore();
String wt = getParams().get(CommonParams.WT);
if (core != null) {
return core.getQueryResponseWriter(wt);
} else {
return SolrCore.DEFAULT_RESPONSE_WRITERS.getOrDefault(
wt, SolrCore.DEFAULT_RESPONSE_WRITERS.get("standard"));
}
}
}
8 changes: 1 addition & 7 deletions solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -934,13 +934,7 @@ protected void logAndFlushAdminRequest(SolrQueryResponse solrResp) throws IOExce
* returns the default query response writer Note: This method must not return null
*/
protected QueryResponseWriter getResponseWriter() {
String wt = solrReq.getParams().get(CommonParams.WT);
if (core != null) {
return core.getQueryResponseWriter(wt);
} else {
return SolrCore.DEFAULT_RESPONSE_WRITERS.getOrDefault(
wt, SolrCore.DEFAULT_RESPONSE_WRITERS.get("standard"));
}
return solrReq.getResponseWriter();
}

protected void handleAdmin(SolrQueryResponse solrResp) {
Expand Down
11 changes: 9 additions & 2 deletions solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,18 @@ public SolrQueryRequest buildRequestFrom(
private SolrQueryRequest buildRequestFrom(
SolrCore core,
SolrParams params,
Collection<ContentStream> streams,
Collection<ContentStream> streams, // might be added to but caller shouldn't depend on it
RTimerTree requestTimer,
final HttpServletRequest req,
final Principal principal)
throws Exception {
// ensure streams is non-null and mutable so we can easily add to it
if (streams == null) {
streams = new ArrayList<>();
} else if (!(streams instanceof ArrayList)) {
streams = new ArrayList<>(streams);
}

// The content type will be applied to all streaming content
String contentType = params.get(CommonParams.STREAM_CONTENTTYPE);

Expand Down Expand Up @@ -293,7 +300,7 @@ public HttpSolrCall getHttpSolrCall() {
return httpSolrCall;
}
};
if (streams != null && streams.size() > 0) {
if (!streams.isEmpty()) {
q.setContentStreams(streams);
}
return q;
Expand Down
7 changes: 6 additions & 1 deletion solr/core/src/test/org/apache/solr/response/TestRawTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.regex.Pattern;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.NoOpResponseParser;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
Expand Down Expand Up @@ -60,7 +61,11 @@ public static void beforeClass() throws Exception {
if (random().nextBoolean()) {
initStandalone();
JSR.start();
CLIENT = JSR.newClient();
if (random().nextBoolean()) {
CLIENT = JSR.newClient();
} else {
CLIENT = new EmbeddedSolrServer(JSR.getCoreContainer(), null);
}
} else {
initCloud();
CLIENT = JSR.newClient();
Expand Down
3 changes: 2 additions & 1 deletion solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ public final T process(SolrClient client, String collection)
throws SolrServerException, IOException {
long startNanos = System.nanoTime();
T res = createResponse(client);
res.setResponse(client.request(this, collection));
var namedList = client.request(this, collection);
res.setResponse(namedList);
long endNanos = System.nanoTime();
res.setElapsedTime(TimeUnit.NANOSECONDS.toMillis(endNanos - startNanos));
return res;
Expand Down
3 changes: 1 addition & 2 deletions solr/solrj/src/java/org/apache/solr/client/solrj/impl/NoOpResponseParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@
import org.apache.solr.common.util.NamedList;

/**
* Simply puts the entire response into an entry in a NamedList. This parser isn't parse response
* into a QueryResponse.
* A special parser that puts the entire response into a string "response" field in the NamedList.
*/
public class NoOpResponseParser extends ResponseParser {

Expand Down

0 comments on commit c5c538a

Please sign in to comment.