Skip to content

[exploratory] abstract QueryComponent.mergeIds' ShardDoc processing #3565

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.

Sign up for GitHub

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
+54 −6
Draft

[exploratory] abstract QueryComponent.mergeIds' ShardDoc processing #3565

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4a26aa
abstract QueryComponent.mergeIds' ShardDoc processing
cpoerschke Aug 29, 2025
7fe997c
add missing @Override annotations
cpoerschke Aug 29, 2025
bcd1c3b
make DefaultShardDocQueue an anonymous class
cpoerschke Sep 1, 2025
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
60 changes: 54 additions & 6 deletions solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiFunction;
import java.util.stream.Collectors;
import org.apache.lucene.index.ExitableDirectoryReader;
import org.apache.lucene.index.IndexReaderContext;
Expand Down Expand Up @@ -905,7 +906,55 @@ protected boolean addFL(StringBuilder fl, String field, boolean additionalAdded)
return true;
}

protected abstract static class ShardDocQueue {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you do an interface, you could have ShardFieldSortedHitQueue implement that interface. Then, DefaultShardDocQueue could be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that idea, thanks for the suggestion.

Though on trying it out, then it didn't quite work out since ShardFieldSortedHitQueue would need to implement the push as either add or insertWithOverflow and making that choice doesn't seem particularly intuitive: https://github.com/apache/lucene/blob/releases/lucene/10.2.2/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java#L150-L184

However, making DefaultShardDocQueue into an anonymous class seems a good alternative, both locally for QueryComponent and if a deriving class needed to maintain a container alongside of ShardFieldSortedHitQueue or instead of it.

public abstract void push(ShardDoc shardDoc);

public abstract ShardDoc pop();

public abstract int size();
}
;

protected static class ShardDocQueueFactory
implements BiFunction<SortField[], Integer, ShardDocQueue> {

private final SolrIndexSearcher searcher;

public ShardDocQueueFactory(SolrIndexSearcher searcher) {
this.searcher = searcher;
}

@Override
public ShardDocQueue apply(SortField[] sortFields, Integer size) {
return new ShardDocQueue() {
private final ShardFieldSortedHitQueue queue =
new ShardFieldSortedHitQueue(sortFields, size, searcher);

@Override
public void push(ShardDoc shardDoc) {
queue.insertWithOverflow(shardDoc);
}

@Override
public ShardDoc pop() {
return queue.pop();
}

@Override
public int size() {
return queue.size();
}
};
}
}
;

protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) {
implementMergeIds(rb, sreq, new ShardDocQueueFactory(rb.req.getSearcher()));
}

private void implementMergeIds(
ResponseBuilder rb, ShardRequest sreq, ShardDocQueueFactory shardDocQueueFactory) {
List<MergeStrategy> mergeStrategies = rb.getMergeStrategies();
if (mergeStrategies != null) {
mergeStrategies.sort(MergeStrategy.MERGE_COMP);
Expand Down Expand Up @@ -952,9 +1001,8 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) {

// Merge the docs via a priority queue so we don't have to sort *all* of the
// documents... we only need to order the top (rows+start)
final ShardFieldSortedHitQueue queue =
new ShardFieldSortedHitQueue(
sortFields, ss.getOffset() + ss.getCount(), rb.req.getSearcher());
final ShardDocQueue shardDocQueue =
shardDocQueueFactory.apply(sortFields, ss.getOffset() + ss.getCount());

NamedList<Object> shardInfo = null;
if (rb.req.getParams().getBool(ShardParams.SHARDS_INFO, false)) {
Expand Down Expand Up @@ -1160,19 +1208,19 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) {

shardDoc.sortFieldValues = unmarshalledSortFieldValues;

queue.insertWithOverflow(shardDoc);
shardDocQueue.push(shardDoc);
} // end for-each-doc-in-response
} // end for-each-response

// The queue now has 0 -> queuesize docs, where queuesize <= start + rows
// So we want to pop the last documents off the queue to get
// the docs offset -> queuesize
int resultSize = queue.size() - ss.getOffset();
int resultSize = shardDocQueue.size() - ss.getOffset();
resultSize = Math.max(0, resultSize); // there may not be any docs in range

Map<Object, ShardDoc> resultIds = new HashMap<>();
for (int i = resultSize - 1; i >= 0; i--) {
ShardDoc shardDoc = queue.pop();
ShardDoc shardDoc = shardDocQueue.pop();
shardDoc.positionInResponse = i;
// Need the toString() for correlation with other lists that must
// be strings (like keys in highlighting, explain, etc)
Expand Down