From 0537fc5024145cf383f9c76039cd26d739df8462 Mon Sep 17 00:00:00 2001 From: Olli Virtanen Date: Tue, 2 Apr 2019 11:47:20 +0200 Subject: Use sort data with java dispatcher --- .../src/main/java/com/yahoo/fs4/DocumentInfo.java | 11 ++++ .../src/main/java/com/yahoo/fs4/QueryPacket.java | 14 +---- .../main/java/com/yahoo/fs4/QueryResultPacket.java | 34 +++++++++--- .../yahoo/prelude/fastsearch/FS4SearchInvoker.java | 7 +-- .../java/com/yahoo/prelude/fastsearch/FastHit.java | 32 +++++++++++ .../com/yahoo/prelude/fastsearch/FastSearcher.java | 4 +- .../prelude/fastsearch/SortDataHitSorter.java | 64 ++++++++++++++++++++++ .../prelude/fastsearch/VespaBackEndSearcher.java | 21 +++---- .../java/com/yahoo/search/dispatch/Dispatcher.java | 4 +- .../search/dispatch/InterleavedSearchInvoker.java | 16 ++---- .../com/yahoo/search/dispatch/InvokerFactory.java | 2 +- .../yahoo/search/dispatch/SearchErrorInvoker.java | 3 +- .../com/yahoo/search/dispatch/SearchInvoker.java | 7 +-- .../search/dispatch/rpc/ProtobufSerialization.java | 3 +- .../search/dispatch/rpc/RpcSearchInvoker.java | 3 +- .../java/com/yahoo/search/result/HitGroup.java | 45 +++++++++++---- .../streamingvisitors/VdsStreamingSearcher.java | 17 +++--- .../java/com/yahoo/fs4/test/QueryTestCase.java | 1 - .../prelude/cluster/ClusterSearcherTestCase.java | 2 +- .../fastsearch/FS4SearchInvokerTestCase.java | 7 +-- .../fastsearch/test/PartialFillTestCase.java | 5 +- .../dispatch/InterleavedSearchInvokerTest.java | 18 +++--- .../com/yahoo/search/dispatch/MockInvoker.java | 3 +- .../search/dispatch/rpc/RpcSearchInvokerTest.java | 5 +- .../VdsStreamingSearcherTestCase.java | 3 +- 25 files changed, 226 insertions(+), 105 deletions(-) create mode 100644 container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java b/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java index e5ab00fb139..dd5a4fff584 100644 --- a/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java +++ b/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java @@ -17,14 +17,20 @@ public class DocumentInfo implements Cloneable { private final double metric; private final int partId; private final int distributionKey; + private final byte[] sortData; DocumentInfo(ByteBuffer buffer, QueryResultPacket owner) { + this(buffer, owner, null); + } + + DocumentInfo(ByteBuffer buffer, QueryResultPacket owner, byte[] sortData) { byte[] rawGid = new byte[GlobalId.LENGTH]; buffer.get(rawGid); globalId = new GlobalId(rawGid); metric = decodeMetric(buffer); partId = owner.getMldFeature() ? buffer.getInt() : 0; distributionKey = owner.getMldFeature() ? buffer.getInt() : 0; + this.sortData = sortData; } public DocumentInfo(GlobalId globalId, int metric, int partId, int distributionKey) { @@ -32,6 +38,7 @@ public class DocumentInfo implements Cloneable { this.metric = metric; this.partId = partId; this.distributionKey = distributionKey; + this.sortData = null; } private double decodeMetric(ByteBuffer buffer) { @@ -49,6 +56,10 @@ public class DocumentInfo implements Cloneable { /** Unique key for the node this document resides on */ public int getDistributionKey() { return distributionKey; } + public byte[] getSortData() { + return sortData; + } + public String toString() { return "document info [globalId=" + globalId + ", metric=" + metric + "]"; } diff --git a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java index 60bd5314cff..d1d8b9a0f77 100644 --- a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java +++ b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java @@ -5,6 +5,7 @@ import com.yahoo.compress.IntegerCompressor; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.prelude.query.Item; import com.yahoo.search.Query; +import com.yahoo.search.dispatch.Dispatcher; import com.yahoo.search.grouping.vespa.GroupingExecutor; import com.yahoo.search.query.Ranking; import com.yahoo.searchlib.aggregation.Grouping; @@ -176,18 +177,7 @@ public class QueryPacket extends Packet { private int getFlagInt() { int flags = getQueryFlags(query); queryPacketData.setQueryFlags(flags); - - /* - * QFLAG_DROP_SORTDATA - * SORTDATA is a mangling of data from the attribute vectors - * which were used in the search which is byte comparable in - * such a way the comparing SORTDATA for two different hits - * will reproduce the order in which the data were returned when - * using sortspec. For now we simply drop these. If they - * become necessary, QueryResultPacket must be - * updated to be able to read the sort data. - */ - flags |= QFLAG_DROP_SORTDATA; + flags |= query.properties().getBoolean(Dispatcher.dispatchInternal, false) ? 0 : QFLAG_DROP_SORTDATA; return flags; } diff --git a/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java b/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java index 239101184ea..6a27beefb5e 100644 --- a/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java +++ b/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java @@ -7,7 +7,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; - /** * A query result packet (code 217). This packet can be decoded only. * @@ -108,10 +107,9 @@ public class QueryResultPacket extends Packet { nodesReplied = buffer.getShort(); } - if (sortData && documentCount > 0) { // sort data is not needed - skip - buffer.position(buffer.position() + (documentCount -1) * 4); // one sortIndex int per document - int sortDataLengthInBytes = buffer.getInt(); - buffer.position(buffer.position() + sortDataLengthInBytes); + byte[][] documentSortData = null; + if (sortData && documentCount > 0) { + documentSortData = decodeSortData(buffer, documentCount); } if (groupDataFeature) { @@ -125,7 +123,7 @@ public class QueryResultPacket extends Packet { soonActiveDocs = buffer.getLong(); degradedReason = buffer.getInt(); - decodeDocuments(buffer, documentCount); + decodeDocuments(buffer, documentCount, documentSortData); if (propsFeature) { int numMaps = buffer.getInt(); propsArray = new FS4Properties[numMaps]; @@ -136,6 +134,25 @@ public class QueryResultPacket extends Packet { } } + private byte[][] decodeSortData(ByteBuffer buffer, int documentCount) { + int[] indexes = new int[documentCount]; + indexes[0] = 0; + for (int i = 1; i < documentCount; i++) { + indexes[i] = buffer.getInt(); + } + + int sortDataLengthInBytes = buffer.getInt(); + byte[][] ret = new byte[indexes.length][]; + + for (int i = 0; i < indexes.length; i++) { + int end = i + 1 >= indexes.length ? sortDataLengthInBytes : indexes[i + 1]; + int len = end - indexes[i]; + ret[i] = new byte[len]; + buffer.get(ret[i], 0, len); + } + return ret; + } + private Number decodeMaxRank(ByteBuffer buffer) { return Double.valueOf(buffer.getDouble()); } @@ -161,9 +178,10 @@ public class QueryResultPacket extends Packet { propsFeature = (QRF_PROPERTIES & features) != 0; } - private void decodeDocuments(ByteBuffer buffer, int documentCount) { + private void decodeDocuments(ByteBuffer buffer, int documentCount, byte[][] documentSortData) { for (int i = 0; i < documentCount; i++) { - documents.add(new DocumentInfo(buffer, this)); + byte[] sort = documentSortData == null ? null : documentSortData[i]; + documents.add(new DocumentInfo(buffer, this, sort)); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java index 24653db5671..0cf6a8a8449 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java @@ -44,13 +44,10 @@ public class FS4SearchInvoker extends SearchInvoker implements ResponseMonitor= max) { + return left.sortData.length - right.sortData.length; + } + int vl = (int) left.sortData[i] & 0xFF; + int vr = (int) right.sortData[i] & 0xFF; + int diff = vl - vr; + return diff; + } + /** For internal use */ public void addSummary(DocsumDefinition docsumDef, Inspector value) { if (removedFields != null) diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java index e61ed1715b6..5f6d4220dd4 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java @@ -142,11 +142,11 @@ public class FastSearcher extends VespaBackEndSearcher { } @Override - public Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + public Result doSearch2(Query query, Execution execution) { if (dispatcher.searchCluster().groupSize() == 1) forceSinglePassGrouping(query); try(SearchInvoker invoker = getSearchInvoker(query)) { - Result result = invoker.search(query, queryPacket, execution); + Result result = invoker.search(query, execution); if (query.properties().getBoolean(Ranking.RANKFEATURES, false)) { // There is currently no correct choice for which diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java new file mode 100644 index 00000000000..e87c94fa3be --- /dev/null +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java @@ -0,0 +1,64 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.prelude.fastsearch; + +import com.yahoo.search.query.Sorting; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; + +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +public class SortDataHitSorter { + public static void sort(HitGroup hitGroup, List hits) { + var sorting = hitGroup.getQuery().getRanking().getSorting(); + var fallbackOrderer = hitGroup.getOrderer(); + if (sorting == null || fallbackOrderer == null) { + return; + } + var fallbackComparator = fallbackOrderer.getComparator(); + Collections.sort(hits, getComparator(sorting, fallbackComparator)); + } + + public static boolean isSortable(Hit hit, HitGroup hitGroup) { + if (hitGroup.getQuery() == null) { + return false; + } + if (hit instanceof FastHit) { + var fhit = (FastHit) hit; + return fhit.hasSortData(hitGroup.getQuery().getRanking().getSorting()); + } else { + return false; + } + } + + public static Comparator getComparator(Sorting sorting, Comparator fallback) { + if (fallback == null) { + return (left, right) -> compareTwo(left, right, sorting); + } else { + return (left, right) -> compareWithFallback(left, right, sorting, fallback); + } + } + + private static int compareTwo(Hit left, Hit right, Sorting sorting) { + if (left == null || right == null || !(left instanceof FastHit) || !(right instanceof FastHit)) { + return 0; + } + FastHit fl = (FastHit) left; + FastHit fr = (FastHit) right; + return FastHit.compareSortData(fl, fr, sorting); + } + + private static int compareWithFallback(Hit left, Hit right, Sorting sorting, Comparator fallback) { + if (left == null || right == null || !(left instanceof FastHit) || !(right instanceof FastHit)) { + return fallback.compare(left, right); + } + FastHit fl = (FastHit) left; + FastHit fr = (FastHit) right; + if (fl.hasSortData(sorting) && fr.hasSortData(sorting)) { + return FastHit.compareSortData(fl, fr, sorting); + } else { + return fallback.compare(left, right); + } + } +} diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java index df72720a46c..f5e437c2410 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java @@ -24,6 +24,7 @@ import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.cluster.PingableSearcher; import com.yahoo.search.grouping.vespa.GroupingExecutor; +import com.yahoo.search.query.Sorting; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorHit; import com.yahoo.search.result.ErrorMessage; @@ -88,10 +89,9 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { * This is an endpoint - searchers will never propagate the search to any nested searcher. * * @param query the query to search - * @param queryPacket the serialized query representation to pass to the search cluster * @param execution the query execution context */ - protected abstract Result doSearch2(Query query, QueryPacket queryPacket, Execution execution); + protected abstract Result doSearch2(Query query, Execution execution); protected abstract void doPartialFill(Result result, String summaryClass); @@ -184,15 +184,10 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { if (root == null || root instanceof NullItem) // root can become null after resolving and transformation? return new Result(query); - QueryPacket queryPacket = createQueryPacket(serverId, query); - - if (isLoggingFine()) - getLogger().fine("made QueryPacket: " + queryPacket); - Result result = null; if (result == null) { - result = doSearch2(query, queryPacket, execution); + result = doSearch2(query, execution); if (isLoggingFine()) getLogger().fine("Result NOT retrieved from cache"); @@ -209,6 +204,10 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { queryPacket.setCompressionLimit(compressionLimit); if (compressionLimit != 0) queryPacket.setCompressionType(query.properties().getString(PACKET_COMPRESSION_TYPE, "lz4")); + + if (isLoggingFine()) + getLogger().fine("made QueryPacket: " + queryPacket); + return queryPacket; } @@ -452,7 +451,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { return new FillHitsResult(skippedHits, lastError); } - private void extractDocumentInfo(FastHit hit, DocumentInfo document) { + private void extractDocumentInfo(FastHit hit, DocumentInfo document, Sorting sorting) { hit.setSource(getName()); Number rank = document.getMetric(); @@ -462,6 +461,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { hit.setDistributionKey(document.getDistributionKey()); hit.setGlobalId(document.getGlobalId()); hit.setPartId(document.getPartId()); + hit.setSortData(document.getSortData(), sorting); } protected DocsumDefinitionSet getDocsumDefinitionSet(Query query) { @@ -498,6 +498,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { QueryPacketData queryPacketData, Optional channelDistributionKey) { Query myQuery = result.getQuery(); + Sorting sorting = myQuery.getRanking().getSorting(); for (DocumentInfo document : documents) { @@ -510,7 +511,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { hit.setFillable(); hit.setCached(false); - extractDocumentInfo(hit, document); + extractDocumentInfo(hit, document, sorting); channelDistributionKey.ifPresent(hit::setDistributionKey); result.hits().add(hit); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index f2dbb1e8557..4f546201e76 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -45,7 +45,7 @@ public class Dispatcher extends AbstractComponent { private static final int MAX_GROUP_SELECTION_ATTEMPTS = 3; /** If enabled, this internal dispatcher will be preferred over fdispatch whenever possible */ - private static final CompoundName dispatchInternal = new CompoundName("dispatch.internal"); + public static final CompoundName dispatchInternal = new CompoundName("dispatch.internal"); /** If enabled, search queries will use protobuf rpc */ public static final CompoundName dispatchProtobuf = new CompoundName("dispatch.protobuf"); @@ -135,6 +135,8 @@ public class Dispatcher extends AbstractComponent { query.setOffset(0); } emitDispatchMetric(invoker); + query.properties().set(dispatchInternal, true); + return invoker; } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java index b8c9bb205e9..457e587da40 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java @@ -1,8 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; -import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.SearchCluster; @@ -37,7 +35,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private static final Logger log = Logger.getLogger(InterleavedSearchInvoker.class.getName()); private final Set invokers; - private final VespaBackEndSearcher searcher; private final SearchCluster searchCluster; private final LinkedBlockingQueue availableForProcessing; private final Set alreadyFailedNodes; @@ -49,6 +46,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private long deadline = 0; private Result result = null; + private long answeredDocs = 0; private long answeredActiveDocs = 0; private long answeredSoonActiveDocs = 0; @@ -60,11 +58,10 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private boolean trimResult = false; - public InterleavedSearchInvoker(Collection invokers, VespaBackEndSearcher searcher, SearchCluster searchCluster, Set alreadyFailedNodes) { + public InterleavedSearchInvoker(Collection invokers, SearchCluster searchCluster, Set alreadyFailedNodes) { super(Optional.empty()); this.invokers = Collections.newSetFromMap(new IdentityHashMap<>()); this.invokers.addAll(invokers); - this.searcher = searcher; this.searchCluster = searchCluster; this.availableForProcessing = newQueue(); this.alreadyFailedNodes = alreadyFailedNodes; @@ -76,7 +73,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM * will be adjusted accordingly. */ @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; invokers.forEach(invoker -> invoker.setMonitor(this)); deadline = currentTime() + query.getTimeLeft(); @@ -88,7 +85,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM trimResult = originalHits != query.getHits() || originalOffset != query.getOffset(); for (SearchInvoker invoker : invokers) { - invoker.sendSearchRequest(query, null); + invoker.sendSearchRequest(query); askedNodes++; } @@ -128,10 +125,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private void trimResult(Execution execution) { if (trimResult) { - if (result.getHitOrderer() != null) { - searcher.fill(result, Execution.ATTRIBUTEPREFETCH, execution); - } - result.hits().trim(query.getOffset(), query.getHits()); } } @@ -209,7 +202,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM result = partialResult; return; } - result.mergeWith(partialResult); result.hits().addAll(partialResult.hits().asUnorderedHits()); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java index 8617c74ec41..0ce1b74c02d 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java @@ -93,7 +93,7 @@ public abstract class InvokerFactory { if (invokers.size() == 1 && failed == null) { return Optional.of(invokers.get(0)); } else { - return Optional.of(new InterleavedSearchInvoker(invokers, searcher, searchCluster, failed)); + return Optional.of(new InterleavedSearchInvoker(invokers, searchCluster, failed)); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java index 4dfdd65714d..b1d25dddbb8 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.result.Coverage; @@ -35,7 +34,7 @@ public class SearchErrorInvoker extends SearchInvoker { } @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; if(monitor != null) { monitor.responseAvailable(this); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java index 1650494db3a..3bfb9089457 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.Node; @@ -31,14 +30,14 @@ public abstract class SearchInvoker extends CloseableInvoker { * nodes, the provided {@link Execution} may be used to retrieve document summaries required * for correct result windowing. */ - public Result search(Query query, QueryPacket queryPacket, Execution execution) throws IOException { - sendSearchRequest(query, queryPacket); + public Result search(Query query, Execution execution) throws IOException { + sendSearchRequest(query); Result result = getSearchResult(execution); setFinalStatus(result.hits().getError() == null); return result; } - protected abstract void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException; + protected abstract void sendSearchRequest(Query query) throws IOException; protected abstract Result getSearchResult(Execution execution) throws IOException; diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java index 9903aacdda0..5a8102ff5c7 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java @@ -181,13 +181,14 @@ public class ProtobufSerialization { result.hits().add(hit); } + var sorting = query.getRanking().getSorting(); for (var replyHit : protobuf.getHitsList()) { FastHit hit = new FastHit(); hit.setQuery(query); hit.setRelevance(new Relevance(replyHit.getRelevance())); hit.setGlobalId(new GlobalId(replyHit.getGlobalId().toByteArray())); - + hit.setSortData(replyHit.getSortData().toByteArray(), sorting); hit.setFillable(); hit.setCached(false); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java index e54ac00f821..d70a7d95b63 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java @@ -3,7 +3,6 @@ package com.yahoo.search.dispatch.rpc; import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; -import com.yahoo.fs4.QueryPacket; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; @@ -44,7 +43,7 @@ public class RpcSearchInvoker extends SearchInvoker implements Client.ResponseRe } @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; CompressionType compression = CompressionType diff --git a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java index ca776aef011..fdebe78c20b 100644 --- a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java +++ b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java @@ -6,6 +6,7 @@ import com.google.common.collect.Iterables; import com.google.common.util.concurrent.ListenableFuture; import com.yahoo.collections.ListenableArrayList; import com.yahoo.net.URI; +import com.yahoo.prelude.fastsearch.SortDataHitSorter; import com.yahoo.processing.response.ArrayDataList; import com.yahoo.processing.response.DataList; import com.yahoo.processing.response.DefaultIncomingData; @@ -19,8 +20,6 @@ import java.util.List; import java.util.Set; import java.util.stream.Collectors; -import static com.yahoo.collections.CollectionUtil.first; - /** *

A group of ordered hits. Since hitGroup is itself a kind of Hit, * this can compose hierarchies of grouped hits. @@ -63,6 +62,9 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< /** The current number of concrete (non-meta) hits in the result */ private int concreteHitCount = 0; + /** Number of hits known to have sort data */ + private int hitsWithSortData = 0; + /** The class used to determine the ordering of the hits of this */ transient private HitOrderer hitOrderer = null; @@ -398,7 +400,7 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< return errorHit; } - /** + /** * Removes the error hit of this. * This removes all error messages of this and the query producing it. * @@ -412,7 +414,7 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< errorHit = null; return removed; } - + /** * Returns the first error in this result, * or null if no searcher has produced an error AND the query doesn't contain an error @@ -451,9 +453,9 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< add(queryErrors); } - /** + /** * Consumes errors from the query and returns them in a new error hit - * + * * @return the error hit containing all query errors, or null if no query errors should be consumed */ private DefaultErrorHit consumeAnyQueryErrors() { @@ -469,9 +471,9 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< /** Compatibility */ private ErrorMessage toSearchError(com.yahoo.processing.request.ErrorMessage error) { - if (error instanceof ErrorMessage) + if (error instanceof ErrorMessage) return (ErrorMessage)error; - else + else return new ErrorMessage(error.getCode(), error.getMessage(), error.getDetailedMessage(), error.getCause()); } @@ -569,6 +571,9 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< if (hitOrderer == null) { Collections.sort(hits); hitsSorted = true; + } else if (sortableWithSortData()) { + SortDataHitSorter.sort(this, hits); + hitsSorted = true; } else { // This may or may not lead to a sorted result set, but it's a best effort hitOrderer.order(hits); @@ -578,6 +583,10 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< } } + private boolean sortableWithSortData() { + return hitsWithSortData > 0 && hitsWithSortData == concreteHitCount; + } + private boolean likelyHitsHaveCorrectValueForSortFields() { if (hitOrderer == null) { return true; @@ -620,7 +629,7 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< /** Called before hit lists or positions are used */ private void ensureSorted() { - if ( ! orderedHits && ! hitsSorted && likelyHitsHaveCorrectValueForSortFields()) { + if ( ! orderedHits && ! hitsSorted && (sortableWithSortData() || likelyHitsHaveCorrectValueForSortFields())) { sort(); } } @@ -678,6 +687,10 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< hit.setAddNumber(size()); } + if (SortDataHitSorter.isSortable(hit, this)) { + hitsWithSortData++; + } + hitsSorted = false; Set hitFilled = hit.getFilled(); @@ -726,6 +739,10 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< errorHit = null; } + if (SortDataHitSorter.isSortable(hit, this)) { + hitsWithSortData--; + } + if (deletionBreaksOrdering) { hitsSorted = false; } @@ -740,6 +757,10 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< if (!hit.isCached()) notCachedCount++; + + if (SortDataHitSorter.isSortable(hit, this)) { + hitsWithSortData++; + } } /** @@ -747,9 +768,11 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< * Recursively also update all subgroups. */ public void analyze() { - concreteHitCount=0; + concreteHitCount = 0; setFilledInternal(null); - notCachedCount=0; + notCachedCount = 0; + hitsWithSortData = 0; + Set filled = getFilledInternal(); Iterator i = unorderedIterator(); diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java index e41a582ea13..0fd3d5b5aa1 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java @@ -1,19 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.streamingvisitors; -import java.io.IOException; -import java.math.BigInteger; -import java.util.List; -import java.util.Map; -import java.util.logging.Logger; - import com.yahoo.document.DocumentId; import com.yahoo.document.idstring.IdString; import com.yahoo.document.select.parser.ParseException; import com.yahoo.document.select.parser.TokenMgrException; import com.yahoo.fs4.DocsumPacket; import com.yahoo.fs4.Packet; -import com.yahoo.fs4.QueryPacket; import com.yahoo.log.LogLevel; import com.yahoo.messagebus.routing.Route; import com.yahoo.prelude.Ping; @@ -22,9 +15,9 @@ import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.GroupingListHit; import com.yahoo.prelude.fastsearch.TimeoutException; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; +import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; -import com.yahoo.processing.request.CompoundName; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.result.Relevance; @@ -34,6 +27,12 @@ import com.yahoo.vdslib.DocumentSummary; import com.yahoo.vdslib.SearchResult; import com.yahoo.vdslib.VisitorStatistics; +import java.io.IOException; +import java.math.BigInteger; +import java.util.List; +import java.util.Map; +import java.util.logging.Logger; + /** * The searcher which forwards queries to storage nodes using visiting. * The searcher is a visitor client responsible for starting search @@ -92,7 +91,7 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { } @Override - public Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + public Result doSearch2(Query query, Execution execution) { // TODO refactor this method into smaller methods, it's hard to see the actual code lazyTrace(query, 7, "Routing to storage cluster ", getStorageClusterRouteSpec()); diff --git a/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java index 911831e3a65..161ea4bdf64 100644 --- a/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/fs4/test/QueryTestCase.java @@ -310,5 +310,4 @@ public class QueryTestCase { } public static final byte ignored = -128; - } diff --git a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java index 1e19a1397e0..84f986d5371 100644 --- a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java @@ -201,7 +201,7 @@ public class ClusterSearcherTestCase { } @Override - protected com.yahoo.search.Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected com.yahoo.search.Result doSearch2(Query query, Execution execution) { return null; // search() is overriden, this should never be called } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java index 8fd3bc1f3eb..b9119528490 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java @@ -3,7 +3,6 @@ package com.yahoo.prelude.fastsearch; import com.yahoo.fs4.BasicPacket; -import com.yahoo.fs4.QueryPacket; import com.yahoo.fs4.mplex.FS4Channel; import com.yahoo.fs4.mplex.InvalidChannelException; import com.yahoo.search.Query; @@ -31,10 +30,10 @@ public class FS4SearchInvokerTestCase { var searcher = mockSearcher(); var cluster = new MockSearchCluster("?", 1, 1); var fs4invoker = new FS4SearchInvoker(searcher, query, mockFailingChannel(), Optional.empty()); - var interleave = new InterleavedSearchInvoker(Collections.singleton(fs4invoker), searcher, cluster, null); + var interleave = new InterleavedSearchInvoker(Collections.singleton(fs4invoker), cluster, null); long start = System.currentTimeMillis(); - interleave.search(query, QueryPacket.create(null, null), null); + interleave.search(query, null); long elapsed = System.currentTimeMillis() - start; assertThat("Connection error should fail fast", elapsed, Matchers.lessThan(500L)); @@ -43,7 +42,7 @@ public class FS4SearchInvokerTestCase { private static VespaBackEndSearcher mockSearcher() { return new VespaBackEndSearcher() { @Override - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { return null; } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java index ed0441d8450..cb43f2e17e3 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java @@ -3,7 +3,6 @@ package com.yahoo.prelude.fastsearch.test; import com.google.common.util.concurrent.MoreExecutors; import com.yahoo.component.chain.Chain; -import com.yahoo.fs4.QueryPacket; import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; @@ -32,7 +31,7 @@ public class PartialFillTestCase { public static class FS4 extends VespaBackEndSearcher { public List history = new ArrayList<>(); - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { return new Result(query); } protected void doPartialFill(Result result, String summaryClass) { @@ -41,7 +40,7 @@ public class PartialFillTestCase { } public static class BadFS4 extends VespaBackEndSearcher { - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { return new Result(query); } protected void doPartialFill(Result result, String summaryClass) { diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java index f84f35020d2..8686ddf229b 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java @@ -49,7 +49,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(4900, 100, 1)); expectedEvents.add(new Event(4800, 100, 2)); - invoker.search(query, null, null); + invoker.search(query, null); assertTrue("All test scenario events processed", expectedEvents.isEmpty()); } @@ -63,7 +63,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(4700, 300, 1)); expectedEvents.add(null); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); assertTrue("All test scenario events processed", expectedEvents.isEmpty()); assertNull("Result is not marked as an error", result.hits().getErrorHit()); @@ -82,7 +82,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(2400, 100, 2)); expectedEvents.add(new Event(0, 0, null)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); assertTrue("All test scenario events processed", expectedEvents.isEmpty()); assertNull("Result is not marked as an error", result.hits().getErrorHit()); @@ -101,7 +101,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(100000L)); @@ -122,7 +122,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(23420L)); @@ -144,7 +144,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(9900L)); @@ -166,7 +166,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(null); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(50155L)); @@ -191,7 +191,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 1, 1)); expectedEvents.add(new Event(null, 100, 0)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(50155L)); @@ -209,7 +209,7 @@ public class InterleavedSearchInvokerTest { invokers.add(new MockInvoker(i)); } - return new InterleavedSearchInvoker(invokers, null, searchCluster, null) { + return new InterleavedSearchInvoker(invokers, searchCluster, null) { @Override protected long currentTime() { return clock.millis(); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java index 00b1edec8b3..e347a884c17 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java @@ -1,7 +1,6 @@ // Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.Node; @@ -25,7 +24,7 @@ class MockInvoker extends SearchInvoker { } @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java index b9d894f9eb5..64863b9a8a6 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java @@ -5,7 +5,6 @@ package com.yahoo.search.dispatch.rpc; import ai.vespa.searchlib.searchprotocol.protobuf.SearchProtocol; import com.google.common.collect.ImmutableMap; import com.yahoo.compress.CompressionType; -import com.yahoo.fs4.QueryPacket; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; @@ -40,7 +39,7 @@ public class RpcSearchInvokerTest { var invoker = new RpcSearchInvoker(mockSearcher(), new Node(7, "seven", 77, 1), mockPool); Query q = new Query("search/?query=test&hits=10&offset=3"); - invoker.sendSearchRequest(q, null); + invoker.sendSearchRequest(q); var bytes = mockPool.compressor().decompress(payloadHolder.get(), compressionTypeHolder.get(), lengthHolder.get()); var request = SearchProtocol.SearchRequest.newBuilder().mergeFrom(bytes).build(); @@ -78,7 +77,7 @@ public class RpcSearchInvokerTest { private VespaBackEndSearcher mockSearcher() { return new VespaBackEndSearcher() { @Override - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { fail("Unexpected call"); return null; } diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java index f4be9bb543b..b656a509683 100644 --- a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java @@ -136,9 +136,8 @@ public class VdsStreamingSearcherTestCase { } private static Result executeQuery(VdsStreamingSearcher searcher, Query query) { - QueryPacket queryPacket = QueryPacket.create("container.0", query); Execution execution = new Execution(new Execution.Context(null, null, null, null, null)); - return searcher.doSearch2(query, queryPacket, execution); + return searcher.doSearch2(query, execution); } private static Query[] generateTestQueries(String queryString) { -- cgit v1.2.3