ES|QL: Score all docs in a single getOutput call in LuceneTopNSourceOperator #136172
Conversation
ioanatia
added
>non-issue
Team:Search - Relevance
elasticsearchmachine
added
Team:Search Relevance
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
There was a problem hiding this comment.
Code looks good to me. One question before merging - could this run for, like, minutes? I imagine in some rare cases with runtime fields and no timestamp query it's possible, though unlikely. But in other cases. Like, in anything approximating a "normal" case?
I ask because we don't have a thing that updates the driver status outside of the main loop. So if this run "forever" then we won't see it in the status. I don't think this is a problem here, but it might be worth a comment.
Mostly I'm having flashbacks to that time when I made an agg run for a week.
|
|
||
| // If we stayed longer than 1 second to execute getOutput, we should return back to the driver, so we can update its status. | ||
| // Even if this should almost never happen, we want to update the driver status even when a query runs "forever". | ||
| if (TimeUnit.SECONDS.convert(System.nanoTime() - start, TimeUnit.NANOSECONDS) >= 1) { |
There was a problem hiding this comment.
I'd yank this into a constant - maybe even one in nanos. Just a little more expected.
There was a problem hiding this comment.
yep - makes sense - guess I was in too much of a hurry - addressed.
There was a problem hiding this comment.
Not too much a of hurry, just a little thing.
The
LuceneTopNOperatorcannot output pages until it collects the top N docs, by scoring all potential matches.LuceneTopNOperatorscores documents in batches - a single call ofrunSingleLoopIterationfromDrivercorresponds to scoring a single batch of documents inLuceneTopNOperatorwithscorer.scoreNextRange(....In
runSingleLoopIterationwe iterate through all the operators if they have a page to output so we can feed it to the next operator. This method is called in a loop inDriver#run. We do a lot of extra computation forLuceneTopNOperatorto start emitting pages.What we'd want is to not require multiple calls to
LuceneTopNOperator#getOutputto actually output pages, and to be able to emit a page from the first time we call this method.However we still want to make sure we don't block the execution such that the query cannot be cancelled while
LuceneTopNOperator#getOutputruns.To mitigate this, we pass the
DriverContextto theLuceneTopNOperatorsuch that we can check if the query has been cancelled withdriverContext.checkForEarlyTermination().We can do a similar optimization for
LuceneCountOperatorif we are happy with this one.