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

Defer FPRule counter updates during normalization #2007

Open
wants to merge 2 commits into
base: qa/1.x
Choose a base branch
from

Conversation

sevein
Copy link
Member

@sevein sevein commented Oct 31, 2024

This pull request introduces a context manager (DeferredFPRuleCounter) to defer the tracked FPRule execution until the call function completes the execution of the entire batch, reducing the risk of deadlocks when multiple MCPClients attempt to update the same FPRule concurrently.

This change helps prevent deadlocks, but keep in mind that normalize.py might be reworked later to avoid using long database transactions altogether.

Tentative fix for archivematica/Issues#752 and archivematica/Issues#1525.
Relates to archivematica/Issues#1161.

@sevein sevein force-pushed the dev/issue-752-fprule-counter-deadlock-fix branch from 3aadaa8 to 09ed00e Compare November 1, 2024 07:24
@sevein sevein requested a review from a team November 7, 2024 10:10
@sevein sevein marked this pull request as ready for review November 7, 2024 10:10
@sevein
Copy link
Member Author

sevein commented Nov 7, 2024

An interesting aspect of this pull request is that I've never been able to reproduce the original issue, so I'm uncertain if this change is necessary or whether it solves the problem. I've tested multiple batch sizes, number of worker replicas and transfers with hundreds of files unsuccessfully, but the issue has occasionally been experienced by both customers and the Artefactual team. Maybe we can keep this work around until needed.

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.

1 participant