Skip to content
This repository has been archived by the owner on Jul 6, 2020. It is now read-only.

Fix operation being referred to but undefined #70

Merged
merged 1 commit into from
Sep 9, 2019

Conversation

kitten
Copy link
Member

@kitten kitten commented Sep 9, 2019

Fix #69

This PR shall be known as the "Concurrency is Hard"-PR

This is an interesting issue.
We thought it was impossible to be referring to the same
operation twice, since we're deduplicating for the operation
key when processing dependencies.
What we're not accounting for is that the operation may have
been reexecuted by another write to the cache.

This may happen in the following condition:

  • A result comes back and a write operation is performed. Dependencies
    are stored as usual
  • Another result comes back and triggers dependencies to be processed,
    which deletes some of the previous dependencies
  • Meanwhile a potential third write starts which expects to see a
    deleted dependency, but it has already been reexecute by the second
    write operation.

This is an interesting issue.
We thought it was impossible to be referring to the same
operation twice, since we're deduplicating for the operation
key when processing dependencies.
What we're not accounting for is that the operation may have
been reexecuted by another write to the cache.

This may happen in the following condition:

- A result comes back and a write operation is performed. Dependencies
  are stored as usual
- Another result comes back and triggers dependencies to be processed,
  which deletes some of the previous dependencies
- Meanwhile a potential third write starts which expects to see a
  deleted dependency, but it has already been reexecute by the second
  write operation.
@kitten kitten requested a review from JoviDeCroock September 9, 2019 22:54
Copy link

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must have really been a pain to find out, great work as always man! Was already wondering a bit what this could be. 💯

@kitten kitten merged commit 07e6b74 into master Sep 9, 2019
@kitten kitten deleted the fix/concurrent-dependencies branch September 9, 2019 22:57
@kitten kitten restored the fix/concurrent-dependencies branch September 9, 2019 22:57
@kitten kitten deleted the fix/concurrent-dependencies branch September 9, 2019 22:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash on subsequent run of multiple 'concurrent' queries
2 participants