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

Backport #13202 to branch_9x #13295

Closed
wants to merge 3 commits into from
Closed

Backport #13202 to branch_9x #13295

wants to merge 3 commits into from

Conversation

vigyasharma
Copy link
Contributor

kaivalnp and others added 2 commits April 11, 2024 11:49
* Add timeout support to graph searches in AbstractKnnVectorQuery

* Also timeout exact searches

* Return partial KNN results

* Add tests for partial KNN results

- Refactor tests to base classes
- Also timeout exact searches in Lucene99HnswVectorsReader

* Add CHANGES.txt entry and fix some comments

---------

Co-authored-by: Kaival Parikh <[email protected]>
* Fix failing BaseKnnVectorQueryTestCase#testTimeout

* Also fix ParentBlockJoinKnnVectorQueryTestCase#testTimeout

---------

Co-authored-by: Kaival Parikh <[email protected]>
@vigyasharma vigyasharma changed the title backport gh 13202 Backport #13202 to branch_9x Apr 11, 2024
@vigyasharma
Copy link
Contributor Author

Now that the test failure have been fixed, I'm backporting #13202 to branck_9x. Running into some build issues on my local setup, which I think are unrelated to the commit. Opening this PR to run precommit checks before backporting.

@vigyasharma vigyasharma marked this pull request as ready for review April 11, 2024 19:34
@vigyasharma
Copy link
Contributor Author

Getting the same test failure here.

> Task :lucene:core:compileTestJava
/home/runner/work/lucene/lucene/lucene/core/src/test/org/apache/lucene/search/BaseKnnVectorQueryTestCase.java:816: error: cannot find symbol
          noTimeoutManager.newCollector(Integer.MAX_VALUE, searcher.leafContexts.getFirst());
                                                                                ^
  symbol:   method getFirst()
  location: variable leafContexts of type List<LeafReaderContext>
/home/runner/work/lucene/lucene/lucene/core/src/test/org/apache/lucene/search/BaseKnnVectorQueryTestCase.java:832: error: cannot find symbol
          timeoutManager.newCollector(Integer.MAX_VALUE, searcher.leafContexts.getFirst());
                                                                              ^
  symbol:   method getFirst()
  location: variable leafContexts of type List<LeafReaderContext>
Note: Some input files use or override a deprecated API.

TimeLimitingKnnCollectorManager noTimeoutManager =
new TimeLimitingKnnCollectorManager(delegate, null);
KnnCollector noTimeoutCollector =
noTimeoutManager.newCollector(Integer.MAX_VALUE, searcher.leafContexts.getFirst());
Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure getFirst is a JDK21+ only thing. The compilation level on branch_9x is too low to allow that API.

@vigyasharma
Copy link
Contributor Author

Backported changes directly to retain original commit messages. This PR would've needed a squash merge as it was not rebased on the latest branch_9x commit.

@vigyasharma vigyasharma deleted the backport_gh-13202 branch April 17, 2024 21:56
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.

3 participants