Skip to content

Fixes for inverted indexes encoding - [MOD-8248]#5347

Merged
GuyAv46 merged 14 commits intomasterfrom
guyav-fix_encoding
Dec 19, 2024
Merged

Fixes for inverted indexes encoding - [MOD-8248]#5347
GuyAv46 merged 14 commits intomasterfrom
guyav-fix_encoding

Conversation

@GuyAv46
Copy link
Collaborator

@GuyAv46 GuyAv46 commented Dec 12, 2024

Describe the changes in the pull request

While it was assumed the numeric encoder can encode any doc_id diff (while other encoders cannot encode delta > UINT32_MAX), all the encoders got a uint32_t delta, so the numeric encoder will not encode the right delta, causing later unrelated results to be returned.

The encoder also cannot encode any delta, but only deltas that fit in 7 bytes, due to the numeric header byte count limitation (counting 0-7 with 3 bits). We now also validate that before attempting to encode the delta, and creating a new block if needed.

Which additional issues this PR fixes

  1. Fix assertion condition
  2. Consume less memory with numeric inverted indexes
  3. Less pointer casting for safer code
  4. Remove dead code

Mark if applicable

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

@codecov
Copy link

codecov bot commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 86.77686% with 16 lines in your changes missing coverage. Please review.

Project coverage is 87.54%. Comparing base (ecb85b7) to head (3065ce1).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/inverted_index.c 85.22% 13 Missing ⚠️
src/fork_gc.c 0.00% 2 Missing ⚠️
src/doc_table.c 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5347      +/-   ##
==========================================
+ Coverage   87.06%   87.54%   +0.47%     
==========================================
  Files         195      196       +1     
  Lines       34802    34740      -62     
==========================================
+ Hits        30302    30414     +112     
+ Misses       4500     4326     -174     

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

@GuyAv46 GuyAv46 requested a review from alonre24 December 16, 2024 09:40
@GuyAv46 GuyAv46 enabled auto-merge December 19, 2024 12:37
@GuyAv46 GuyAv46 requested a review from alonre24 December 19, 2024 13:36
alonre24
alonre24 previously approved these changes Dec 19, 2024
@GuyAv46 GuyAv46 added this pull request to the merge queue Dec 19, 2024
@GuyAv46 GuyAv46 removed this pull request from the merge queue due to a manual request Dec 19, 2024
@GuyAv46 GuyAv46 added this pull request to the merge queue Dec 19, 2024
Merged via the queue into master with commit c03c15a Dec 19, 2024
@GuyAv46 GuyAv46 deleted the guyav-fix_encoding branch December 19, 2024 19:32
@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-5347-to-2.8 origin/2.8
cd .worktree/backport-5347-to-2.8
git switch --create backport-5347-to-2.8
git cherry-pick -x c03c15af98d8beb46512273a3ba9e64ad4058e25

@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-5347-to-2.6 origin/2.6
cd .worktree/backport-5347-to-2.6
git switch --create backport-5347-to-2.6
git cherry-pick -x c03c15af98d8beb46512273a3ba9e64ad4058e25

@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-5347-to-2.10 origin/2.10
cd .worktree/backport-5347-to-2.10
git switch --create backport-5347-to-2.10
git cherry-pick -x c03c15af98d8beb46512273a3ba9e64ad4058e25

redisearch-backport-pull-request bot pushed a commit that referenced this pull request Dec 19, 2024
* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)
@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 19, 2024
Fixes for inverted indexes encoding - [MOD-8248] (#5347)

* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)

Co-authored-by: GuyAv46 <[email protected]>
GuyAv46 added a commit that referenced this pull request Dec 20, 2024
* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)
GuyAv46 added a commit that referenced this pull request Dec 20, 2024
* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)
GuyAv46 added a commit that referenced this pull request Dec 20, 2024
* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)
GuyAv46 added a commit that referenced this pull request Dec 20, 2024
* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)
github-merge-queue bot pushed a commit that referenced this pull request Dec 22, 2024
* Fixes for inverted indexes encoding - [MOD-8248] (#5347)

* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)

* fix test for 2.6

* more fixes for 2.6

* another 2.6 fix

* final fixes for 2.6
github-merge-queue bot pushed a commit that referenced this pull request Dec 22, 2024
* Fixes for inverted indexes encoding - [MOD-8248] (#5347)

* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)

* fixes for 2.8

* final fix for 2.8

* build fix for 2.8
github-merge-queue bot pushed a commit that referenced this pull request Dec 22, 2024
* Fixes for inverted indexes encoding - [MOD-8248] (#5347)

* code cleanup

* fix numeric encoding

* fix unit-tests

* fix flow tests

* cleanup

* fix type

* minor cleanup

* some more cleanup

* simplify repair flow

* review fixes

* improve test to include all flags

* minor changes and cleanup

* added a sanity test

(cherry picked from commit c03c15a)

* final fixes for 2.10

* fix test for 2.10
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