Skip to content

Fix SortedSkipperScorerSupplier.cost() calculation#16096

Merged
romseygeek merged 2 commits into
apache:mainfrom
romseygeek:dv/sorted-skipper-cost
May 21, 2026
Merged

Fix SortedSkipperScorerSupplier.cost() calculation#16096
romseygeek merged 2 commits into
apache:mainfrom
romseygeek:dv/sorted-skipper-cost

Conversation

@romseygeek
Copy link
Copy Markdown
Contributor

This fixes two issues:

  • SortedSkipperScorerSupplier.cost() could overestimate its value if the lower value of a range didn't align with a skipper block boundary. We only need to check the upper value for alignment, as if the lower value is not aligned the read cost will be lower than the calculation.
  • Tests for this were using the number of live docs to calculate the expected value but we also need to take into account docs added and then deleted.

We also add some explicit tests for the cost calculation for various range and skip block configurations.

This fixes two issues:
- SortedSkipperScorerSupplier.cost() could overestimate its value if the
  lower value of a range didn't align with a skipper block boundary.  We
  only need to check the upper value for alignment, as if the lower value
  is not aligned the read cost will be lower than the calculation.
- Tests for this were using the number of live docs to calculate the
  expected value but we also need to take into account docs added and
  then deleted.

We also add some explicit tests for the cost calculation for various
range and skip block configurations.
@romseygeek romseygeek requested a review from iverase May 21, 2026 09:55
@romseygeek romseygeek self-assigned this May 21, 2026
@romseygeek romseygeek added the skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check. label May 21, 2026
Copy link
Copy Markdown
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

Thanks Alan, LGTM

@romseygeek romseygeek merged commit 2959b06 into apache:main May 21, 2026
13 checks passed
romseygeek added a commit that referenced this pull request May 21, 2026
This fixes two issues:
- SortedSkipperScorerSupplier.cost() could overestimate its value if the
  lower value of a range didn't align with a skipper block boundary.  We
  only need to check the upper value for alignment, as if the lower value
  is not aligned the read cost will be lower than the calculation.
- Tests for this were using the number of live docs to calculate the
  expected value but we also need to take into account docs added and
  then deleted.

We also add some explicit tests for the cost calculation for various
range and skip block configurations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:core/search skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants