You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "villebro (via GitHub)" <gi...@apache.org> on 2024/08/02 22:04:25 UTC
[I] [SIP] Global Async Task Framework [superset]
villebro opened a new issue, #29839:
URL: https://github.com/apache/superset/issues/29839
## [SIP] Proposal for Global Async Task Framework
### Motivation
*Note*: This replaces [[SIP-141] Global Async Queries 2.0](#29515) which aimed at completing [[SIP-39] Global Async Query Support](#9190).
### Proposed Change
Superset currently has varied and and often times opaque solutions for executing async operations (all require Celery):
- SQL Lab supports async query execution, and utilizes long polling for checking for results
- Thumbnails and Alerts & Reports are executed as background Celery tasks
- Chart queries can be executed async by enabling the GAQ feature flag, and supports both WebSocket and long polling
- Scheduled cron jobs (added via `CeleryConfig` in `superset_config.py`)
Currently none of the above support deduplication or cancellation of queued or running tasks, or even viewing which tasks are queued/executing. In addition, beyond sharing Celery as the execution backend, there is limited sharing of code, like utils or ORM models, which has lead to significant code duplication. This both increases the risk of regressions, limits reusability of good functionality and adds significant maintenance burden.
For this reason this SIP recommends adding a new Global Async Task Framework (GATF), which will introduce the following:
- A new ORM model with a menu which makes it possible to view and cancel queued or executing tasks. Admins will have access to all tasks, while regular users will only be able to view tasks they have spawned. This model will be ephemeral by nature, i.e. the task entries will be removed once they are completed.
- Add locking for all tasks to ensure deduplication. This applies particularly to async chart queries and thumbnails, which currently can cause significant resource waste.
- Deprecate long polling in both chart and SQL Lab queries - going forward only WebSockets would be supported.
### New or Changed Public Interfaces
A new ORM model would be introduced for async tasks with a string based identifier. When a new task is created, an entry is added to the table if it's not already there. For instance, for thumbnails, we would use the digest as the identifier. And for chart queries, we would use the cache key and so on. If the entry is already there, we consider the task already locked, and don't schedule a new one. In addition, there would be the following fields (non exhaustive list, mainly here to give a general picture of the model):
- created, updated etc (the basic stuff we have in other similar models)
- type (alerts & reports, sql lab query, chart query, thumbnail)
- status (queued, executing)
- payload (any necessary )
- last heartbeat (the executor thread would post a heartbeat here evey x seconds, ensuring that tasks that fail due to worker malfunction can be cleaned up)
We propose making WebSockets the sole mechanism for broadcasting task completion. This means that we will remove long polling support from async chart queries, and replace long polling in SQL Lab with WebSockets.
### New dependencies
None - however, we would likely make the WebSocket server mandatory for both SQL Lab and async chart queries.
### Migration Plan and Compatibility
**Phase 1 - GAQ**
In the first phase, we clean up GAQ by simplifying the architecture (see details about redundant form data cache keys etc from SIP-141) and remove long polling support. We also introduce the new ORM model, along with a UI, and migrate the GAQ metadata from Redis to the new ORM model.
At the end of phase one, we will have removed long polling from GAQ, and will have both chart query deduplication and cancellation support.
#### Phase 2 - The rest
In the second phase, we migrate the remaining async tasks to GATF. At the end of this phase, thumbnails will be deduplicated, and it will be possible to cancel any queued or running async task via the UI.
### Rejected Alternatives
SIP-39 and SIP-141 were rejected in favor of making a more general purpose Async Task Framework.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-3815515455
Thanks for the feedback @aminghadersohi ! Good callout on locking - we actually have the `DistributedLock` context manager that's perfect for this. Let me implement that to make sure we eliminate races.
Regarding retries, I'm still on the fence about that. [`backoff`](https://pypi.org/project/backoff/) already has very rich retry semantics, and I the way things are looking it should be straight forward to wrap the tasks with backoff to achieve whichever backoff strategy you want. But we'll see. At any rate we'll tackle that when we get to migrating Alerts & Reports (if you look at the PR, you can see that it's trivial to add new functionality via `TaskProperties`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2551777216
Closing this since it was approved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2284507815
@mistercrunch I've moved the ORM model to the body and made some general updates. Feel free to make updates if needed. I think this is approaching votability - WDYT @rusackas @michael-s-molina ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2269979472
Summary from meeting today (@mistercrunch, @michael-s-molina and myself in attendance):
- @villebro proposed migrating Thumbnails to the new framework first, as it would be quite straight forward to migrate, and is known to cause significant resource waste due to missing locking.
- @mistercrunch proposed investigating if Celery could be replaced with something better maintained, like Dask. Some reasons for considering this include:
- Lack of active maintenance of Celery
- Lack of advanced features that are available in Dask
- Opaque design of Celery, esp. the Kombu transport layer
- @villebro noted that it may be difficult to find a replacement for Celery that works well with Flask. However, we will investigate if this would indeed be possible, e.g. by extracting the needed Flask properties during task creation and placing them in the payload, or proxying/wrapping them so that Flask APIs are only used when available (e.g. `g`, `current_app`, `request`).
- @michael-s-molina proposed implementing the APIs for the new framework on Celery, and later replacing it with something else. @villebro noted that the choice of scheduler may well affect the API design, as the public APIs of the scheduler will likely affect the final API.
- @michael-s-molina wanted to emphasize, that if we do introduce Dask (or another replacement for Celery), we need to commit to a deprecation and removal plan that concludes in the full removal of Celery in a future major version of Superset.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2274484811
Trying to formalize the Model (?):
```python
class AsyncTask(Base):
__tablename__ = 'async_tasks'
id = Column(Integer, primary_key=True)
task_id = Column(String(256), unique=True, nullable=False, index=True)
task_name = Column(String(256), nullable=False)
status = Column(Enum('PENDING', 'IN_PROGRESS', 'SUCCESS', 'FAILURE', name='task_status'), nullable=False)
created_at = Column(DateTime, nullable=False)
updated_at = Column(DateTime, nullable=False)
ended_at = Column(DateTime, nullable=False)
error = Column(String, nullable=True)
extra_json = Column(Text, nullable=True)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2278812837
> Trying to formalize the Model (?):
>
> ```python
> class AsyncTask(Base):
> __tablename__ = 'async_tasks'
>
> id = Column(Integer, primary_key=True)
> task_id = Column(String(256), unique=True, nullable=False, index=True)
> task_name = Column(String(256), nullable=False)
> status = Column(Enum('PENDING', 'IN_PROGRESS', 'SUCCESS', 'REVOKED', 'FAILURE', name='task_status'), nullable=False)
> created_at = Column(DateTime, nullable=False)
> updated_at = Column(DateTime, nullable=False)
> ended_at = Column(DateTime, nullable=False)
> error = Column(String, nullable=True)
> extra_json = Column(Text, nullable=True)
> ```
I think this looks generally ok. Maybe we could just rename `extra_json` to `payload` or `state` (JSON serializable ofc), as I assume all tasks will have some state. Also, I find `Enum` works pretty badly on ORM tables, so we may just want to use `IntEnum` in the backend and have `INT` on the ORM table (we would call it `status_id` on the table, and then have a property on the SQLA model that maps the `INT` value to a proper Python `Enum`). Finally, I think we should add a `type` field (I assume `name` will be some human readable representation, like "Email report of 'My Dashboard'"), with the following `Enum`-type values (this list is probably non-exhaustive, but you get the idea):
- CHART_QUERY
- SQLLAB_QUERY
- THUMBNAIL
- ALERTS_REPORTS
- LOG_ROTATE
- CACHE_WARMUP
- CUSTOM (this would be for used defined tasks that could be cronned in the `CeleryConfig` or whichever scheduler we use).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-3787937587
Initial implementation PR is now ready for review: https://github.com/apache/superset/pull/36368
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2271725637
Thanks for the comments @zhaoyongjie . I think you raised a lot of good points, really appreciate the feedback 👍
> @villebro Thanks for document the SIP. I want to post a bit thought in here.
>
> I don't think we should replace Celery as task queue. The reasons are:
>
> Celery is a task queue but the Dask is a parallel computing framework. I'm not familiar with Dask, just have a look its document, it looks like Spark but in Python implementation so it should be different use case for a task queue.
After digesting this a bit more last night, I'm also starting to lean back in the direction of Celery for now. While there may be use cases for a full DAG-style computing framework (e.g. caching chart data to be cached before triggering the dashboard or something to that nature), I think the focus of the Dask project is slightly misaligned with what we're looking for. Regarding the other alternatives (Redis Queue, Dramatiq, Huey, APScheduler et al), I don't think any of them offer a clear improvement over Celery. So I agree, I'm kind of feeling like remaining on Celery for now, and postponing the architectural overhaul of Celery.
> > Lack of active maintenance of Celery
>
> Celery is activated, the latest version released at Apr 17, 2024.
I was going to say Celery has a long standing bug where the workers silently stop consuming tasks since many years, and this has served as an indicator of stagnation to me, as it has caused significant issues for us and required ugly workarounds: https://github.com/celery/celery/discussions/7276. However, it seems this bug has finally been fixed a few weeks ago as of Celery 5.5.0b1 (!). See Redis broker stability notes here: https://github.com/celery/celery/releases/tag/v5.5.0b1. So maybe things aren't as gloomy as I had thought, and things are slowly moving forward.
> > Lack of advanced features that are available in Dask
>
> Which advanced features should we want to use?
I think one of the main features Celery lacks is proper task cancellation, which is critical to us (Dask has good mechanisms for this). Also, Dask has some nice autoscaling features which makes it possible to handle bursty operations better than Celery. Finally, Dask is newer, so it incorporates many of the mechanisms that were not so relevant during Celery's inception. Granted, many of them may not be critical for us right now (e.g. graph/DAG support), but could have use cases when they become available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed issue #29839: [SIP-143] Global Async Task Framework
URL: https://github.com/apache/superset/issues/29839
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "giftig (via GitHub)" <gi...@apache.org>.
giftig commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2273759015
@villebro what do you mean by "proper" task cancellation here? Celery does support "revoking" tasks: https://docs.celeryq.dev/en/stable/userguide/workers.html#revoke-revoking-tasks
It's been a while since I've done a lot of work with celery but generally I've found it fairly reliable if properly configured.
I'm not familiar with Dask though, so can't comment on what advantages it might bring.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2274344041
> "proper" task cancellation
I'm guessing it's about running a certain routine when that happens. Some databases don't cancel the query on client disconnections, and would require us to run a routine of to kill the query. Something as simple as the STOP button in SQL Lab would have to hit an endpoint that would `celery.revoke(task_id, terminate=True)`, and rely on some except
Conversing with GPT it seems there are ways to configure and catch things:
<img width="832" alt="Screenshot 2024-08-07 at 1 59 44 PM" src="https://github.com/user-attachments/assets/f4f0a8fe-6290-4fee-a608-43274facd61b">
In my experience (mostly around Airflow's CeleryExecutor) it's common to end up with things not firing as expected, zombie tasks (Superset think the job is dead but still running somewhere on a worker machine), undead task (Superset the job is running, but nothing is running), forcing us to double-check the state. Seems like the framework is built for fire-and-forget typical web-scale workloads, as opposed to long-running tasks with good support for cancelation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2274516834
About this SIP, I wanted to highlight that there is a significant opportunity to formalize and eventually isolate the "Data Access Layer" (DAL) within Superset, exposing it as a set of APIs and related backend services. The idea would be to evolve it toward a robust, independent service that facilitates not only Superset but other apps to access data, while offering mechanisms to authenticate, authorize & limit access (RLS), cache, and audit access to data within your organization.
First step in that direction is to organize the code in a way that clarifies what is and what's not part of the DAL. Something to keep in mind while working on the async framework.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2278916801
Sounds good - also, given that this table will likely be ephemeral, INT vs VARCHAR for the type likely doesn't make any difference. I'll integrate the final proposal into the body 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "zhaoyongjie (via GitHub)" <gi...@apache.org>.
zhaoyongjie commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2271489939
@villebro Thanks for document the SIP. I want to post a bit thought in here.
I don't think we should replace Celery as task queue. The reasons are:
1. Celery is a task queue but the Dask is a parallel computing framework. I'm not familiar with Dask, just have a look its document, it looks like Spark but in Python implementation so it should be different use case for a task queue.
> Lack of active maintenance of Celery
Celery is activated, the latest version released at Apr 17, 2024.
> Lack of advanced features that are available in Dask
Which advanced features should we want to use?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-3819435373
@aminghadersohi I fixed the myriad race issues in the previous implementation. Here's the summary of the approach taken from the PR description:
### Distributed Locking
GTF uses distributed locks to prevent race conditions. Commands acquire a lock keyed by `dedup_key` before deciding what DAO operations to execute based on task state.
| Command | Condition | DAO Operations |
|---------|-----------|----------------|
| `SubmitTaskCommand` | No existing task | `create_task()` (auto-subscribes creator) |
| `SubmitTaskCommand` | Task exists | `add_subscriber()` |
| `CancelTaskCommand` | `subscriber_count <= 1` | `abort_task()` |
| `CancelTaskCommand` | `subscriber_count > 1` | `remove_subscriber()` |
| `UpdateTaskCommand` | Always | `update()` |
DAOs perform pure data operations and assume the caller holds the lock. The lock uses Superset's existing `KeyValueDistributedLock`.
---
For waiting for async tasks to complete, eventually we'll have webhooks, but for now you'll just have to poll unfortunately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2291821891
Notes from meeting this Monday ( @rusackas @michael-s-molina and I in attendance):
- We discussed if the Async Task list view would need to be real time. @villebro argued that it's out of scope, and that we can consider that as a different SIP (something like "Make React based FAB list views real time")
- @rusackas and @michael-s-molina asked to add something on how the async task status should be handled in the UI. @villebro will be adding details about this.
- Since the ORM table is going to be ephemeral (=entries are deleted once a task is no longer queued or executing), the question came up of how to have a persistent log of these events. @villebro proposed _not_ adding a separate history table, but rather logging to the `EVENT_LOGGER`. Additionally, we could also introduce new StatsD metrics to make it possible to do high level metrics monitoring.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-3815402536
Hey Ville, great proposal! A couple things came to mind:
Deduplication race conditions - The "check if exists, then insert" pattern for deduplication can race under load. Two requests could both see no entry and both try to schedule. Might be worth specifying the locking strategy - something like INSERT ... ON CONFLICT DO NOTHING or Redis SETNX. The current GAQ code uses Redis for exactly this reason.
Retry semantics - Alerts & Reports has pretty sophisticated retry logic (autoretry_for, max_retries=3, exponential backoff). The AsyncTask model doesn't have any retry-related fields. Will GATF support configurable retry policies per task type, or would tasks need to handle their own retries internally?
Also been working on MCP background tasks using FastMCP's task protocol - could be interesting to have MCP tools create GATF entries so AI-initiated operations get the same deduplication/visibility as web-initiated ones. Happy to chat if useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-3815555581
Really clean work on this. The ambient context pattern with contextvars is slick - having tasks call get_context() anywhere without threading it through every function signature is way nicer than what we have in MCP right now where context gets passed explicitly.
The on_abort auto-registration is clever too - first handler registered = marks abortable + starts listener. No ceremony required.
One question on integration: if I wanted to trigger a GTF task from an MCP tool and have the MCP client track its progress, what would that look like? I'm thinking something like:
```
@mcp_tool(task=True)
async def run_report(report_id: int, ctx: MCPContext):
# Create GTF task
gtf_task = generate_report.schedule(report_id=report_id)
# Bridge GTF progress → MCP progress?
while not gtf_task.is_terminal:
status = gtf_task.poll()
await ctx.report_progress(status.progress, 100, status.message)
```
Is there a way to subscribe to GTF task updates (maybe via the Redis pub/sub channel) so MCP doesn't have to poll? Or would you see GTF eventually supporting a callback/webhook pattern for external integrations?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-3825175742
Makes sense, clean separation keeping DAOs pure and locking at the command level. KeyValueDistributedLock keyed by dedup_key is a solid approach.
Webhooks would be perfect for the MCP bridge when you get there - would let us subscribe to GTF task completion without spinning on a poll loop. Happy to help spec that out when the time comes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP] Global Async Task Framework [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2278879839
Oh right, added `task_type`, and feedback all seem reasonable, feel free to edit my comment or to promote to the main body of the issue. About enums, personally I prefer a human-readable in-database for things like `status` that are pretty static/finite to avoid having to refer to the code or FK while referring directly to the database. I know it's less efficient and all, but that's for databases to figure out how to optimize/compress/dict-encode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2302669285
FYI: DISCUSS sent to Dev list: https://lists.apache.org/thread/ytv9vx7wrk07xxjg4m4kx5rx0zl5whx1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2291631879
I think this SIP is lookin' good! I think the SIP (now numbered 143) is _most definitely_ ready for a [DISCUSS] thread, and likelt ready for voting if there aren't any dealbreakers that turn up from that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2291823000
> I think this SIP is lookin' good! I think the SIP (now numbered 143) is _most definitely_ ready for a [DISCUSS] thread, and likely ready for voting if there aren't any dealbreakers that turn up from that.
Thanks @rusackas - I'll do the final changes to the body of the SIP this week and trigger the discuss once those are in place!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] [SIP-143] Global Async Task Framework [superset]
Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on issue #29839:
URL: https://github.com/apache/superset/issues/29839#issuecomment-2311128611
FYI: VOTE sent to Dev list: https://lists.apache.org/thread/b39obsb2qmqbo8k9twj9b17pfb42c6qr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org