lock before opening indexes if info - [MOD-10007] [MOD-9761] by BenGoldberger · Pull Request #6279 · RediSearch/RediSearch

BenGoldberger · 2025-06-08T13:52:17Z

Move the locking of the spec in IndexInfoCommand->fillReplyWithIndexInfo
Hopefully this will fix the bug and some flaky tests

Plus handle the while loop in FGC_recvFixed, that seems to block the gc thread

codecov · 2025-06-08T14:21:59Z

Codecov Report

Attention: Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.

Project coverage is 88.77%. Comparing base (4940349) to head (6733c68).
Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/fork_gc.c	70.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6279      +/-   ##
==========================================
- Coverage   88.78%   88.77%   -0.01%     
==========================================
  Files         243      243              
  Lines       40893    40900       +7     
  Branches     3483     3483              
==========================================
+ Hits        36307    36311       +4     
- Misses       4543     4546       +3     
  Partials       43       43

Flag	Coverage Δ
flow	`82.20% <57.14%> (-0.12%)`	⬇️
unit	`46.31% <66.66%> (+<0.01%)`	⬆️

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.

src/info/info_command.c

…ch/RediSearch into beng-lock-speck-in-info

tests/pytests/test_gc.py

src/gc.c

tests/pytests/test_gc.py

…info

alonre24

💪

kei-nan · 2025-06-17T10:40:11Z

src/fork_gc.c

 // Buff shouldn't be NULL.
 static void FGC_sendFixed(ForkGC *fgc, const void *buff, size_t len) {
  RS_LOG_ASSERT(len > 0, "buffer length cannot be 0");
-  ssize_t size = write(fgc->pipefd[GC_WRITERFD], buff, len);


@BenGoldberger how are these GC changes related to the info changes and the locking placement?

If they are not, then maybe move them to a different PR?

It a small fix to an issue that came up while testing this, so I think it fine to include it here.

The GC changes are not related directly, but they are needed to deflake tests. We could move it to a different PR, but since it is a small fix and PR is ready, I suggest we leave it as is. @BenGoldberger, please update the PR description and the MOD tickets that are related

redisearch-backport-pull-request · 2025-06-17T12:40:39Z

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-6279-to-2.8 origin/2.8
cd .worktree/backport-6279-to-2.8
git switch --create backport-6279-to-2.8
git cherry-pick -x 52cbc9c53a83f1cefcf2807ab4fc06fac5c1ca77

redisearch-backport-pull-request · 2025-06-17T12:40:41Z

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-6279-to-2.10 origin/2.10
cd .worktree/backport-6279-to-2.10
git switch --create backport-6279-to-2.10
git cherry-pick -x 52cbc9c53a83f1cefcf2807ab4fc06fac5c1ca77

redisearch-backport-pull-request · 2025-06-17T12:40:42Z

Backport failed for 8.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 8.0
git worktree add -d .worktree/backport-6279-to-8.0 origin/8.0
cd .worktree/backport-6279-to-8.0
git switch --create backport-6279-to-8.0
git cherry-pick -x 52cbc9c53a83f1cefcf2807ab4fc06fac5c1ca77

* move the lock to include the opening of the indexes * add pytest * try to fix test * fix test * move lock sooner * spell checker * try to fix test * pr changes * pr changes * remove debugPrint in test * remove debug prints * add gc invokes while deleting docs * add timeout before read * add the timeout to gc struct * change timeout to 3 min * try to change the while * switch to poll * refactor gc struct * add gc_wait in test * pr changes * pr changes * pr changes * pr change * try to fix leak in test * revert priority change * remove deps changes * pr changes * revert len check (cherry picked from commit 52cbc9c)

…6279) (#6338) lock before opening indexes if info - [MOD-10007] (#6279) * move the lock to include the opening of the indexes * add pytest * try to fix test * fix test * move lock sooner * spell checker * try to fix test * pr changes * pr changes * remove debugPrint in test * remove debug prints * add gc invokes while deleting docs * add timeout before read * add the timeout to gc struct * change timeout to 3 min * try to change the while * switch to poll * refactor gc struct * add gc_wait in test * pr changes * pr changes * pr changes * pr change * try to fix leak in test * revert priority change * remove deps changes * pr changes * revert len check (cherry picked from commit 52cbc9c)

…6279) (#6340) lock before opening indexes if info - [MOD-10007] (#6279) * move the lock to include the opening of the indexes * add pytest * try to fix test * fix test * move lock sooner * spell checker * try to fix test * pr changes * pr changes * remove debugPrint in test * remove debug prints * add gc invokes while deleting docs * add timeout before read * add the timeout to gc struct * change timeout to 3 min * try to change the while * switch to poll * refactor gc struct * add gc_wait in test * pr changes * pr changes * pr changes * pr change * try to fix leak in test * revert priority change * remove deps changes * pr changes * revert len check (cherry picked from commit 52cbc9c)

…6279) (#6339) lock before opening indexes if info - [MOD-10007] (#6279) * move the lock to include the opening of the indexes * add pytest * try to fix test * fix test * move lock sooner * spell checker * try to fix test * pr changes * pr changes * remove debugPrint in test * remove debug prints * add gc invokes while deleting docs * add timeout before read * add the timeout to gc struct * change timeout to 3 min * try to change the while * switch to poll * refactor gc struct * add gc_wait in test * pr changes * pr changes * pr changes * pr change * try to fix leak in test * revert priority change * remove deps changes * pr changes * revert len check (cherry picked from commit 52cbc9c)

move the lock to include the opening of the indexes

014c82b

github-actions bot added the size:XS label Jun 8, 2025

BenGoldberger requested a review from kei-nan June 9, 2025 07:57

kei-nan reviewed Jun 9, 2025

View reviewed changes

src/info/info_command.c Outdated Show resolved Hide resolved

add pytest

0b0b45a

github-actions bot added the size:M label Jun 9, 2025

BenGoldberger added 3 commits June 9, 2025 17:27

try to fix test

dc66792

fix test

4bae1b6

move lock sooner

103ae92

BenGoldberger requested a review from alonre24 June 10, 2025 08:47

kei-nan approved these changes Jun 10, 2025

View reviewed changes

kei-nan previously approved these changes Jun 10, 2025

View reviewed changes

spell checker

63035ab

BenGoldberger dismissed kei-nan’s stale review via 63035ab June 10, 2025 09:02

BenGoldberger added 3 commits June 10, 2025 12:21

Merge branch 'master' into beng-lock-speck-in-info

29351a7

try to fix test

2acae02

Merge branch 'beng-lock-speck-in-info' of https://github.com/RediSear…

7a2c49d

…ch/RediSearch into beng-lock-speck-in-info

alonre24 reviewed Jun 10, 2025

View reviewed changes

tests/pytests/test_gc.py Show resolved Hide resolved

tests/pytests/test_gc.py Outdated Show resolved Hide resolved

tests/pytests/test_gc.py Outdated Show resolved Hide resolved

tests/pytests/test_gc.py Outdated Show resolved Hide resolved

BenGoldberger added 4 commits June 10, 2025 16:48

pr changes

a2a591f

pr changes

311e9b7

remove debugPrint in test

5ad159c

remove debug prints

9443a04

BenGoldberger requested a review from alonre24 June 11, 2025 06:44

BenGoldberger added 6 commits June 11, 2025 10:00

add gc invokes while deleting docs

6b41bec

add timeout before read

8125e52

add the timeout to gc struct

380b00d

change timeout to 3 min

35b29ec

try to change the while

ae12dae

switch to poll

d0e78d7

BenGoldberger added 2 commits June 15, 2025 23:43

pr change

11fe635

try to fix leak in test

2d9e2af

alonre24 reviewed Jun 16, 2025

View reviewed changes

src/gc.c Outdated Show resolved Hide resolved

alonre24 reviewed Jun 16, 2025

View reviewed changes

tests/pytests/test_gc.py Show resolved Hide resolved

alonre24 reviewed Jun 16, 2025

View reviewed changes

tests/pytests/test_gc.py Show resolved Hide resolved

BenGoldberger added 5 commits June 16, 2025 22:16

Merge remote-tracking branch 'origin/master' into beng-lock-speck-in-…

78ed624

…info

revert priority change

bd6b615

remove deps changes

8fa8c3d

pr changes

216840b

revert len check

6733c68

alonre24 approved these changes Jun 17, 2025

View reviewed changes

BenGoldberger requested a review from kei-nan June 17, 2025 09:35

kei-nan reviewed Jun 17, 2025

View reviewed changes

BenGoldberger added this pull request to the merge queue Jun 17, 2025

BenGoldberger added backport 2.8 backport 2.10 backport 8.0 labels Jun 17, 2025

Merged via the queue into master with commit 52cbc9c Jun 17, 2025
55 of 56 checks passed

BenGoldberger deleted the beng-lock-speck-in-info branch June 17, 2025 12:40

BenGoldberger changed the title ~~lock before opening indexes if info - [MOD-10007]~~ lock before opening indexes if info - [MOD-10007] [MOD-9761] Jun 17, 2025

Conversation

BenGoldberger commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alonre24 left a comment

Choose a reason for hiding this comment

Uh oh!

kei-nan Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kei-nan Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenGoldberger Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

alonre24 Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

redisearch-backport-pull-request bot commented Jun 17, 2025

Uh oh!

redisearch-backport-pull-request bot commented Jun 17, 2025

Uh oh!

redisearch-backport-pull-request bot commented Jun 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BenGoldberger commented Jun 8, 2025 •

edited

Loading

codecov bot commented Jun 8, 2025 •

edited

Loading

kei-nan Jun 17, 2025 •

edited

Loading

kei-nan Jun 17, 2025 •

edited

Loading