Repository navigation
[6.x] Lock the Comb index file across each read-modify-write - #15725
Merged
duncanmcclean merged 3 commits intoOct 27, 2026
Merged
Conversation
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
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.
Comb\Index::insertDocuments()anddelete()read the whole index file, change it in memory, and write it back, with nothing held between the read and the write.lock: trueonget/put(#10695) only stops torn reads of a single call. With more than one queue worker,php please search:updatedispatches oneInsertMultipleJobper chunk and two workers can both read{a}, then write{a, b}and{a, c}; whichever write lands last silently drops the other chunk. An entry save racing a reindex chunk does the same.Reproduced on a 2,001-entry collection,
chunk_size10, Redis queue, fourqueue:workprocesses started before the jobs are dispatched, counting documents indefault.jsonafterwards (script: https://gist.github.com/edalzell/f2e053aef66515967c4c340e95cd6534):No job failed in any run, and each worker processed roughly a quarter of the 201 jobs both before and after.
Fix wraps each read-modify-write in a blocking cache lock (
Cache::lock('search-index-{name}', 60)->block(30, ...)), which waits up to 30 seconds for the lock and releases it once the read and write finish, even if one throws. The lock only spans processes when every worker shares a lock-capable cache store (Redis, database, file, etc.). With thearrayornulldriver, it doesn't protect anything and you get today's behaviour. Tests assert the lock is held from before the read until after the write for insert and delete.