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

defer monitorMatchType support (always use simple matching) #3

Merged
Merged
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 @@ -27,8 +27,6 @@ private MonitorConstants() {}
public static final String MONITOR_DOCUMENT_KEY = "monitorDocument";
public static final String MONITOR_QUERIES_KEY = "queries";
public static final String MONITOR_QUERIES_RUN = "queriesRun";
public static final String QUERY_MATCH_TYPE_KEY = "monitorMatchType";
public static final String MONITOR_OUTPUT_KEY = "monitor";
public static final String WRITE_TO_DOC_LIST_KEY = "writeToDocList";
public static final String HITS_KEY = "hits";
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,16 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.apache.lucene.monitor.HighlightsMatch;
import org.apache.lucene.monitor.MonitorFields;
import org.apache.lucene.monitor.MultiMatchingQueries;
import org.apache.lucene.monitor.QueryMatch;
import org.apache.lucene.util.BytesRef;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.monitor.MonitorConstants;

@SuppressWarnings("unchecked")
class QueryMatchResponseCodec {

static Map<String, Object> mergeResponses(
List<NamedList<Object>> responses, QueryMatchType type) {
static Map<String, Object> mergeResponses(List<NamedList<Object>> responses) {
Map<Integer, List<QueryMatchWrapper>> wrappedMerged = new HashMap<>();
int queriesRun = 0;
for (var response : responses) {
Expand All @@ -58,7 +54,7 @@ static Map<String, Object> mergeResponses(
SolrException.ErrorCode.BAD_REQUEST, MONITOR_OUTPUT_KEY + " must be a map");
}
Map<String, Object> singleMonitorOutput = (Map<String, Object>) smq;
decodeToWrappers(singleMonitorOutput, wrappedMerged, type);
decodeToWrappers(singleMonitorOutput, wrappedMerged);
var delta = singleMonitorOutput.get(MONITOR_QUERIES_RUN);
if (!(delta instanceof Integer)) {
throw new SolrException(
Expand All @@ -83,7 +79,7 @@ static Map<String, Object> mergeResponses(
}

private static void decodeToWrappers(
Object object, Map<Integer, List<QueryMatchWrapper>> output, QueryMatchType type) {
Object object, Map<Integer, List<QueryMatchWrapper>> output) {
if (!(object instanceof Map)) {
throw new IllegalArgumentException("input object should be a map");
}
Expand All @@ -109,15 +105,9 @@ private static void decodeToWrappers(
int doc = (Integer) docRaw;
List<QueryMatchWrapper> wrappers = output.computeIfAbsent(doc, i -> new ArrayList<>());
List<Object> serializableFormMQs = (List<Object>) monitorQueriesRaw;
if (type == QueryMatchType.SIMPLE) {
serializableFormMQs.forEach(
serializableForm ->
wrappers.add(SimpleQueryMatchWrapper.fromSerializableForm(serializableForm)));
} else if (type == QueryMatchType.HIGHLIGHTS) {
serializableFormMQs.forEach(
serializableForm ->
wrappers.add(HighlightedQueryMatchWrapper.fromSerializableForm(serializableForm)));
}
serializableFormMQs.forEach(
serializableForm ->
wrappers.add(SimpleQueryMatchWrapper.fromSerializableForm(serializableForm)));
}
}

Expand All @@ -129,14 +119,6 @@ static void simpleEncode(
matchingQueries, output, docBatchSize, SimpleQueryMatchWrapper::fromQueryMatch);
}

static void highlightEncode(
MultiMatchingQueries<HighlightsMatch> matchingQueries,
Map<String, Object> output,
int docBatchSize) {
encodeMultiMatchingQuery(
matchingQueries, output, docBatchSize, HighlightedQueryMatchWrapper::new);
}

private static <T extends QueryMatch> void encodeMultiMatchingQuery(
MultiMatchingQueries<T> matchingQueries,
Map<String, Object> output,
Expand Down Expand Up @@ -217,63 +199,4 @@ public BytesRef queryId() {
return queryId;
}
}

private static class HighlightedQueryMatchWrapper implements QueryMatchWrapper {

private final BytesRef queryId;
private final Map<Object, Object> hits;

private HighlightedQueryMatchWrapper(HighlightsMatch highlightsMatch) {
this(
new BytesRef(highlightsMatch.getQueryId()),
highlightsMatch.getHits().entrySet().stream()
.collect(
Collectors.toMap(
Map.Entry::getKey,
entry ->
entry.getValue().stream()
.map(HighlightedQueryMatchWrapper::hitMap)
.collect(Collectors.toList()))));
}

private static Map<Object, Object> hitMap(HighlightsMatch.Hit hit) {
Map<Object, Object> map = new LinkedHashMap<>();
map.put("startPosition", hit.startPosition);
map.put("endPosition", hit.endPosition);
map.put("startOffset", hit.startOffset);
map.put("endOffset", hit.endOffset);
return map;
}

private HighlightedQueryMatchWrapper(BytesRef queryId, Map<Object, Object> hits) {
this.hits = hits;
this.queryId = queryId;
}

@Override
public Object toSerializableForm() {
Map<Object, Object> map = new LinkedHashMap<>();
map.put(MonitorFields.QUERY_ID, queryId.utf8ToString());
map.put(MonitorConstants.HITS_KEY, hits);
return map;
}

private static HighlightedQueryMatchWrapper fromSerializableForm(Object serializableForm) {
if (!(serializableForm instanceof Map)) {
decodingError(serializableForm, HighlightedQueryMatchWrapper.class);
}
Object queryId = ((Map<?, ?>) serializableForm).get(MonitorFields.QUERY_ID);
Object hits = ((Map<?, ?>) serializableForm).get(MonitorConstants.HITS_KEY);
if (!(queryId instanceof String) || !(hits instanceof Map<?, ?>)) {
decodingError(serializableForm, HighlightedQueryMatchWrapper.class);
}
return new HighlightedQueryMatchWrapper(
new BytesRef((String) queryId), (Map<Object, Object>) hits);
}

@Override
public BytesRef queryId() {
return queryId;
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@

import static org.apache.solr.monitor.MonitorConstants.MONITOR_DOCUMENTS_KEY;
import static org.apache.solr.monitor.MonitorConstants.MONITOR_OUTPUT_KEY;
import static org.apache.solr.monitor.MonitorConstants.QUERY_MATCH_TYPE_KEY;
import static org.apache.solr.monitor.MonitorConstants.WRITE_TO_DOC_LIST_KEY;
import static org.apache.solr.monitor.search.PresearcherFactory.DEFAULT_ALIAS_PREFIX;
import static org.apache.solr.monitor.search.QueryMatchResponseCodec.mergeResponses;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -91,9 +89,8 @@ public void prepare(ResponseBuilder rb) {
var req = rb.req;
var documentBatch = documentBatch(req);
boolean writeToDocList = req.getParams().getBool(WRITE_TO_DOC_LIST_KEY, false);
var matchType = QueryMatchType.fromString(req.getParams().get(QUERY_MATCH_TYPE_KEY));
Map<String, Object> monitorResult = new HashMap<>();
var matcherSink = solrMatcherSinkFactory.build(matchType, documentBatch, monitorResult);
var matcherSink = solrMatcherSinkFactory.build(documentBatch, monitorResult);
Query preFilterQuery = presearcher.buildQuery(documentBatch.get(), getTermAcceptor(rb.req));
List<Query> mutableFilters =
Optional.ofNullable(rb.getFilters()).map(ArrayList::new).orElseGet(ArrayList::new);
Expand All @@ -106,12 +103,9 @@ public void prepare(ResponseBuilder rb) {
mutableFilters.add(
new MonitorPostFilter(
new SolrMonitorQueryCollector.CollectorContext(
solrMonitorCache, queryDecoder, matcherSink, writeToDocList, matchType)));
solrMonitorCache, queryDecoder, matcherSink, writeToDocList)));
rb.setFilters(mutableFilters);
rb.rsp.add(QUERY_MATCH_TYPE_KEY, matchType.name());
if (matchType != QueryMatchType.NONE) {
rb.rsp.add(MONITOR_OUTPUT_KEY, monitorResult);
}
rb.rsp.add(MONITOR_OUTPUT_KEY, monitorResult);
}

@SuppressWarnings({"unchecked"})
Expand Down Expand Up @@ -149,11 +143,8 @@ public void handleResponses(ResponseBuilder rb, ShardRequest sreq) {
.map(shardResponse -> shardResponse.getSolrResponse().getResponse())
.collect(Collectors.toList());
rb.rsp.getValues().removeAll(MONITOR_OUTPUT_KEY);
var matchType = QueryMatchType.fromString(rb.req.getParams().get(QUERY_MATCH_TYPE_KEY));
if (matchType != QueryMatchType.NONE) {
var finalOutput = mergeResponses(responses, matchType);
rb.rsp.add(MONITOR_OUTPUT_KEY, finalOutput);
}
var finalOutput = QueryMatchResponseCodec.mergeResponses(responses);
rb.rsp.add(MONITOR_OUTPUT_KEY, finalOutput);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.function.Function;
import org.apache.lucene.monitor.CandidateMatcher;
import org.apache.lucene.monitor.DocumentBatchVisitor;
import org.apache.lucene.monitor.HighlightsMatch;
import org.apache.lucene.monitor.MultiMatchingQueries;
import org.apache.lucene.monitor.QueryMatch;
import org.apache.lucene.search.IndexSearcher;
Expand All @@ -33,25 +32,12 @@ class SolrMatcherSinkFactory {

SolrMatcherSinkFactory() {}

SolrMatcherSink build(
QueryMatchType matchType,
DocumentBatchVisitor documentBatch,
Map<String, Object> monitorResult) {
if (matchType == QueryMatchType.SIMPLE) {
return buildSimple(
documentBatch,
matchingQueries ->
QueryMatchResponseCodec.simpleEncode(
matchingQueries, monitorResult, documentBatch.size()));
} else if (matchType == QueryMatchType.HIGHLIGHTS) {
return build(
documentBatch,
HighlightsMatch.MATCHER::createMatcher,
matchingQueries ->
QueryMatchResponseCodec.highlightEncode(
matchingQueries, monitorResult, documentBatch.size()));
}
return buildSimple(documentBatch, __ -> {});
SolrMatcherSink build(DocumentBatchVisitor documentBatch, Map<String, Object> monitorResult) {
return buildSimple(
documentBatch,
matchingQueries ->
QueryMatchResponseCodec.simpleEncode(
matchingQueries, monitorResult, documentBatch.size()));
}

private SolrMatcherSink buildSimple(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.io.IOException;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.monitor.QCEVisitor;
import org.apache.lucene.search.ConstantScoreQuery;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops this should have been put back with 483e46f

import org.apache.lucene.search.ScoreMode;
import org.apache.solr.monitor.MonitorDataValues;
import org.apache.solr.monitor.SolrMonitorQueryDecoder;
Expand All @@ -36,14 +35,12 @@ class SolrMonitorQueryCollector extends DelegatingCollector {
private final SolrMatcherSink matcherSink;
private final MonitorDataValues dataValues = new MonitorDataValues();
private final boolean writeToDocList;
private final QueryMatchType queryMatchType;

SolrMonitorQueryCollector(CollectorContext collectorContext) {
this.monitorQueryCache = collectorContext.queryCache;
this.queryDecoder = collectorContext.queryDecoder;
this.matcherSink = collectorContext.solrMatcherSink;
this.writeToDocList = collectorContext.writeToDocList;
this.queryMatchType = collectorContext.queryMatchType;
}

@Override
Expand All @@ -52,10 +49,9 @@ public void collect(int doc) throws IOException {
var entry = getEntry(dataValues);
var queryId = dataValues.getQueryId();
var originalMatchQuery = entry.getMatchQuery();
var matchQuery =
queryMatchType.needsScores
? originalMatchQuery
: new ConstantScoreQuery(originalMatchQuery);

var matchQuery = new ConstantScoreQuery(originalMatchQuery);

boolean isMatch = matcherSink.matchQuery(queryId, matchQuery, entry.getMetadata());
if (isMatch && writeToDocList) {
super.collect(doc);
Expand Down Expand Up @@ -95,19 +91,16 @@ static class CollectorContext {
private final SolrMonitorQueryDecoder queryDecoder;
private final SolrMatcherSink solrMatcherSink;
private final boolean writeToDocList;
private final QueryMatchType queryMatchType;

CollectorContext(
MonitorQueryCache queryCache,
SolrMonitorQueryDecoder queryDecoder,
SolrMatcherSink solrMatcherSink,
boolean writeToDocList,
QueryMatchType queryMatchType) {
boolean writeToDocList) {
this.queryCache = queryCache;
this.queryDecoder = queryDecoder;
this.solrMatcherSink = solrMatcherSink;
this.writeToDocList = writeToDocList;
this.queryMatchType = queryMatchType;
}
}
}
Loading