Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CQ: Fix entry missing from cache leading to crash on read (backport #11288) #11292

Merged
merged 1 commit into from
May 21, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented May 21, 2024

The issue comes from a mechanic that allows us to avoid writing to disk when a message has already been consumed. It works fine in normal circumstances, but fan-out makes things trickier.

When multiple queues write and read the same message, we could get a crash. Let's say queues A and B both handle message Msg.

  • Queue A asks store to write Msg
  • Queue B asks store to write Msg
  • Queue B asks store to delete Msg (message was immediately consumed)
  • Store processes Msg write from queue A
    • Store writes Msg to current file
  • Store processes Msg write from queue B
    • Store notices queue B doesn't need Msg anymore; doesn't write
    • Store clears Msg from the cache
  • Queue A tries to read Msg
    • Msg is missing from the cache
    • Queue A tries to read from disk
    • Msg is in the current write file and may not be on disk yet
    • Crash

The problem is that the store clears Msg from the cache. We need all messages written to the current file to remain in the cache as we can't guarantee the data is on disk when comes the time to read. That is, until we roll over to the next file.

The issue was that a match was wrong, instead of matching a single location from the index, the code was matching against a list. The error was present in the code for almost 13 years since commit 2ef30dc.

Types of Changes

The issue comes from a mechanic that allows us to avoid writing
to disk when a message has already been consumed. It works fine
in normal circumstances, but fan-out makes things trickier.

When multiple queues write and read the same message, we could
get a crash. Let's say queues A and B both handle message Msg.

* Queue A asks store to write Msg
* Queue B asks store to write Msg
* Queue B asks store to delete Msg (message was immediately consumed)
* Store processes Msg write from queue A
  * Store writes Msg to current file
* Store processes Msg write from queue B
  * Store notices queue B doesn't need Msg anymore; doesn't write
  * Store clears Msg from the cache
* Queue A tries to read Msg
  * Msg is missing from the cache
  * Queue A tries to read from disk
  * Msg is in the current write file and may not be on disk yet
  * Crash

The problem is that the store clears Msg from the cache. We need
all messages written to the current file to remain in the cache
as we can't guarantee the data is on disk when comes the time
to read. That is, until we roll over to the next file.

The issue was that a match was wrong, instead of matching a single
location from the index, the code was matching against a list. The
error was present in the code for almost 13 years since commit
2ef30dc.

(cherry picked from commit 93e4e88)
@michaelklishin michaelklishin added this to the 3.13.3 milestone May 21, 2024
@michaelklishin michaelklishin merged commit 6dbe4de into v3.13.x May 21, 2024
8 of 10 checks passed
@michaelklishin michaelklishin deleted the mergify/bp/v3.13.x/pr-11288 branch May 21, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants