salvatorecampagna opened a new pull request, #16704: URL: https://github.com/apache/lucene/pull/16704
## TL;DR `TestHnswQueueSaturationCollector#testEarlyExitRelation` fails on rare seeds because it asserts an exact hit count for both of the collector's early exit modes. The test is split so each mode is covered deterministically. ## Summary `HnswQueueSaturationCollector#earlyTerminated` returns true either because patience was exhausted or because the delegate ran out of its visit budget, but `#topDocs` only rewrites the relation to `EQUAL_TO` for the former. That asymmetry is deliberate: `AbstractKnnVectorQuery` uses the relation to decide whether to fall back to exact search, so a saturated queue means the search converged and the fallback can be skipped, while an exhausted visit budget means it did not converge and the fallback is still needed. The test asserted `EQUAL_TO` for both, so it failed whenever the delegate was the reason for stopping: ``` java.lang.AssertionError: expected:<EQUAL_TO> but was:<GREATER_THAN_OR_EQUAL_TO> ``` The failure is rare because nothing in the test consumes the visit budget. `AbstractKnnCollector#earlyTerminated` compares `visitedCount` against `visitLimit`, and the test never calls `incVisitedCount`, so `visitedCount` stays zero and the delegate only reports early termination when `random.nextInt(numDocs)` happens to return a visit limit of zero. That is the same degenerate argument GITHUB#14434 already fixed for `k` on the line above. This has been reachable since GITHUB#14094 added the collector and the test, so it is long standing rather than a recent regression. Constraining the visit limit alone would hide the problem instead of fixing it, because the delegate branch would then be unreachable and only one of the two modes would still be covered. Instead `testEarlyExitRelationOnSaturation` keeps the randomized saturation path with a visit limit of at least one, and `testEarlyExitRelationOnExhaustedVisitLimit` drives `incVisitedCount` past the limit the way the graph searcher does, after saturating the queue, so that patience and the visit limit both ask to stop and the collector must still report a lower bound. Relaxing the guard in `#topDocs` makes that second test fail on every seed, so the production invariant is now pinned. No production code changes. ## Testing ``` ./gradlew :lucene:core:test --tests "org.apache.lucene.search.TestHnswQueueSaturationCollector" -Ptests.seed=FB332803A49A8C90 ./gradlew :lucene:core:test --tests "org.apache.lucene.search.TestHnswQueueSaturationCollector" -Ptests.iters=500 ./gradlew tidy ``` The previously failing seed passes, and 500 iterations of the class run 2500 tests with no failures. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
