Skip to content

[MOD-12005] Add slot ranges comparison logic#7130

Merged
GuyAv46 merged 69 commits intomasterfrom
joan-slots-comparison
Oct 28, 2025
Merged

[MOD-12005] Add slot ranges comparison logic#7130
GuyAv46 merged 69 commits intomasterfrom
joan-slots-comparison

Conversation

@JoanFM
Copy link
Collaborator

@JoanFM JoanFM commented Oct 22, 2025

Describe the changes in the pull request

Add logic to compare slot ranges


Note

Introduce CompareSlotRanges with SlotRangesComparisonResult and add C++ tests covering match/subset/non-inclusion and edge cases.

  • Core:
    • Add SlotRangesComparisonResult enum and CompareSlotRanges(const RedisModuleSlotRangeArray* ranges_expected, const RedisModuleSlotRangeArray* ranges_actual) to compare slot range arrays (match, subset, does-not-include).
    • Update src/slot_ranges.h with API, documentation, and C linkage guards; forward-declare RedisModuleSlotRangeArray.
  • Tests:
    • Add tests/cpptests/test_cpp_slot_ranges.cpp with unit tests covering exact match, subsets (single/multiple), non-inclusion, overlaps, gaps, boundaries, and max slot values.

Written by Cursor Bugbot for commit 7eda101. This will update automatically on new commits. Configure here.

@JoanFM JoanFM requested a review from GuyAv46 October 22, 2025 15:30
@JoanFM JoanFM force-pushed the joan-slots-comparison branch from 475bd20 to f0c8181 Compare October 22, 2025 15:35
@JoanFM JoanFM force-pushed the joan-slots-comparison branch from 871332c to eeaca26 Compare October 23, 2025 08:41
@JoanFM JoanFM added the enforce:sanitize Run sanitizer flow even on draft pull request label Oct 23, 2025
Base automatically changed from guyav-ASM_notifications to master October 23, 2025 16:57
@JoanFM JoanFM marked this pull request as ready for review October 24, 2025 09:07
@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.65%. Comparing base (19f9245) to head (7eda101).
⚠️ Report is 15 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7130      +/-   ##
==========================================
+ Coverage   85.59%   85.65%   +0.06%     
==========================================
  Files         328      328              
  Lines       51001    51020      +19     
  Branches    11016    11016              
==========================================
+ Hits        43652    43700      +48     
+ Misses       7179     7151      -28     
+ Partials      170      169       -1     
Flag Coverage Δ
flow 84.14% <0.00%> (+0.03%) ⬆️
unit 51.83% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@GuyAv46 GuyAv46 added this pull request to the merge queue Oct 28, 2025
Merged via the queue into master with commit 70486dc Oct 28, 2025
30 checks passed
@GuyAv46 GuyAv46 deleted the joan-slots-comparison branch October 28, 2025 17:30
redisearch-backport-pull-request bot pushed a commit that referenced this pull request Oct 28, 2025
* initial registration implementation

* Set up most of the notification events

* address review comment and add logs

* implement propagation

* minor improvements

* small fix

* review fixes

* [TEMP] set CI for new API

* dictionary tidy up

* add first test

* fix helper and test for duplicates

* Add missing sub-command

* add unit-tests for coverage

* change workersThreadPool_OnEvent* behavior to handle overlapping events

* rename events after API update

* implement managed and thread-safe slot ranges API

* drop cached API when needed

* rename API and use in the pipeline

* move includes to C file

* skip current flow test, and introduce a stable one

* fix initial reference count

* implement mock and add a test

* add missing key-space notification subscription

* add a test with adding a new node and propagating the schema

* review fixes

* fix assertion

* leak fix

* use malloc

* leak actual fix

* don't drop the cache on failed events

* add BG variants to flow tests

* initial registration implementation

* Set up most of the notification events

* address review comment and add logs

* implement propagation

* minor improvements

* small fix

* review fixes

* [TEMP] set CI for new API

* dictionary tidy up

* add first test

* fix helper and test for duplicates

* Add missing sub-command

* add unit-tests for coverage

* change workersThreadPool_OnEvent* behavior to handle overlapping events

* rename events after API update

* implement managed and thread-safe slot ranges API

* drop cached API when needed

* rename API and use in the pipeline

* move includes to C file

* skip current flow test, and introduce a stable one

* fix initial reference count

* implement mock and add a test

* add missing key-space notification subscription

* add a test with adding a new node and propagating the schema

* review fixes

* fix assertion

* leak fix

* use malloc

* leak actual fix

* don't drop the cache on failed events

* add BG variants to flow tests

* feat: add slot ranges comparition logic

* fix: change implementation

* fix problems merging

* fix problems merging

* fix problems merging

---------

Co-authored-by: GuyAv46 <[email protected]>
(cherry picked from commit 70486dc)
@redisearch-backport-pull-request
Copy link
Contributor

Successfully created backport PR for 8.4:

github-merge-queue bot pushed a commit that referenced this pull request Oct 28, 2025
[MOD-12005] Add slot ranges comparison logic (#7130)

* initial registration implementation

* Set up most of the notification events

* address review comment and add logs

* implement propagation

* minor improvements

* small fix

* review fixes

* [TEMP] set CI for new API

* dictionary tidy up

* add first test

* fix helper and test for duplicates

* Add missing sub-command

* add unit-tests for coverage

* change workersThreadPool_OnEvent* behavior to handle overlapping events

* rename events after API update

* implement managed and thread-safe slot ranges API

* drop cached API when needed

* rename API and use in the pipeline

* move includes to C file

* skip current flow test, and introduce a stable one

* fix initial reference count

* implement mock and add a test

* add missing key-space notification subscription

* add a test with adding a new node and propagating the schema

* review fixes

* fix assertion

* leak fix

* use malloc

* leak actual fix

* don't drop the cache on failed events

* add BG variants to flow tests

* initial registration implementation

* Set up most of the notification events

* address review comment and add logs

* implement propagation

* minor improvements

* small fix

* review fixes

* [TEMP] set CI for new API

* dictionary tidy up

* add first test

* fix helper and test for duplicates

* Add missing sub-command

* add unit-tests for coverage

* change workersThreadPool_OnEvent* behavior to handle overlapping events

* rename events after API update

* implement managed and thread-safe slot ranges API

* drop cached API when needed

* rename API and use in the pipeline

* move includes to C file

* skip current flow test, and introduce a stable one

* fix initial reference count

* implement mock and add a test

* add missing key-space notification subscription

* add a test with adding a new node and propagating the schema

* review fixes

* fix assertion

* leak fix

* use malloc

* leak actual fix

* don't drop the cache on failed events

* add BG variants to flow tests

* feat: add slot ranges comparition logic

* fix: change implementation

* fix problems merging

* fix problems merging

* fix problems merging

---------


(cherry picked from commit 70486dc)

Co-authored-by: Joan Fontanals <[email protected]>
Co-authored-by: GuyAv46 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 8.4 enforce:sanitize Run sanitizer flow even on draft pull request size:M size:XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants