fix: remove draining from Flush callback#7420
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7420 +/- ##
==========================================
+ Coverage 84.85% 84.87% +0.02%
==========================================
Files 346 347 +1
Lines 53208 53716 +508
Branches 13935 14292 +357
==========================================
+ Hits 45148 45592 +444
- Misses 7866 7928 +62
- Partials 194 196 +2
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:
|
alonre24
left a comment
alonre24
left a comment
There was a problem hiding this comment.
What would happen if we call flushall while jobs are pending (due to not draining), and then we immediately create a new index and push new jobs? Should we test such a scenario?
Okey, but I guess the expected behavior is that the queries that were queued before and during FLUSHALL will fail and the subsequent ones should succeed right? |
What seems weird to me is that we are draining but after we have freed spectdict_g, so isn't there a chance that one of the workers could try to access an invalid struct? |
JoanFM
force-pushed
the
joan-remove-flush-drain
branch
from
244ee7f to
22c2d75
Compare
yes. |
Only the main thread is allowed to access the |
| from common import * | ||
|
|
||
| @skip(cluster=True) | ||
| def test_query_while_flush(env): |
There was a problem hiding this comment.
We should create an env with workers, no?
Also consider using getWorkersThpoolStats for validation
There was a problem hiding this comment.
oh sure, will create with WORKERS
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.8
git worktree add -d .worktree/backport-7420-to-2.8 origin/2.8
cd .worktree/backport-7420-to-2.8
git switch --create backport-7420-to-2.8
git cherry-pick -x e56bf5bd7abca36aad32d6cae4c1d433c288ae3a |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 2.10
git worktree add -d .worktree/backport-7420-to-2.10 origin/2.10
cd .worktree/backport-7420-to-2.10
git switch --create backport-7420-to-2.10
git cherry-pick -x e56bf5bd7abca36aad32d6cae4c1d433c288ae3a |
* fix: remove draining from Flush callback * test: add test for query while flushall * improve testing with workers * improve test (cherry picked from commit e56bf5b)
|
Successfully created backport PR for |
* fix: remove draining from Flush callback * test: add test for query while flushall * improve testing with workers * improve test (cherry picked from commit e56bf5b)
|
Successfully created backport PR for |
* fix: remove draining from Flush callback * test: add test for query while flushall * improve testing with workers * improve test (cherry picked from commit e56bf5b)
* fix: remove draining from Flush callback * test: add test for query while flushall * improve testing with workers * improve test (cherry picked from commit e56bf5b)
* fix: remove draining from Flush callback * test: add test for query while flushall * improve testing with workers * improve test (cherry picked from commit e56bf5b)
* fix: remove draining from Flush callback (#7420) * fix: remove draining from Flush callback * test: add test for query while flushall * improve testing with workers * improve test (cherry picked from commit e56bf5b) * skip test on sanitizer --------- Co-authored-by: Joan Fontanals <[email protected]>
* fix: remove draining from Flush callback (#7420) * fix: remove draining from Flush callback * test: add test for query while flushall * improve testing with workers * improve test (cherry picked from commit e56bf5b) * skip test on sanitizer --------- Co-authored-by: Joan Fontanals <[email protected]>
3 participants
Describe the changes in the pull request
Remove Draining from onFlush callback
Note
Stop draining the workers thread pool in
onFlushand add a test that exercises querying duringFLUSHALLand after creating a new index.workersThreadPool_Drain(ctx, 0)fromonFlushinsrc/spec.c.tests/pytests/test_query_while_flush.pyto continuously query an index before/afterFLUSHALL, verify expected errors post-flush, and validate new index queries succeed.Written by Cursor Bugbot for commit 3b10983. This will update automatically on new commits. Configure here.