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

Conversation

cpoerschke
Copy link
Collaborator

motivation is to reduce the scope and complexity of the initial integration.

defer as opposed to remove i.e. looking at https://lucene.apache.org/core/9_11_0/monitor/org/apache/lucene/monitor/QueryMatch.html documentation there's not only HighlightsMatch (deferred here) but also ExplainingMatch and ScoringMatch (both not yet on the monitorMatchType list)

@kotman12
Copy link
Owner

kotman12 commented Jun 24, 2024

motivation is to reduce the scope and complexity of the initial integration.

defer as opposed to remove i.e. looking at https://lucene.apache.org/core/9_11_0/monitor/org/apache/lucene/monitor/QueryMatch.html documentation there's not only HighlightsMatch (deferred here) but also ExplainingMatch and ScoringMatch (both not yet on the monitorMatchType list)

For context, simple matching returns the matching query Id list. Now that we may have removed the parallel matcher (depending on the viability of segment parallelization) there is possibly no benefit to this response type. I kept it around before because it let you get the simple query Id list response without a doc list response object (which was hard to populate asynchronously). But I think the doc list response is the correct approach because users are already familiar with it. It seamlessly integrates with other solr features like column fetching and possibly streaming. Moreover, a simple monitor response always has a numFound=0 which is confusing. This could be documented but it feels like a weird API from the start and now I can't even justify it anymore.

I'm still thinking about how features like highlights or explaining match could fit into this (in a later iteration) ..

Co-authored-by: Christine Poerschke <[email protected]>
@@ -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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants