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

lock is released when the task is skipped #11

Closed
eur00t opened this issue Nov 2, 2021 · 4 comments · Fixed by #12
Closed

lock is released when the task is skipped #11

eur00t opened this issue Nov 2, 2021 · 4 comments · Fixed by #12

Comments

@eur00t
Copy link
Contributor

eur00t commented Nov 2, 2021

Hi maintainers,

In the current implementation, if the task is running, the first concurrent task removes the lock, so the next one after that will come through.
Is this the intended behaviour?

I think the lock should prevent any concurrent tasks from executing, so the goal is to only run one task at any given time.

@artemistomaras
Copy link
Owner

Hi,

The intented behaviour is to indeed prevent any concurrent tasks from executing.

If the task is running, the first concurrent task that runs, executes the following statement:
https://github.com/artemistomaras/django-ethereum-events/blob/master/django_ethereum_events/tasks.py#L59
thus it does not remove the lock. (acquired=False since cache.add fails).

Can you verify this against the output of your celery worker?

@artemistomaras
Copy link
Owner

@eur00t I think you are indeed right.

Received task: app.tasks.interledger_scheduler_new[90d633bf-6f40-4ea9-b0c8-d0f9d9a45f18] (long running task)...
Received task: django_ethereum_events.tasks.event_listener[e8e44261-eee6-4836-b2be-96511c1be0a7] (first concurrent task)
Event listener is already running. Skipping execution. (this is printed by the first concurrent task and the it releases the lock **MISTAKE**)
(now here the second concurrent task will run while the first task is also running and set the lock again)

@eur00t
Copy link
Contributor Author

eur00t commented Nov 2, 2021

@artemistomaras thanks for swift reply. I can confirm: I also see this in my celery log.

@artemistomaras
Copy link
Owner

I released version 4.1.1.

Thank you for your contribution.

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