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

Actively notfiy a job of an abort request #1084

Open
medihack opened this issue Jun 25, 2024 · 3 comments
Open

Actively notfiy a job of an abort request #1084

medihack opened this issue Jun 25, 2024 · 3 comments
Labels
Issue appropriate for: Occasional contributors 😉 This issue will be best tackled by people with a minimum of experience on the project Issue contains: Some Python 🐍 This issue involves writing some Python code Issue contains: Some SQL 🐘 This features require changing the SQL model Issue status: Blocked ⛔️ Issue cannot be processed for now Issue type: Feature ⭐️ Add a new feature that didn't exist before

Comments

@medihack
Copy link
Member

medihack commented Jun 25, 2024

With PR #1081, we added a feature to mark a currently running job for abortion, but we have to poll the database in the task ourselves to determine whether this mark was set (represented by the aborting status). As discussed in #511, it would be nice if the task was actively notified of an abortion (without the need for intense database polling) using a PostgreSQL listen-notify. We could then raise an asyncio.CancelledError in an async task. It's unclear what to do with a sync task (maybe a callback method like context.listen_abort(cb: Callable?!). It should be optional and triggered by something like job_manager.cancel_job_by_id(33, abort=True, notify=True).
It would be best implemented after the worker refactoring #933, as it must be integrated into the worker class.

@medihack medihack changed the title Actively notfiy jobs of an abort request Actively notfiy a job of an abort request Jun 25, 2024
@ewjoachim
Copy link
Member

It would be best implemented after the worker refactoring #933

Or be part of the design of #933

@ewjoachim ewjoachim added Issue contains: Some SQL 🐘 This features require changing the SQL model Issue type: Feature ⭐️ Add a new feature that didn't exist before Issue contains: Some Python 🐍 This issue involves writing some Python code Issue appropriate for: Occasional contributors 😉 This issue will be best tackled by people with a minimum of experience on the project Issue status: Blocked ⛔️ Issue cannot be processed for now labels Jun 25, 2024
@ewjoachim
Copy link
Member

If we do #1083, is this worth the complexity ? I won't close this but I think it's interesting to spell out the gains we expect from this before tackling it.

@medihack
Copy link
Member Author

In some cases, it could simplify the abortion process and save the user some code (e.g., no need for another asyncio task that monitors the abort status). On the other hand, it's not a feature that needs to have high priority or be integrated right now as part of the worker refactoring (the refactoring is complicated enough anyway). It's more a nice to have feature and something I have not seen in any other distributed task queue library. So I would say, let us leave this issue open and see what the future brings. Regarding #1083, it's the next thing I'll deal with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue appropriate for: Occasional contributors 😉 This issue will be best tackled by people with a minimum of experience on the project Issue contains: Some Python 🐍 This issue involves writing some Python code Issue contains: Some SQL 🐘 This features require changing the SQL model Issue status: Blocked ⛔️ Issue cannot be processed for now Issue type: Feature ⭐️ Add a new feature that didn't exist before
Projects
None yet
Development

No branches or pull requests

2 participants