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

update_lock not race-condition-safe #168

Open
timobrembeck opened this issue Feb 22, 2023 · 3 comments
Open

update_lock not race-condition-safe #168

timobrembeck opened this issue Feb 22, 2023 · 3 comments

Comments

@timobrembeck
Copy link
Contributor

The update_lock uses the threading-library to avoid race conditions between the asynchronous post-save signal listeners:

# A global lock, showing whether linkcheck is busy
update_lock = threading.Lock()

with update_lock:

However, when Django is deployed in a production environment using multiple processes, this might not be race-condition-safe since each process has its own lock.

An alternative might be django-db-mutex (https://django-db-mutex.readthedocs.io/), which achieves the synchronization via a separate database model. There might be environments where more performant solutions are available (e.g. a redis cache etc.), but the least common denominator might simply be the database.

@claudep
Copy link
Contributor

claudep commented Feb 23, 2023

I think adding django-db-mutex dependency is reasonable to solve this.

@timobrembeck
Copy link
Contributor Author

Ah, I just noticed that this package does not implement a waiting lock, it will just throw an exception when the lock is currently in use. Adding a retrying-mechanism (by adding the failed task back to the queue) would also be possible, but I wonder whether there is an easier way?

multiprocessing.Lock() seems to be unsuitable because it's hard to share the lock resource between the processes that are spawned by the web server, right?

Do you think we could somehow abuse select_for_update() for this task?
If I understand it correctly, this would only be supported by postgresql, oracle, and mysql db backends... is that a problem?

Another alternative seems to be django-cache-lock, but the caveat is that all dependent applications would have to use a caching backend (and the LocMemCache is not shared between processes, so this is a non-trivial setup)...

@claudep
Copy link
Contributor

claudep commented Feb 26, 2023

I think it will be hard to find a process-safe locking mechanism working in all situations. I have no better solution to suggest for now. django-db-mutex with a retry mechanism or extracting the django-db-mutex functionality we need might be the way to go.

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 a pull request may close this issue.

2 participants