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

Add @tree_mutex() decorator and trick treebeard into using transactions #3076

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

PeterNerlich
Copy link
Collaborator

@PeterNerlich PeterNerlich commented Sep 25, 2024

Short description

Force treebeard to use transactions. This should remove any race conditions caused by the library. The tests seem promising.

Proposed changes

  • Add @tree_mutex() decorator to replace @django.db.transactions.atomic (still uses it inside in addition to the mutex implementation)
    • Monkeypatch treebeards Node class to use djangos database cursor for its raw SQL queries so that changes get rolled back on failure
  • Add tests for @tree_mutex()

Side effects

  • The system might respond slower when many page move operations are attempted simultaneously, because treebeard now waits with each operation until any previous operations are finished

Resolved issues

Fixes: #1518, possibly #1716


Pull Request Review Guidelines

Copy link

codeclimate bot commented Sep 25, 2024

Code Climate has analyzed commit b43b88a and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 93.4% (50% is the threshold).

This pull request will bring the total coverage in the repository to 82.5% (0.0% change).

View more on Code Climate.

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.

Race conditions in tree operations causing Internal Server Errors
1 participant