Skip to content

Improve SkipToBlock logic - [MOD-8255]#5350

Merged
GuyAv46 merged 7 commits intomasterfrom
guyav-fix_binary_search
Dec 17, 2024
Merged

Improve SkipToBlock logic - [MOD-8255]#5350
GuyAv46 merged 7 commits intomasterfrom
guyav-fix_binary_search

Conversation

@GuyAv46
Copy link
Collaborator

@GuyAv46 GuyAv46 commented Dec 15, 2024

Describe the changes in the pull request

Fix the IndexReader_SkipToBlock logic to always set the current block as the first one containing a document with an ID greater or equal to the one we search for.

In some edge cases, we may have set the current block to the one before the next relevant block, causing an additional full block scan before we find the next relevant result.

This issue also causes an unexpected out-of-range case in the seekRawDocIdsOnly skipper, which assumes we are currently at the right block.

Main objects this PR modified

  1. IndexReader_SkipToBlock

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

@codecov
Copy link

codecov bot commented Dec 15, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 86.52%. Comparing base (98b342c) to head (a7c0b9b).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/inverted_index.c 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5350      +/-   ##
==========================================
+ Coverage   86.49%   86.52%   +0.03%     
==========================================
  Files         193      195       +2     
  Lines       34773    34809      +36     
==========================================
+ Hits        30076    30119      +43     
+ Misses       4697     4690       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GuyAv46 GuyAv46 added this pull request to the merge queue Dec 17, 2024
GuyAv46 added a commit that referenced this pull request Dec 17, 2024
Merged via the queue into master with commit 27870fe Dec 17, 2024
@GuyAv46 GuyAv46 deleted the guyav-fix_binary_search branch December 17, 2024 17:00
@redisearch-backport-pull-request
Copy link
Contributor

Backport failed for 2.8, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.8
git worktree add -d .worktree/backport-5350-to-2.8 origin/2.8
cd .worktree/backport-5350-to-2.8
git switch --create backport-5350-to-2.8
git cherry-pick -x 27870fedbe22ebc6ac288c3c05045342aba05fd0

@redisearch-backport-pull-request
Copy link
Contributor

Backport failed for 2.6, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.6
git worktree add -d .worktree/backport-5350-to-2.6 origin/2.6
cd .worktree/backport-5350-to-2.6
git switch --create backport-5350-to-2.6
git cherry-pick -x 27870fedbe22ebc6ac288c3c05045342aba05fd0

@redisearch-backport-pull-request
Copy link
Contributor

Backport failed for 2.10, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.10
git worktree add -d .worktree/backport-5350-to-2.10 origin/2.10
cd .worktree/backport-5350-to-2.10
git switch --create backport-5350-to-2.10
git cherry-pick -x 27870fedbe22ebc6ac288c3c05045342aba05fd0

redisearch-backport-pull-request bot pushed a commit that referenced this pull request Dec 17, 2024
* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)
@redisearch-backport-pull-request
Copy link
Contributor

Successfully created backport PR for 8.0:

github-merge-queue bot pushed a commit that referenced this pull request Dec 17, 2024
Improve SkipToBlock logic - [MOD-8255] (#5350)

* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)

Co-authored-by: GuyAv46 <[email protected]>
GuyAv46 added a commit that referenced this pull request Dec 18, 2024
* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)
GuyAv46 added a commit that referenced this pull request Dec 18, 2024
* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)
GuyAv46 added a commit that referenced this pull request Dec 18, 2024
* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)
github-merge-queue bot pushed a commit that referenced this pull request Dec 18, 2024
Improve SkipToBlock logic - [MOD-8255] (#5350)

* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)
github-merge-queue bot pushed a commit that referenced this pull request Dec 18, 2024
Improve SkipToBlock logic - [MOD-8255] (#5350)

* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)
github-merge-queue bot pushed a commit that referenced this pull request Dec 18, 2024
Improve SkipToBlock logic - [MOD-8255] (#5350)

* fix skip to block logic

* fix seekRawDocIdsOnly

* another cleanup

* remove dead code

* more dead code removal

* small fix

* revert seeker fix (moved to another PR)

(cherry picked from commit 27870fe)
github-merge-queue bot pushed a commit that referenced this pull request Dec 18, 2024
* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test
redisearch-backport-pull-request bot pushed a commit that referenced this pull request Dec 18, 2024
* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)
redisearch-backport-pull-request bot pushed a commit that referenced this pull request Dec 18, 2024
* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)
redisearch-backport-pull-request bot pushed a commit that referenced this pull request Dec 18, 2024
* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)
GuyAv46 added a commit that referenced this pull request Dec 18, 2024
* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)
github-merge-queue bot pushed a commit that referenced this pull request Dec 18, 2024
Improve "Raw doc id" encoding (#5364)

* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)

Co-authored-by: GuyAv46 <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2024
* Improve "Raw doc id" encoding (#5364)

* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)

* fix API for old version

---------

Co-authored-by: GuyAv46 <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2024
* Improve "Raw doc id" encoding (#5364)

* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)

* fix API for old version

---------

Co-authored-by: GuyAv46 <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2024
* Improve "Raw doc id" encoding (#5364)

* steal seeker fix from #5350

* try running RAW_DOCID_ENCODING with coverage

* [TEMP] disable skip on QUICK=1

* Revert temporary changes

* add a unit test

(cherry picked from commit 565633a)

* fix API for 2.6 version
raz-mon added a commit that referenced this pull request Feb 4, 2025
@raz-mon raz-mon mentioned this pull request Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants