MOD-13143: Solve Hybrid ASM Memory Leak by kei-nan · Pull Request #7903 · RediSearch/RediSearch

kei-nan · 2025-12-28T13:33:39Z

Describe the changes in the pull request

A clear and concise description of what the PR is solving, including:

Current: There is a memory leak in 8.4 in regards to FT.HYBRID and the use of GetLocalSlots
Change: Take the slots in the main thread and pass them in the concurrent ctx struct, clone them to each subquery so they will be destroyed correctly
Outcome: No memory leaks from the asm feature in regard to hybrid.

Mark if applicable

This PR introduces API changes
This PR introduces serialization changes

Note

Addresses leaks and thread-safety around cluster slot ranges in query execution.

Change AREQ_ApplyContext to accept localSlots; store on request and pass to pipeline; update all call sites
Capture Slots_GetLocalSlots() on main thread in concurrent context, expose via ConcurrentCmdCtx_GetSlotsMutable, and free after worker completion
Add Slots_Clone and ensure slots are cloned per hybrid subquery and freed in cleanup paths (hybrid exec/debug/parse)
Update FT.HYBRID flow to thread slot ranges through parseHybridCommand, HybridRequest_prepareForExecution, and execution cleanup
Adjust tests to pass and free slot ranges
Command info: change FILTER arg to a block with COUNT, POLICY (ADHOC|BATCHES), optional BATCH_SIZE; bump since to 8.4.4

^{Written by Cursor Bugbot for commit 669f47f. This will update automatically on new commits. Configure here.}

src/hybrid/hybrid_debug.c

src/hybrid/parse_hybrid.c

JoanFM · 2025-12-28T13:50:14Z

The implementation is ablut to change drastically, so I am not sure it is worth applying this change now and making the real ASM backport more complex

src/command_info/command_info.c

src/concurrent_ctx.c

src/concurrent_ctx.h

src/hybrid/parse_hybrid.c

tests/cpptests/test_cpp_parse_hybrid_iterators.cpp

GuyAv46 · 2025-12-28T15:07:28Z

Should be reverted in #7829. Let's add the revert commit once this PR is merged, so we don't forget to do so.
Plus, there is the command info stuff that I guess should not be reverted. @kei-nan can you handle the revert commit?

tests/cpptests/test_cpp_parsed_hybrid_pipeline.cpp

src/slot_ranges.c

src/slot_ranges.h

src/command_info/command_info.c

src/slot_ranges.c

codecov · 2025-12-28T17:03:10Z

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.06%. Comparing base (45628e4) to head (669f47f).
⚠️ Report is 3 commits behind head on 8.4.

Files with missing lines	Patch %	Lines
src/slot_ranges.c	85.71%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##              8.4    #7903   +/-   ##
=======================================
  Coverage   86.05%   86.06%           
=======================================
  Files         332      332           
  Lines       53683    53610   -73     
  Branches    11998    11998           
=======================================
- Hits        46198    46140   -58     
+ Misses       7318     7303   -15     
  Partials      167      167

Flag	Coverage Δ
flow	`84.68% <96.66%> (-0.11%)`	⬇️
unit	`52.39% <40.00%> (-0.03%)`	⬇️

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.

kei-nan · 2025-12-30T12:22:43Z

#7903 (comment)
The PR already removed part of the slots code
going to be a bit tricky to revert

GuyAv46 · 2025-12-30T12:30:02Z

@kei-nan right.
Maybe branch from 8.4, revert this PR, and then CP the rest of #7829.

This reverts commit fa4a8bb.

kei-nan · 2025-12-30T12:44:28Z

@kei-nan right. Maybe branch from 8.4, revert this PR, and then CP the rest of #7829.

I the end opened a PR in 8.4 which reverts this PR after talking to Joan.
we should merge that one before we merge Joan's PR.

**Bug Fixes:** * [redis#7385](RediSearch/RediSearch#7385) Fix high temporary memory consumption when loading multiple search indexes from RDB * [redis#7430](RediSearch/RediSearch#7430) Fix a potential deadlock in `FT.HYBRID` in cluster mode during updates. * [redis#7454](RediSearch/RediSearch#7454) Fix a garbage collection performence regression * [redis#7460](RediSearch/RediSearch#7460) Fix potential double-free in Fork GC error paths * [redis#7455](RediSearch/RediSearch#7455) Fix internal cursors not being deleted promptly in cluster mode * [redis#7667](RediSearch/RediSearch#7667) Fix a cursor logical leak upon dropping the index * [redis#7796](RediSearch/RediSearch#7796) Fix a potential use-after-free when removing connections * [redis#7792](RediSearch/RediSearch#7792) Fix string comparison for binary data with embedded NULLs in TOLIST reducer in FT.AGGREGATE * [redis#7823](RediSearch/RediSearch#7823) Update `FT.HYBRID` to accept vector blobs only via parameters * [redis#7903](RediSearch/RediSearch#7903) Fix a memory leak in Hybrid ASM * [redis#8052](RediSearch/RediSearch#8052) Fix `FT.HYBRID` behavior when used with `LOAD *` * [redis#8082](RediSearch/RediSearch#8082) Fix incorrect FULLTEXT field metric counts * [redis#8089](RediSearch/RediSearch#8089) Fix an edge case in `CLUSTERSET` handling * [redis#8152](RediSearch/RediSearch#8152) Fix configuration registration issues **Improvements:** * [redis#7427](RediSearch/RediSearch#7427) Enhance `FT.PROFILE` with vector search execution details * [redis#7431](RediSearch/RediSearch#7431) Ensure full `FT.PROFILE` output is returned on timeout with RETURN policy * [redis#7507](RediSearch/RediSearch#7507) Track timeout warnings and errors in INFO * [redis#7576](RediSearch/RediSearch#7576) Track OOM warnings and errors in INFO * [redis#7612](RediSearch/RediSearch#7612) Track `maxprefixexpansions` warnings and errors in INFO * [redis#7960](RediSearch/RediSearch#7960) Persist query warnings across cursor reads * [redis#7551](RediSearch/RediSearch#7551), [redis#7616](RediSearch/RediSearch#7616), [redis#7622](RediSearch/RediSearch#7622), [redis#7625](RediSearch/RediSearch#7625) Add runtime thread and pending-jobs metrics * [redis#7589](RediSearch/RediSearch#7589) Support multiple slot ranges in `search.CLUSTERSET` * [redis#7707](RediSearch/RediSearch#7707) Add `WITHCOUNT` support to `FT.AGGREGATE` * [redis#7862](RediSearch/RediSearch#7862) Add support for subquery `COUNT` in `FT.HYBRID` * [redis#8087](RediSearch/RediSearch#8087) Add warnings when cursor results may be affected by ASM and expose ASM warnings in `FT.PROFILE` * [redis#8049](RediSearch/RediSearch#8049) Add logging for index-related commands * [redis#8150](RediSearch/RediSearch#8150) Fix shard total profile time reporting in `FT.PROFILE`

**Bug Fixes:** * [#7385](RediSearch/RediSearch#7385) Fix high temporary memory consumption when loading multiple search indexes from RDB * [#7430](RediSearch/RediSearch#7430) Fix a potential deadlock in `FT.HYBRID` in cluster mode during updates. * [#7454](RediSearch/RediSearch#7454) Fix a garbage collection performence regression * [#7460](RediSearch/RediSearch#7460) Fix potential double-free in Fork GC error paths * [#7455](RediSearch/RediSearch#7455) Fix internal cursors not being deleted promptly in cluster mode * [#7667](RediSearch/RediSearch#7667) Fix a cursor logical leak upon dropping the index * [#7796](RediSearch/RediSearch#7796) Fix a potential use-after-free when removing connections * [#7792](RediSearch/RediSearch#7792) Fix string comparison for binary data with embedded NULLs in TOLIST reducer in FT.AGGREGATE * [#7704](RediSearch/RediSearch#7704) Use asynchronous jobs for GC in SVS to accelerate execution * [#7823](RediSearch/RediSearch#7823) Update `FT.HYBRID` to accept vector blobs only via parameters * [#7903](RediSearch/RediSearch#7903) Fix a memory leak in Hybrid ASM * [#8052](RediSearch/RediSearch#8052) Fix `FT.HYBRID` behavior when used with `LOAD *` * [#8082](RediSearch/RediSearch#8082) Fix incorrect FULLTEXT field metric counts * [#8089](RediSearch/RediSearch#8089) Fix an edge case in `CLUSTERSET` handling * [#8152](RediSearch/RediSearch#8152) Fix configuration registration issues **Improvements:** * [#7427](RediSearch/RediSearch#7427) Enhance `FT.PROFILE` with vector search execution details * [#7431](RediSearch/RediSearch#7431) Ensure full `FT.PROFILE` output is returned on timeout with RETURN policy * [#7507](RediSearch/RediSearch#7507) Track timeout warnings and errors in INFO * [#7576](RediSearch/RediSearch#7576) Track OOM warnings and errors in INFO * [#7612](RediSearch/RediSearch#7612) Track `maxprefixexpansions` warnings and errors in INFO * [#7960](RediSearch/RediSearch#7960) Persist query warnings across cursor reads * [#7551](RediSearch/RediSearch#7551), [#7616](RediSearch/RediSearch#7616), [#7622](RediSearch/RediSearch#7622), [#7625](RediSearch/RediSearch#7625) Add runtime thread and pending-jobs metrics * [#7589](RediSearch/RediSearch#7589) Support multiple slot ranges in `search.CLUSTERSET` * [#7707](RediSearch/RediSearch#7707) Add `WITHCOUNT` support to `FT.AGGREGATE` * [#7862](RediSearch/RediSearch#7862) Add support for subquery `COUNT` in `FT.HYBRID` * [#8087](RediSearch/RediSearch#8087) Add warnings when cursor results may be affected by ASM and expose ASM warnings in `FT.PROFILE` * [#8049](RediSearch/RediSearch#8049) Add logging for index-related commands * [#8150](RediSearch/RediSearch#8150) Fix shard total profile time reporting in `FT.PROFILE`

kei-nan added 2 commits December 28, 2025 15:29

initial commit

bda0bab

update tests

1eb5864

kei-nan requested review from GuyAv46 and JoanFM December 28, 2025 13:33

kei-nan self-assigned this Dec 28, 2025

github-actions bot added the size:M label Dec 28, 2025

cleanup

1e4a1f5

cursor bot reviewed Dec 28, 2025

View reviewed changes

src/hybrid/hybrid_debug.c Show resolved Hide resolved

src/hybrid/parse_hybrid.c Show resolved Hide resolved

fix leak + spellcheck

d930ddc

GuyAv46 reviewed Dec 28, 2025

View reviewed changes

src/command_info/command_info.c Show resolved Hide resolved

src/concurrent_ctx.c Show resolved Hide resolved

src/concurrent_ctx.h Outdated Show resolved Hide resolved

src/hybrid/parse_hybrid.c Show resolved Hide resolved

code review fixes + fix cpp tests

e273c5b

GuyAv46 previously approved these changes Dec 28, 2025

View reviewed changes

kei-nan requested a review from oshadmi December 28, 2025 15:05

cursor bot reviewed Dec 28, 2025

View reviewed changes

tests/cpptests/test_cpp_parse_hybrid_iterators.cpp Show resolved Hide resolved

fix sanitize compilation failure

bc42168

kei-nan dismissed GuyAv46’s stale review via bc42168 December 28, 2025 15:17

kei-nan added 2 commits December 28, 2025 17:39

fix sanitize compilation

30b692d

free slots in test

d036643

cursor bot reviewed Dec 28, 2025

View reviewed changes

tests/cpptests/test_cpp_parsed_hybrid_pipeline.cpp Show resolved Hide resolved

GuyAv46 previously approved these changes Dec 28, 2025

View reviewed changes

src/slot_ranges.c Outdated Show resolved Hide resolved

src/slot_ranges.h Outdated Show resolved Hide resolved

use correct functions

647cf7b

kei-nan dismissed GuyAv46’s stale review via 647cf7b December 28, 2025 15:55

another attempt to solve sanitize compilation

174d050

oshadmi reviewed Dec 28, 2025

View reviewed changes

src/command_info/command_info.c Show resolved Hide resolved

oshadmi reviewed Dec 28, 2025

View reviewed changes

src/slot_ranges.c Outdated Show resolved Hide resolved

GuyAv46 previously approved these changes Dec 28, 2025

View reviewed changes

change to relaxed memory ordering

f344f59

kei-nan dismissed GuyAv46’s stale review via f344f59 December 29, 2025 07:52

kei-nan enabled auto-merge December 29, 2025 07:53

revert command_info.c changes

1dafde7

github-actions bot added the size:S label Dec 29, 2025

update command_info.c

669f47f

oshadmi approved these changes Dec 29, 2025

View reviewed changes

GuyAv46 approved these changes Dec 29, 2025

View reviewed changes

kei-nan added this pull request to the merge queue Dec 29, 2025

Merged via the queue into 8.4 with commit fa4a8bb Dec 29, 2025
32 checks passed

kei-nan deleted the 8.4_jk_fix_hybrid_memory_leak branch December 29, 2025 10:46

kei-nan added a commit that referenced this pull request Dec 30, 2025

Revert "MOD-13143: Solve Hybrid ASM Memory Leak (#7903)"

8b72cef

This reverts commit fa4a8bb.

GuyAv46 mentioned this pull request Dec 30, 2025

Revert "MOD-13143: Solve Hybrid ASM Memory Leak (#7903)" #7938

Closed

alonre24 mentioned this pull request Jan 26, 2026

Update Redisearch module version to v8.4.5 redis/redis#14743

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MOD-13143: Solve Hybrid ASM Memory Leak#7903

MOD-13143: Solve Hybrid ASM Memory Leak#7903
kei-nan merged 13 commits into8.4from
8.4_jk_fix_hybrid_memory_leak

kei-nan commented Dec 28, 2025 •

edited by cursor bot

Loading

Uh oh!

Uh oh!

Uh oh!

JoanFM commented Dec 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GuyAv46 commented Dec 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Dec 28, 2025 •

edited

Loading

Uh oh!

Uh oh!

kei-nan commented Dec 30, 2025

Uh oh!

GuyAv46 commented Dec 30, 2025

Uh oh!

kei-nan commented Dec 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kei-nan commented Dec 28, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the changes in the pull request

Mark if applicable

Uh oh!

Uh oh!

Uh oh!

JoanFM commented Dec 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GuyAv46 commented Dec 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

kei-nan commented Dec 30, 2025

Uh oh!

GuyAv46 commented Dec 30, 2025

Uh oh!

kei-nan commented Dec 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kei-nan commented Dec 28, 2025 •

edited by cursor bot

Loading

codecov bot commented Dec 28, 2025 •

edited

Loading