Use jdk11 primitives to allow backport to branch_9x #13311
Conversation
| @@ -781,7 +781,7 @@ public void testTimeLimitingKnnCollectorManager() throws IOException { | |||
| TimeLimitingKnnCollectorManager noTimeoutManager = | |||
| new TimeLimitingKnnCollectorManager(delegate, null); | |||
| KnnCollector noTimeoutCollector = | |||
| noTimeoutManager.newCollector(Integer.MAX_VALUE, searcher.leafContexts.getFirst()); | |||
| noTimeoutManager.newCollector(Integer.MAX_VALUE, searcher.leafContexts.get(0)); | |||
There was a problem hiding this comment.
This doesn't really make sense to me :/. Main requires JDK21 and using all the nice things 21 provides is, well, nice.
Its pretty common for backports to have one or more weird issues due to compilation levels. The biggest one I have seen is if (klass instanceof ClassFoo classFoo) or the new switch statement syntax.
There was a problem hiding this comment.
What is our preferred approach for handling such compilation issues during backport? I was worried about recurring conflicts if we just change it in branch_9x, is that not the case? Skipping backport for something like this didn't seem right either. I'd like to learn what we consider as good practice for Lucene codebase.
There was a problem hiding this comment.
I was worried about recurring conflicts if we just change it in branch_9x, is that not the case?
I wouldn't expect the conflicts to be to horrible. The biggest ones I have ran into is the new switch statement usages.
Skipping backport for something like this didn't seem right either.
No, backporting the test fix is good. Allows them to be unmuted & ensures we are actually testing these paths.
I'd like to learn what we consider as good practice for Lucene codebase.
There is no prevalent pattern. Its a "gut" thing, and I would rather there not be any mandate.
Its fine that you did this, just wanted to indicate that in this particular case, seemed like it wasn't necessary.
BaseKnnVectorQueryTestCase#testTimeLimitingKnnCollectorManageruses some new java primitives that are breaking on backport to branch_9x, which builds with OpenJdk 11. We don't really need the new APIs, this small change allows us to backport without conflicts.