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

Race condition on updates in parallel with counter_cache or after_save #71

Open
pzac opened this issue Mar 28, 2018 · 5 comments
Open
Assignees
Labels
bug CRITICAL Critical issues

Comments

@pzac
Copy link
Contributor

pzac commented Mar 28, 2018

When a record has after_save hooks (or rails built-ins such as the counter cache) that update another object, and there are parallel updates that will affect the same object, there are regular failures with timeline consistency errors.
The entire callback chain should be wrapped in a transaction or have an exclusive lock on the dependent object.
#70 has a spec to reproduce the issue

@vjt vjt self-assigned this Mar 28, 2018
@vjt vjt added the bug label Mar 28, 2018
@vjt
Copy link
Contributor

vjt commented Mar 28, 2018

Thanks for the report. Will fix asap.

@pzac
Copy link
Contributor Author

pzac commented Mar 7, 2019

..or deadlocks
image

@vjt vjt closed this as completed in b57ff40 Apr 7, 2019
vjt added a commit that referenced this issue Apr 7, 2019
* parallel-updates-bug:
  Describe the race condition context [skip ci]
  Fix #71 via configuration
  Make Race spec self-contained
  Add failing spec
@vjt
Copy link
Contributor

vjt commented Apr 7, 2019

Sorry for the delay. The issue emerges because Active Record counter cache issues an UPDATE on the ChronoModel view, and this in turn runs the triggers for each row in your table.

This could be fixed by overriding Active Record counter cache behaviour, to issue the update on the temporal table only, but it would be another Monkey-Patch to maintain.

Further, given it is pointless to store changes to the counter cache in the history, please disable journaling on the counter cache column by using selective journaling, i.e.:

change_table :sections, temporal: true, no_journal: %w( articles_count )

I have added a section in the README to mention the requirement.

The spec is merged in b57ff40, and passes with the relevant no_journal option.

@pzac
Copy link
Contributor Author

pzac commented Apr 8, 2019

Then we'd lose the purpose and benefit of the counter cache, but this was an example: the same happens when an after_save callback updates another parent object

@pzac pzac reopened this Apr 8, 2019
@vjt
Copy link
Contributor

vjt commented Apr 8, 2019

Let me check in the spec whether the no_journal breaks counter cache in the history schema.

The Callbacks problem is a completely different one, that will be researched separately.

@vjt vjt changed the title Race conditions with parallel updates Race condition on updates in parallel with counter_cache or after_save Apr 8, 2019
@tagliala tagliala added the CRITICAL Critical issues label Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CRITICAL Critical issues
Projects
None yet
Development

No branches or pull requests

3 participants