Simplify Shared Lock internals#7267
Merged
JoanFM merged 7 commits intojoan-double-lockfrom Nov 10, 2025
Merged
Conversation
JoanFM
reviewed
Nov 9, 2025
| GIL_lent = true; | ||
| pthread_mutex_unlock(&InternalLock); | ||
| // Signal any waiting threads that they may try to borrow the GIL. | ||
| pthread_cond_broadcast(&GILAvailable); |
Collaborator
There was a problem hiding this comment.
Are we sure about this pattern? didn't we say to broadcast before unlocking?
Collaborator
Author
There was a problem hiding this comment.
In this case there is no relation between the lock and condition. We do broadcast the signal before releasing GILAlternativeLock
JoanFM
reviewed
Nov 9, 2025
This comment was marked as resolved.
This comment was marked as resolved.
JoanFM
requested changes
Nov 9, 2025
| typedef enum { | ||
| GIL_Locked, | ||
| Internal_Locked, | ||
| Unlocked = 0, |
Collaborator
There was a problem hiding this comment.
Since this is only used for the output and input of the Acquire/Release cycle, I would not expose the Unlocked as a potential enum value.
I would handle this by having a local variable in the stack, and not expose it.
Imagine this being migrated to Rust, we would be forced to add a lot of panics and patterns that would only add noise.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## joan-double-lock #7267 +/- ##
====================================================
- Coverage 85.04% 85.03% -0.02%
====================================================
Files 342 342
Lines 51734 51729 -5
Branches 12526 12526
====================================================
- Hits 43999 43987 -12
- Misses 7543 7550 +7
Partials 192 192
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
commented
Nov 10, 2025
| // 2. The value of 'type' is the result of the previous call to SharedExclusiveLock_Acquire | ||
| void SharedExclusiveLock_Release(RedisModuleCtx *ctx, SharedExclusiveLockType type) { | ||
| if (type == Internal_Locked) { | ||
| if (type == Borrowed) { |
Collaborator
Author
There was a problem hiding this comment.
Even though it's useful to get the lock type from SharedExclusiveLock_Acquire for testing or logging, I think the Release function should make its decision based on the GIL_borrowed variable
Collaborator
There was a problem hiding this comment.
I prefer to rely as little as possible on globals, and testing is not at all a minor issue
JoanFM
approved these changes
Nov 10, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 29, 2025
* first commit * fix: handle lock in two phases * inform ownership in regular command process handlers * fix: handle proper init * fix: use conditional variable * fix: add some comments * test: add test proving deadlock * fix: changes as per comments * fix: remove this locking from conn * remove code where certainity is not so high * fix potential issue * test: add first unit tests version * test: add proper testing with the value proposition * test: improve testing * alternative using auxiliary lock * simplify thpool a bit * make sure RedisModule_Yield is protected * fix: fix issue raised by cursor * fix: handle potential deadlock in drain also * fix: fix potential TOCTOU concurrency bug * fix: fix order of release * fix: fix potential overflow issue * fix tests * fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * change as per PR comments * add micro benchmarks with jobs in threads * remove changes not wanted * checkout redis feature branch in task test * remove draining * move atomic to new header * remove _internal naming * fix formatting --------- Co-authored-by: GuyAv46 <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 29, 2025
* first commit * fix: handle lock in two phases * inform ownership in regular command process handlers * fix: handle proper init * fix: use conditional variable * fix: add some comments * test: add test proving deadlock * fix: changes as per comments * fix: remove this locking from conn * remove code where certainity is not so high * fix potential issue * test: add first unit tests version * test: add proper testing with the value proposition * test: improve testing * alternative using auxiliary lock * simplify thpool a bit * make sure RedisModule_Yield is protected * fix: fix issue raised by cursor * fix: handle potential deadlock in drain also * fix: fix potential TOCTOU concurrency bug * fix: fix order of release * fix: fix potential overflow issue * fix tests * fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * change as per PR comments * add micro benchmarks with jobs in threads * remove changes not wanted * checkout redis feature branch in task test * remove draining * move atomic to new header * remove _internal naming * fix formatting --------- Co-authored-by: GuyAv46 <[email protected]>
redisearch-backport-pull-request bot
pushed a commit
that referenced
this pull request
Nov 29, 2025
* first commit * fix: handle lock in two phases * inform ownership in regular command process handlers * fix: handle proper init * fix: use conditional variable * fix: add some comments * test: add test proving deadlock * fix: changes as per comments * fix: remove this locking from conn * remove code where certainity is not so high * fix potential issue * test: add first unit tests version * test: add proper testing with the value proposition * test: improve testing * alternative using auxiliary lock * simplify thpool a bit * make sure RedisModule_Yield is protected * fix: fix issue raised by cursor * fix: handle potential deadlock in drain also * fix: fix potential TOCTOU concurrency bug * fix: fix order of release * fix: fix potential overflow issue * fix tests * fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * change as per PR comments * add micro benchmarks with jobs in threads * remove changes not wanted * checkout redis feature branch in task test * remove draining * move atomic to new header * remove _internal naming * fix formatting --------- Co-authored-by: GuyAv46 <[email protected]> (cherry picked from commit 44f0253)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 29, 2025
[MOD-12170] Implement ASM State Machine on notifications (#7331) * first commit * fix: handle lock in two phases * inform ownership in regular command process handlers * fix: handle proper init * fix: use conditional variable * fix: add some comments * test: add test proving deadlock * fix: changes as per comments * fix: remove this locking from conn * remove code where certainity is not so high * fix potential issue * test: add first unit tests version * test: add proper testing with the value proposition * test: improve testing * alternative using auxiliary lock * simplify thpool a bit * make sure RedisModule_Yield is protected * fix: fix issue raised by cursor * fix: handle potential deadlock in drain also * fix: fix potential TOCTOU concurrency bug * fix: fix order of release * fix: fix potential overflow issue * fix tests * fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * change as per PR comments * add micro benchmarks with jobs in threads * remove changes not wanted * checkout redis feature branch in task test * remove draining * move atomic to new header * remove _internal naming * fix formatting --------- (cherry picked from commit 44f0253) Co-authored-by: Joan Fontanals <[email protected]> Co-authored-by: GuyAv46 <[email protected]>
alonre24
added a commit
that referenced
this pull request
Dec 3, 2025
* fix: Avoid Rust cache contamination across platforms. (#7569) * [MOD-12170] Implement ASM State Machine on notifications (#7331) * first commit * fix: handle lock in two phases * inform ownership in regular command process handlers * fix: handle proper init * fix: use conditional variable * fix: add some comments * test: add test proving deadlock * fix: changes as per comments * fix: remove this locking from conn * remove code where certainity is not so high * fix potential issue * test: add first unit tests version * test: add proper testing with the value proposition * test: improve testing * alternative using auxiliary lock * simplify thpool a bit * make sure RedisModule_Yield is protected * fix: fix issue raised by cursor * fix: handle potential deadlock in drain also * fix: fix potential TOCTOU concurrency bug * fix: fix order of release * fix: fix potential overflow issue * fix tests * fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * change as per PR comments * add micro benchmarks with jobs in threads * remove changes not wanted * checkout redis feature branch in task test * remove draining * move atomic to new header * remove _internal naming * fix formatting --------- Co-authored-by: GuyAv46 <[email protected]> * [MOD-12627] Add Debug Support for `FT.PROFILE` Command (#7510) * imp debug profile for SA: introduce in mocule.h: RSProfileCommandImp RSProfileCommand calls RSProfileCommandImp(isDebug = false) for regular execution ProfileCommandCommand_DebugWrapper mcalls it with isDebug=true and skips _FT.DEBUG introduce entrypoint for _FT.DEBUG FT.PROFILE in debug_commands: ProfileCommandCommand_DebugWrapper RSProfileCommandImp calls DEBUG_execCommandCommon is its debug _recursiveProfilePrint skips printing debug RP * pass is debug instead of extracting: module.h: replace declaration: DistAggregateCommand DistSearchCommand with Imp version that receives isDebug expose ProfileCommandHandlerImp align debug_commands introducr _FT.DEBUG _FT.PROFILE * add test for cluster * return res * augi fixes * fix spell check * fix for real * fix test * skip tests according to env * revrt test_profile changes * reove changes from internal_only * [MOD-12694] [MOD-12069] Add active_coord_threads metric (#7546) * Add multi-threading statistics tracking for active I/O threads * fix comment * add cpp test * fix spelling * address comment * remove unnecessary nre line * add "active_worker_threads" metric * fix comment imp tests * test cleanups * add test comne about the num queries and cleanups * fix declartion * remove coord threads * add active_coord_threads expose ConcurrentSearchPool_WorkingThreadCount * make the tests run... * add "active_worker_threads" metric * fix declartion * remove coord threads * make the tests run... * introduce workersThreadPool_isInitialized assert is initizlied in GlobalStats_GetMultiThreadingStats * cleanup * rename workersThreadPool_isCreated * introduce ConcurrentSearchPool_IsCreated * fix test * we dont need workers * remove ConcurrentSearchPool_IsCreated and workersThreadPool_isInitialized * fix merge * [MOD-12789] test: fix flaky thpool test (#7581) test: fix flaky thpool test * Support Multiple Slot Ranges in search.CLUSTERSET - [MOD-11657] (#7508) * support multiple slot ranges * implement and move around helpers * add tests * ignore shards with no slots * improve parsing * better error message * fix flow tests * fix tests * sort by node id * improve error testing * cover missing cases * more error messages improvements * address AI review * stabilize Unexpected argument error path * stabilize more error paths * fix tests accordingly * last fix * add logs to cluster set command * add more logs per @alonre24 request * rename MetricIterator to Metric (#7586) The other iterators are not suffixed with 'Iterator'. * [MOD-12701] Split the execution of Rust and C/C++ unit tests across two different CI steps. (#7587) Split the execution of Rust and C/C++ unit tests across two different CI steps. * [MOD-12519] implement skip multi in II iterators (#7426) * remove it->skipMulti No longer needed as the full iterators have been removed. The query ones always set it to true. * implement skip multi II query iterators are supposed to skip results having the same ids. Test ported from test_cpp_iterator_index.cpp * inline read() and skip_to() * test more ii iterator edge cases Port of GetCorrectValue and EOFAfterFiltering from test_cpp_iterator_index.cpp * Compress layers prior to exporting them to the Docker layer cache (#7529) * Compress Docker layers prior to exporting them. * Ignore boost subfolders recursively * [MOD-12417] Track maxprefixexpansions errors and warnings in info (#7570) track maxprefixexpanions * [MOD-12409]: Port DocumentType enum to Rust (#7590) Port DocumentType enum to Rust Port DocumentType enum to Rust as `document::DocumentType`, removing it from `redisearch.h`. Use `document::DocumentType` in `rlookup` instead of `rlookup::bindings::DocumentType`. * [MOD-12069] Add `*_pending_jobs` metrics (#7556) * align info/* to active_coord * add APIs to get queues length * add to info * fix * test * fix test * catch general error * rename * fix moduleArgs * rename * rename test_active_worker_threads * rename to wworketrs * [MOD-12392] Remove numDocs parameter from non-optimized Wildcard iterator (#7602) Remove numDocs parameter from non-optimized Wildcard iterator * [MOD-12701] Enforce a per-test timeout in the C++ rstest suite using ctest (#7588) * Enforce a per-test timeout in the C++ rstest suite using ctest * Disable problematic tests * Raise timeout to 60s * Raise timeout * Add a timeout for coordinator tests too * Skip ActivateIoThreadsMetric test * Register Cursor Sub-Commands as such - [MOD-12807, MOD-12808] (#7571) * split cursor command * fix and improve tests * cover error cases * fix cursor leaks * Add "TODO: run hybrid cursor" back * remove new empty line * small test improvement * fix FT.CURSOR GC * de-flake test * make CURSOR PROFILE internal only * test the free * Keep just the prints * test on macos and noble as well * Move the free * Remove * Add to non container * Back to regular test * Remove spaces * Moved the remove and add the repo size * Moved it again * Move it to after repo build * Move print after repo build * test all * run the temp flow * add mount to container * Fix curly --------- Co-authored-by: Luca Palmieri <[email protected]> Co-authored-by: Joan Fontanals <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: meiravgri <[email protected]> Co-authored-by: Guillaume Desmottes <[email protected]> Co-authored-by: lerman25 <[email protected]> Co-authored-by: Henk Oordt <[email protected]> Co-authored-by: alonre24 <[email protected]>
pull bot
pushed a commit
to Mu-L/RediSearch
that referenced
this pull request
Dec 3, 2025
* Reduce merge queue * add ability to run manually * CR comments * allow workflow call for testing * Fix naming in build image + remove quick from flow intel as well * use ubuntu nobel rather than latest * fix noble typo * CR fixes * CR fixes 2 * don't use container for cov and san * restore leftover * remove mac + intel and workflow call * remove macos intel from matrix * change back ubuntu:latest to ubuntu:noble in merge-to-queue as per Jonathan comment * measure disk space * fix step name * Free disk on container (RediSearch#7613) * fix: Avoid Rust cache contamination across platforms. (RediSearch#7569) * [MOD-12170] Implement ASM State Machine on notifications (RediSearch#7331) * first commit * fix: handle lock in two phases * inform ownership in regular command process handlers * fix: handle proper init * fix: use conditional variable * fix: add some comments * test: add test proving deadlock * fix: changes as per comments * fix: remove this locking from conn * remove code where certainity is not so high * fix potential issue * test: add first unit tests version * test: add proper testing with the value proposition * test: improve testing * alternative using auxiliary lock * simplify thpool a bit * make sure RedisModule_Yield is protected * fix: fix issue raised by cursor * fix: handle potential deadlock in drain also * fix: fix potential TOCTOU concurrency bug * fix: fix order of release * fix: fix potential overflow issue * fix tests * fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (RediSearch#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * change as per PR comments * add micro benchmarks with jobs in threads * remove changes not wanted * checkout redis feature branch in task test * remove draining * move atomic to new header * remove _internal naming * fix formatting --------- Co-authored-by: GuyAv46 <[email protected]> * [MOD-12627] Add Debug Support for `FT.PROFILE` Command (RediSearch#7510) * imp debug profile for SA: introduce in mocule.h: RSProfileCommandImp RSProfileCommand calls RSProfileCommandImp(isDebug = false) for regular execution ProfileCommandCommand_DebugWrapper mcalls it with isDebug=true and skips _FT.DEBUG introduce entrypoint for _FT.DEBUG FT.PROFILE in debug_commands: ProfileCommandCommand_DebugWrapper RSProfileCommandImp calls DEBUG_execCommandCommon is its debug _recursiveProfilePrint skips printing debug RP * pass is debug instead of extracting: module.h: replace declaration: DistAggregateCommand DistSearchCommand with Imp version that receives isDebug expose ProfileCommandHandlerImp align debug_commands introducr _FT.DEBUG _FT.PROFILE * add test for cluster * return res * augi fixes * fix spell check * fix for real * fix test * skip tests according to env * revrt test_profile changes * reove changes from internal_only * [MOD-12694] [MOD-12069] Add active_coord_threads metric (RediSearch#7546) * Add multi-threading statistics tracking for active I/O threads * fix comment * add cpp test * fix spelling * address comment * remove unnecessary nre line * add "active_worker_threads" metric * fix comment imp tests * test cleanups * add test comne about the num queries and cleanups * fix declartion * remove coord threads * add active_coord_threads expose ConcurrentSearchPool_WorkingThreadCount * make the tests run... * add "active_worker_threads" metric * fix declartion * remove coord threads * make the tests run... * introduce workersThreadPool_isInitialized assert is initizlied in GlobalStats_GetMultiThreadingStats * cleanup * rename workersThreadPool_isCreated * introduce ConcurrentSearchPool_IsCreated * fix test * we dont need workers * remove ConcurrentSearchPool_IsCreated and workersThreadPool_isInitialized * fix merge * [MOD-12789] test: fix flaky thpool test (RediSearch#7581) test: fix flaky thpool test * Support Multiple Slot Ranges in search.CLUSTERSET - [MOD-11657] (RediSearch#7508) * support multiple slot ranges * implement and move around helpers * add tests * ignore shards with no slots * improve parsing * better error message * fix flow tests * fix tests * sort by node id * improve error testing * cover missing cases * more error messages improvements * address AI review * stabilize Unexpected argument error path * stabilize more error paths * fix tests accordingly * last fix * add logs to cluster set command * add more logs per @alonre24 request * rename MetricIterator to Metric (RediSearch#7586) The other iterators are not suffixed with 'Iterator'. * [MOD-12701] Split the execution of Rust and C/C++ unit tests across two different CI steps. (RediSearch#7587) Split the execution of Rust and C/C++ unit tests across two different CI steps. * [MOD-12519] implement skip multi in II iterators (RediSearch#7426) * remove it->skipMulti No longer needed as the full iterators have been removed. The query ones always set it to true. * implement skip multi II query iterators are supposed to skip results having the same ids. Test ported from test_cpp_iterator_index.cpp * inline read() and skip_to() * test more ii iterator edge cases Port of GetCorrectValue and EOFAfterFiltering from test_cpp_iterator_index.cpp * Compress layers prior to exporting them to the Docker layer cache (RediSearch#7529) * Compress Docker layers prior to exporting them. * Ignore boost subfolders recursively * [MOD-12417] Track maxprefixexpansions errors and warnings in info (RediSearch#7570) track maxprefixexpanions * [MOD-12409]: Port DocumentType enum to Rust (RediSearch#7590) Port DocumentType enum to Rust Port DocumentType enum to Rust as `document::DocumentType`, removing it from `redisearch.h`. Use `document::DocumentType` in `rlookup` instead of `rlookup::bindings::DocumentType`. * [MOD-12069] Add `*_pending_jobs` metrics (RediSearch#7556) * align info/* to active_coord * add APIs to get queues length * add to info * fix * test * fix test * catch general error * rename * fix moduleArgs * rename * rename test_active_worker_threads * rename to wworketrs * [MOD-12392] Remove numDocs parameter from non-optimized Wildcard iterator (RediSearch#7602) Remove numDocs parameter from non-optimized Wildcard iterator * [MOD-12701] Enforce a per-test timeout in the C++ rstest suite using ctest (RediSearch#7588) * Enforce a per-test timeout in the C++ rstest suite using ctest * Disable problematic tests * Raise timeout to 60s * Raise timeout * Add a timeout for coordinator tests too * Skip ActivateIoThreadsMetric test * Register Cursor Sub-Commands as such - [MOD-12807, MOD-12808] (RediSearch#7571) * split cursor command * fix and improve tests * cover error cases * fix cursor leaks * Add "TODO: run hybrid cursor" back * remove new empty line * small test improvement * fix FT.CURSOR GC * de-flake test * make CURSOR PROFILE internal only * test the free * Keep just the prints * test on macos and noble as well * Move the free * Remove * Add to non container * Back to regular test * Remove spaces * Moved the remove and add the repo size * Moved it again * Move it to after repo build * Move print after repo build * test all * run the temp flow * add mount to container * Fix curly --------- Co-authored-by: Luca Palmieri <[email protected]> Co-authored-by: Joan Fontanals <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: meiravgri <[email protected]> Co-authored-by: Guillaume Desmottes <[email protected]> Co-authored-by: lerman25 <[email protected]> Co-authored-by: Henk Oordt <[email protected]> Co-authored-by: alonre24 <[email protected]> * Revert "Free disk on container (RediSearch#7613)" This reverts commit 9c68740. * add free disk step * rename + remove temp test * remove leftover * better readability in container input Co-authored-by: GuyAv46 <[email protected]> * fix double defaults --------- Co-authored-by: dor-forer <[email protected]> Co-authored-by: Luca Palmieri <[email protected]> Co-authored-by: Joan Fontanals <[email protected]> Co-authored-by: GuyAv46 <[email protected]> Co-authored-by: meiravgri <[email protected]> Co-authored-by: Guillaume Desmottes <[email protected]> Co-authored-by: lerman25 <[email protected]> Co-authored-by: Henk Oordt <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 17, 2025
* fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * remove SharedSlotRangeArray from code * implement basic logic * change as per PR comments * fix: remove latest SlotRange reference * set local slots on UpdateTopology * change as per PR comments * fix: fix query error * some code changes needed * handle tests * fix compilation in Linux * fix loading func in module * fix tests hybrid internal standalone * fix test standalone acl * fix test standalone asm * test: handle tests internal to different * handle more tests slowly * improve testing * improve testing * fix sanitizer errors in tests * remove ASM comment * small improvement commenting * schedule refresh topology in timer * remove println * test: fix test ASM * test: improve ASM test * further improve * improve asm tests * fix formatting issue * try to handle hybrid test * test: change test to compare scores * improve testing a little * add micro benchmarks with jobs in threads * fix: fail on error from coordinator * small changes in test asm * remove changes not wanted * checkout redis feature branch in task test * small change * remove draining * move atomic to new header * remove _internal naming * fix formatting * add structure of timers * add initial version of KeySpaceVersionTracker * implementation and unit testing of KeySpaceVersionTracker * Plugin keyspace version tracker counters into the flow * small code comment * align with trimming redismodule.h * install feature redis in CI workflow * plug new RedisModule API to delay trimming * remove sleep from test * fix values and enable logging * make query version map extern to avoid different instances per compilation unit * rename vars * small cleanup and rename * use debug print * query all shards * fix linting issue * align with latest commit from Redis dependency * fix on RedisModulesSDK * do not rely on TimerID for initialization state * handle ASM Decrease Query Count * fix access to keyspace version * fix: remove ASM count from AREQ_Free * more progress towards keyspace query counting * handle some small PR comments * avoid cplusplus code duplication * align test with no cplusplus deduplication * add helper to identify leaks of query counts on sanitizer * add keyspaceversion to BockQueryNode * control leaks of cursor query count when queries fail * check if is hybrid request from AREQ itself * explain better the should_filter_slots * refactor how to handle topology timer reset * remove decrease of count on cursor Read * align with dedicated API for cluster enable/disable trimming * make query cursor tracker map thread safe * remove some changes * handle Decrease of Query Count for each AREQ individually in a RAII manner * small cleanup of PR * fix leak: decrease without previous increase * add log to debug * change in github workflow * fix a test * Apply suggestions from code review * all ranges are owned in StandAlone cluster * remove txt from pytests * all slots dynamic allocation * fix compilation warning * register new API to c_symbols.rs * proper mutex lifetime management * degine mutex as extern to avoid duplication in translation units * reschedule with slots * fix: fix potential leak and correct log message condition * reset slots when local slots are updated * fix: accept not normlized slot ranges as input * Revert "fix: accept not normlized slot ranges as input" This reverts commit 1babfed. * add configuration for max min delay trimming ms * Apply suggestions from code review Co-authored-by: GuyAv46 <[email protected]> * apply some suggestion changes * fix test acl * proper verbose level * remove inlining and create struct in notifications * make config underscore * add comments in code and fix typo * add configuration for delay check ms * avoid trim delay if no Indices found (no module usage) * Update tests/pytests/test_hybrid_internal.py Co-authored-by: GuyAv46 <[email protected]> * refactor asm state machine sanitizer code * ASM_StartTrim properly * refactor test_hybrid_internal * small refactor test asm * add tests for FT AGGREGATE WITHCURSORS * fix naming to be private of config * test: handle test where cursor returns empty result after trimming starts * add proper trimmed tests * adapt test asm * fix use after free connection * use arr.h to avoid reallocating in ASM Sanitizer * Apply suggestions from code review Co-authored-by: GuyAv46 <[email protected]> * test against redis unstable * update RedisModulesSDK * fix how to pass hybrid query vectors in query * fix .gitmodules back * update RedisModulesSDK * proper error handling * clarify invariants with an ASSERT * skip while no fix is found * do not assert, stop timers if they are scheduled * reply to cursor comments * small refactoring notifications * improve logging * add test for empty indices * test: improve testing * small fix * small test change * include timeout message * test no indexes * adapt changes * make test longer to cover timers * fix test * fix: avoid memory leak of ever growing dict * Update src/asm_state_machine.h Co-authored-by: GuyAv46 <[email protected]> --------- Co-authored-by: GuyAv46 <[email protected]>
JoanFM
added a commit
that referenced
this pull request
Dec 18, 2025
* fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * remove SharedSlotRangeArray from code * implement basic logic * change as per PR comments * fix: remove latest SlotRange reference * set local slots on UpdateTopology * change as per PR comments * fix: fix query error * some code changes needed * handle tests * fix compilation in Linux * fix loading func in module * fix tests hybrid internal standalone * fix test standalone acl * fix test standalone asm * test: handle tests internal to different * handle more tests slowly * improve testing * improve testing * fix sanitizer errors in tests * remove ASM comment * small improvement commenting * schedule refresh topology in timer * remove println * test: fix test ASM * test: improve ASM test * further improve * improve asm tests * fix formatting issue * try to handle hybrid test * test: change test to compare scores * improve testing a little * add micro benchmarks with jobs in threads * fix: fail on error from coordinator * small changes in test asm * remove changes not wanted * checkout redis feature branch in task test * small change * remove draining * move atomic to new header * remove _internal naming * fix formatting * add structure of timers * add initial version of KeySpaceVersionTracker * implementation and unit testing of KeySpaceVersionTracker * Plugin keyspace version tracker counters into the flow * small code comment * align with trimming redismodule.h * install feature redis in CI workflow * plug new RedisModule API to delay trimming * remove sleep from test * fix values and enable logging * make query version map extern to avoid different instances per compilation unit * rename vars * small cleanup and rename * use debug print * query all shards * fix linting issue * align with latest commit from Redis dependency * fix on RedisModulesSDK * do not rely on TimerID for initialization state * handle ASM Decrease Query Count * fix access to keyspace version * fix: remove ASM count from AREQ_Free * more progress towards keyspace query counting * handle some small PR comments * avoid cplusplus code duplication * align test with no cplusplus deduplication * add helper to identify leaks of query counts on sanitizer * add keyspaceversion to BockQueryNode * control leaks of cursor query count when queries fail * check if is hybrid request from AREQ itself * explain better the should_filter_slots * refactor how to handle topology timer reset * remove decrease of count on cursor Read * align with dedicated API for cluster enable/disable trimming * make query cursor tracker map thread safe * remove some changes * handle Decrease of Query Count for each AREQ individually in a RAII manner * small cleanup of PR * fix leak: decrease without previous increase * add log to debug * change in github workflow * fix a test * Apply suggestions from code review * all ranges are owned in StandAlone cluster * remove txt from pytests * all slots dynamic allocation * fix compilation warning * register new API to c_symbols.rs * proper mutex lifetime management * degine mutex as extern to avoid duplication in translation units * reschedule with slots * fix: fix potential leak and correct log message condition * reset slots when local slots are updated * fix: accept not normlized slot ranges as input * Revert "fix: accept not normlized slot ranges as input" This reverts commit 1babfed. * add configuration for max min delay trimming ms * Apply suggestions from code review Co-authored-by: GuyAv46 <[email protected]> * apply some suggestion changes * fix test acl * proper verbose level * remove inlining and create struct in notifications * make config underscore * add comments in code and fix typo * add configuration for delay check ms * avoid trim delay if no Indices found (no module usage) * Update tests/pytests/test_hybrid_internal.py Co-authored-by: GuyAv46 <[email protected]> * refactor asm state machine sanitizer code * ASM_StartTrim properly * refactor test_hybrid_internal * small refactor test asm * add tests for FT AGGREGATE WITHCURSORS * fix naming to be private of config * test: handle test where cursor returns empty result after trimming starts * add proper trimmed tests * adapt test asm * fix use after free connection * use arr.h to avoid reallocating in ASM Sanitizer * Apply suggestions from code review Co-authored-by: GuyAv46 <[email protected]> * test against redis unstable * update RedisModulesSDK * fix how to pass hybrid query vectors in query * fix .gitmodules back * update RedisModulesSDK * proper error handling * clarify invariants with an ASSERT * skip while no fix is found * do not assert, stop timers if they are scheduled * reply to cursor comments * small refactoring notifications * improve logging * add test for empty indices * test: improve testing * small fix * small test change * include timeout message * test no indexes * adapt changes * make test longer to cover timers * fix test * fix: avoid memory leak of ever growing dict * Update src/asm_state_machine.h Co-authored-by: GuyAv46 <[email protected]> --------- Co-authored-by: GuyAv46 <[email protected]> (cherry picked from commit a3f5468)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 19, 2026
…7829) * [MOD-12171] [MOD-12169] Implement ASM-aware search flow (#7343) * fix: fix counts in all variables * small refactor * some refactoring of Shared Exclusive Lock * simplify * protect GILOwned simple bool * clarify some comments * set GILAlternativeLockHeld to true properly * fix spelling * add assertion * fix: fix import RS_LOG_ASSERT * test: add more conditions to testing * add some more logic * improve testing to proper signal main thread can finish while other threads may be waiting for the Shared Lock in the loop * test: handl test properly * add another pattern of tests * fix comments from cursor * fix concurrency bug * fix: fix potential race condition at release lock time * fix: add condition * test: add more testing * force testing further to capture more potential errors * parametrize tests * test: make tests a little faster * test: add micro benchmark * fix compile microbenchmarks * fix: avoid potential reentrant deadlocks * fix: avoid potential reentrant deadlocks * test: avoid leak in test * test: avoid leak in test * fix: fix assertion * fix: fix assertion * Simplify Shared Lock internals (#7267) * simplify shared lock * small improvement to set_timeout * fix comment * fix nd improve comments * condition fix * remove lock type from release API * Add lock type back to the release API * remove Unlocked from enum and handle clock init for macOS * adapt to use new API * chnange according to comments * handle PR comments * handle PR comments * fix: take shared lock in other cases * change as PR comments * test: simplify test, do not allow query errors * fix tests as per comments * fix: handle number of high priority jobs running * fix: fix test comment * add ASM to help slot tracking in notifications * test: add some testing idea * change as per PR comments * compile and link test fix * change draining method to drain high priority * add ctests for ASM State Machine * test: add ASM tests * test: complete tests * remove SharedSlotRangeArray from code * implement basic logic * change as per PR comments * fix: remove latest SlotRange reference * set local slots on UpdateTopology * change as per PR comments * fix: fix query error * some code changes needed * handle tests * fix compilation in Linux * fix loading func in module * fix tests hybrid internal standalone * fix test standalone acl * fix test standalone asm * test: handle tests internal to different * handle more tests slowly * improve testing * improve testing * fix sanitizer errors in tests * remove ASM comment * small improvement commenting * schedule refresh topology in timer * remove println * test: fix test ASM * test: improve ASM test * further improve * improve asm tests * fix formatting issue * try to handle hybrid test * test: change test to compare scores * improve testing a little * add micro benchmarks with jobs in threads * fix: fail on error from coordinator * small changes in test asm * remove changes not wanted * checkout redis feature branch in task test * small change * remove draining * move atomic to new header * remove _internal naming * fix formatting * add structure of timers * add initial version of KeySpaceVersionTracker * implementation and unit testing of KeySpaceVersionTracker * Plugin keyspace version tracker counters into the flow * small code comment * align with trimming redismodule.h * install feature redis in CI workflow * plug new RedisModule API to delay trimming * remove sleep from test * fix values and enable logging * make query version map extern to avoid different instances per compilation unit * rename vars * small cleanup and rename * use debug print * query all shards * fix linting issue * align with latest commit from Redis dependency * fix on RedisModulesSDK * do not rely on TimerID for initialization state * handle ASM Decrease Query Count * fix access to keyspace version * fix: remove ASM count from AREQ_Free * more progress towards keyspace query counting * handle some small PR comments * avoid cplusplus code duplication * align test with no cplusplus deduplication * add helper to identify leaks of query counts on sanitizer * add keyspaceversion to BockQueryNode * control leaks of cursor query count when queries fail * check if is hybrid request from AREQ itself * explain better the should_filter_slots * refactor how to handle topology timer reset * remove decrease of count on cursor Read * align with dedicated API for cluster enable/disable trimming * make query cursor tracker map thread safe * remove some changes * handle Decrease of Query Count for each AREQ individually in a RAII manner * small cleanup of PR * fix leak: decrease without previous increase * add log to debug * change in github workflow * fix a test * Apply suggestions from code review * all ranges are owned in StandAlone cluster * remove txt from pytests * all slots dynamic allocation * fix compilation warning * register new API to c_symbols.rs * proper mutex lifetime management * degine mutex as extern to avoid duplication in translation units * reschedule with slots * fix: fix potential leak and correct log message condition * reset slots when local slots are updated * fix: accept not normlized slot ranges as input * Revert "fix: accept not normlized slot ranges as input" This reverts commit 1babfed. * add configuration for max min delay trimming ms * Apply suggestions from code review Co-authored-by: GuyAv46 <[email protected]> * apply some suggestion changes * fix test acl * proper verbose level * remove inlining and create struct in notifications * make config underscore * add comments in code and fix typo * add configuration for delay check ms * avoid trim delay if no Indices found (no module usage) * Update tests/pytests/test_hybrid_internal.py Co-authored-by: GuyAv46 <[email protected]> * refactor asm state machine sanitizer code * ASM_StartTrim properly * refactor test_hybrid_internal * small refactor test asm * add tests for FT AGGREGATE WITHCURSORS * fix naming to be private of config * test: handle test where cursor returns empty result after trimming starts * add proper trimmed tests * adapt test asm * fix use after free connection * use arr.h to avoid reallocating in ASM Sanitizer * Apply suggestions from code review Co-authored-by: GuyAv46 <[email protected]> * test against redis unstable * update RedisModulesSDK * fix how to pass hybrid query vectors in query * fix .gitmodules back * update RedisModulesSDK * proper error handling * clarify invariants with an ASSERT * skip while no fix is found * do not assert, stop timers if they are scheduled * reply to cursor comments * small refactoring notifications * improve logging * add test for empty indices * test: improve testing * small fix * small test change * include timeout message * test no indexes * adapt changes * make test longer to cover timers * fix test * fix: avoid memory leak of ever growing dict * Update src/asm_state_machine.h Co-authored-by: GuyAv46 <[email protected]> --------- Co-authored-by: GuyAv46 <[email protected]> (cherry picked from commit a3f5468) * align * fix compilation * ci: fix * update RedisModulesSDK * remove unnecessary imports * adapt to get 8.4 in checkout * test: add test that ft hybrid does not accept slots info multiple times * add dummy step in ci * clean TODO * Fix C++ compilation error by removing unused <stdatomic.h> include --------- Co-authored-by: GuyAv46 <[email protected]>
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
3 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.
Describe the changes in the pull request
Mostly adding comments and reorganize, and some renaming
MONOTONIC_RAWfor better performancePerformance
Mark if applicable
Note
Redesigns the shared/exclusive lock with a simplified lend/borrow API and monotonic timed waits, updating workers, result processing, tests, and benchmarks to the new interface.
GILAvailable,GILIsBorrowed); timeouts useCLOCK_MONOTONIC_RAW.shared_exclusive_lock.h:SharedExclusiveLock_SetOwned/UnsetOwned->SharedExclusiveLock_LendGIL/TakeBackGIL.SharedExclusiveLock_Acquire(ctx, bool)->SharedExclusiveLock_Acquire(ctx)returning{Unlocked, Owned, Borrowed}.SharedExclusiveLockTypeenum and release semantics accordingly.result_processor.c: Safe loader acquires/release via newSharedExclusiveLock_Acquire/Releasesignature.workers.c: UseLendGIL/TakeBackGIL; yield path assertsBorrowed.Written by Cursor Bugbot for commit 4f5a535. This will update automatically on new commits. Configure here.