Handle ASM Notifications - [MOD-11120]#7022
Merged
Conversation
tezc
reviewed
Oct 13, 2025
tezc
reviewed
Oct 13, 2025
GuyAv46
force-pushed
the
guyav-ASM_notifications
branch
from
63231b1 to
6ab0481
Compare
GuyAv46
changed the base branch from
master
to
guyav-serialize_schema_to_command
tezc
reviewed
Oct 13, 2025
tezc
reviewed
Oct 13, 2025
tezc
reviewed
Oct 13, 2025
GuyAv46
force-pushed
the
guyav-ASM_notifications
branch
6 times, most recently
from
c89f550 to
a538911
Compare
GuyAv46
added
enforce:coverage
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7022 +/- ##
==========================================
- Coverage 85.58% 85.56% -0.03%
==========================================
Files 327 328 +1
Lines 51985 52121 +136
Branches 11133 11133
==========================================
+ Hits 44492 44597 +105
- Misses 7325 7356 +31
Partials 168 168
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
GuyAv46
force-pushed
the
guyav-ASM_notifications
branch
from
c62a603 to
012f9f7
Compare
GuyAv46
force-pushed
the
guyav-ASM_notifications
branch
from
012f9f7 to
c8bcfc2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
alonre24
approved these changes
Oct 23, 2025
This comment was marked as outdated.
This comment was marked as outdated.
Collaborator
Author
|
/backport after also backporting #7129, which was missing. |
redisearch-backport-pull-request bot
pushed a commit
that referenced
this pull request
Oct 24, 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 * update SDK * Revert "[TEMP] set CI for new API" This reverts commit 9d2d165. * use new API RedisModule_ClusterKeySlotC * run against redis unstable until 8.4 is released * fix test flakiness - wait for both shards * increase node timeout for ASM tests * try adding a sleep * increase to 5 (cherry picked from commit 3bd770e)
Contributor
|
Successfully created backport PR for |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 24, 2025
Handle ASM Notifications - [MOD-11120] (#7022) * 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 * update SDK * Revert "[TEMP] set CI for new API" This reverts commit 9d2d165. * use new API RedisModule_ClusterKeySlotC * run against redis unstable until 8.4 is released * fix test flakiness - wait for both shards * increase node timeout for ASM tests * try adding a sleep * increase to 5 (cherry picked from commit 3bd770e) Co-authored-by: GuyAv46 <[email protected]>
YaacovHazan
pushed a commit
to redis/redis
that referenced
this pull request
Oct 27, 2025
(RediSearch/RediSearch#7076, RediSearch/RediSearch#6857) - Introducing `FT.HYBRID`, a new command that enables hybrid queries combining both text and vector search, with support for **RRF** and **LINEAR** result combination. This update enhances performance and reliability through a more efficient networking layer, smoother query execution, and improved overall stability. (RediSearch/RediSearch#7065) - Add `search-default-scorer` configuration to set the default text scorer across queries (by default it is BM25). RediSearch/RediSearch#7022 - Handle Atomic Slot Migration events upon moving slots from one node to another in a cluster mode. (RediSearch/RediSearch#6769, RediSearch/RediSearch#6828, RediSearch/RediSearch#6877, RediSearch/RediSearch#6921) - Introduce query time memory guardrails by adding a new `search-on-oom` configuration which defines the query engine behavior when OOM (Out Of Memory) is reached. OOM checks are applied to `FT.SEARCH`, `FT.AGGREGATE`, and `FT.HYBRID` commands. The behavior on OOM can be configured to one of three modes: `IGNORE`, `FAIL`, or `RETURN`. `IGNORE` - The default behavior, run queries anyway (not recommended for heavy queries returning a large result set). `FAIL` - Fail query execution immediately if any of the nodes are in OOM state when query execution starts. `RETURN` - A best effort appraoch to return partial results when OOM is detected in only some of the nodes in a cluster mode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Subscribe to new notifications and implement expected behavior
This PR relies on previous preparations made in:
The main logic required from the module to support the ASM feature is:
We need to propagate index schema definitions and spell-check dictionaries.
For the dictionaries, we use the existing local variant of
FT.DICTADDthat can handle an entire dictionary in one command, and doesn't fail if the dictionary already exists (we expect 0 to be returned in this case, indicating the entire word set was already present)For the schema propagation, we use the new internal and local
FT._RESTOREIFNX SCHEMAcommand (see #7032 for more info). This command gets a serialized representation of the schema (similar to RedisRESTOREcommand) and creates the schema at the destination node if it doesn't exist. Under a valid trigger, this command always returns "OK" (even if the index already exists)We still rely on key notifications to index documents, and we subscribe to a new
key_trimnotification for the trimmingThread Safety - Since we have the multi-threading feature for querying, we can't use the new
RedisModule_ClusterCanAccessKeysInSlotas it requires holding the GIL.Instead, we implement
SharedSlotRangeArrayinslot_ranges.c, which we set and update from the main thread, and take a thread-safe reference before moving to the background. Slots info may be slightly outdated, but since we currently can't guarantee full and exclusive slot coverage for a query (to be solved in a following release), this is done as a best effort. The decision to use this info to filter results depends on the migration events.Mark if applicable
Note
Adds ASM event handling with slot-based query filtering and schema/dictionary propagation, introduces cached local slot ranges, and updates workers, pipeline, and tests.
slot_ranges.{c,h}for cached local slot ranges with refcounting and access checks; drop cache on topology/migration changes.AREQand pass throughQueryPipelineParamstoRPQueryIterator.RPQueryIteratorfilters results by owned slots when enabled; manages slot range lifetime.Indexes_PropagateandDictionary_Propagate; propagate schemas (_FT._RESTOREIFNX SCHEMA) and dictionaries (_FT.DICTADD) during ASM migrate.Dictionary_AddAPI; fix delete arg types; addDictionary_Size.workersThreadPool_OnEventEndreturns status and guards waiting.unstableRedis tag temporarily.deps/RedisModulesSDKpointer.Written by Cursor Bugbot for commit e8da26a. This will update automatically on new commits. Configure here.