dweiss opened a new pull request, #16557: URL: https://github.com/apache/lucene/pull/16557
This failure reproduces https://jenkins.thetaphi.de/job/Lucene-nightly-main-Linux/1922/: ``` ./gradlew :lucene:core:test --tests "org.apache.lucene.search.comparators.TestTermOrdValComparatorAdaptiveSkipping.testSkippingEffectiveForClusteredData" -Ptests.asserts=false -Ptests.file.encoding=ISO-8859-1 -Ptests.gui=true -Ptests.haltonfailure=false -Ptests.jvmargs= -Ptests.jvms=6 -Ptests.multiplier=3 -Ptests.nightly=true -Ptests.seed=8D38751139429793 -Ptests.vectorsize=512 ``` I let Claude analyze the cause and it says what's shown below. I am not familiar with this code at all, @romseygeek - does it make sense to you? We can skip SimpleText in this test (like the patch does) but I wonder if it's something that should be addressed deeper: --- Root cause — the adaptive-disable warmup budget is measured in skipper block crossings, and SimpleText's skipper blocks are tiny. The chain of events: 1. The test builds a 200k-doc segment where the first 100k docs sort-value is "a" and the last 100k are "z", then asserts that the skipper-based competitive iterator prunes the "z" half (totalHits well below 200k). 2. SkipperBasedCompetitiveState.AdaptiveSkipIterator (TermOrdValComparator.java:715) starts in WARMING state: it counts level-0 block-boundary crossings, and if it crosses WARMUP_BOUNDARY_CROSSINGS = 16 boundaries without ever observing an effective skip (result > target), it permanently flips to DISABLED (TermOrdValComparator.java:747). 3. SimpleText's DocValuesSkipper is single-level with 8-doc intervals (SKIP_INTERVAL_SIZE = 8 in SimpleTextDocValuesWriter.java:75, numLevels() == 1 in SimpleTextDocValuesReader.java:929). So while scanning the competitive "a" prefix — where no skipping is possible because every doc is competitive — a boundary is crossed every 8 docs, and docIDRunEnd() can't extend runs past one 8-doc block since there are no higher levels. The 16-crossing budget is exhausted around doc ~130, still 99,870 docs away from the "z" region where skipping would have kicked in. Skipping is disabled, the whole segment is collected, and totalHits == 200000 on both sides of the comparison. 4. The default codec survives because its skipper has 4096-doc level-0 blocks plus higher levels: SkipBlockRangeIterator.docIDRunEnd() climbs levels and consumes the "a" half in a handful of large advances, so the first advance past the a/z boundary produces a real skip well within the 16-crossing budget, flipping the state to permanently ACTIVE. So the test's assumption — "clustered data ⇒ skipping stays active" — only holds for codecs whose skipper blocks are large (or multi-level). It's arguably also a mild weakness of the heuristic itself (the warmup budget's real-world meaning scales with the codec's block size: 16 crossings samples 64k docs on the default codec but only 128 docs on SimpleText), but SimpleText is a test-only codec, so the practical fix is on the test side: annotate the class with @SuppressCodecs("SimpleText") (or force a codec with a realistic skipper), the same way other skipper-granularity-sensitive tests do. Note the previous fix on this test (d02fb2a4cc7) addressed a different randomization axis (merge policy); this one is the codec axis. Want me to apply the @SuppressCodecs fix? -- 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]
