You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/04/14 00:33:03 UTC

[GitHub] [superset] benjreinhart opened a new issue #14112: Async query deduplication and cancellation support

benjreinhart opened a new issue #14112:
URL: https://github.com/apache/superset/issues/14112


   [SIP-39 - Global Async Query Support](https://github.com/apache/superset/issues/9190) mentions a future effort around deduplicating and cancelling asynchronous queries. I'm opening this issue to document our implementation plans for visibility + feedback.
   
   While we explored a few options for implementation, this is our current preferred approach.
   
   cc active participants in SIP-39: @robdiciuccio @etr2460 @DiggidyDave @willbarrett @williaster 
   
   ## Overview
   
   Deduplication will require the current flow, a 1:1 mapping of query to client, to move to a 1:N mapping of query to N clients.
   
   ### State
   
   **Existing state**
   
   The application currently performs four writes to Redis to support both sync/async queries:
   
   - [sync/async] Write the query results (and some other metadata) to cache
   - [async] Write the query context (for validating user access when later requesting cached values)
   - [async] Write to global event stream (for WebSocket server)
   - [async] Write to user-specific event stream (for WebSocket server)
   
   **New state requirements**
   
   All of the above values are currently written *after* the background worker has executed the query. Deduplication and cancellation features will need some state *before* the query has executed. To track requests, we'll need additional state:
   
   - All job ids that are waiting for the results of a given query
   - Some job metadata such as the query key, channel id, and user id
   
   ### Architecture
   
   ![Async Query Deduplication](https://user-images.githubusercontent.com/606233/114635006-a9942900-9c78-11eb-9389-3dde03a508c4.png)
   
   - Each request generates a key unique to that particular request using the generated SQL and any other relevant parameters (e.g., impersonating user). Referring to this as the *query key*.
   - A set of job ids are associated with the query key. These are waiting on query completion.
   - Each job id has its own JSON blob in Redis, containing job metadata.
   - Deduplication occurs when a query request generates a query key that maps to an existing key in Redis with a non-empty set of job ids. In this case, a new job is created and added to the set of jobs waiting on that query to complete. No new celery tasks are enqueued.
   - When the Celery task running the query completes, it looks up all job ids waiting completion of the query. For each one, it grabs its job metadata and adds events to the Redis streams which will be consumed by the WebSocket server and forwarded to connected clients.
   - If a request comes in for a query whose results are already cached, the server should return the cached results.
   - Cancellation for a particular client can occur via removing its job id from the set. If the set of job ids awaiting query completion is empty, the query can be safely cancelled.
   
   **Pros**
   
   - Existing state in Redis does not need to be modified
   - All Redis operations are atomic, no read-modify-write race conditions to consider
   
   **Cons**
   
   - Some cognitive overhead given the number of k/v pairs in Redis needed


-- 
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.

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


[GitHub] [superset] stale[bot] commented on issue #14112: Async query deduplication and cancellation support

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on issue #14112:
URL: https://github.com/apache/superset/issues/14112#issuecomment-868936095


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the issue.
   


-- 
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


[GitHub] [superset] csmith940 commented on issue #14112: Async query deduplication and cancellation support

Posted by GitBox <gi...@apache.org>.
csmith940 commented on issue #14112:
URL: https://github.com/apache/superset/issues/14112#issuecomment-889428345


   Hey all, has there been any progress made on this?


-- 
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


[GitHub] [superset] etr2460 commented on issue #14112: Async query deduplication and cancellation support

Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #14112:
URL: https://github.com/apache/superset/issues/14112#issuecomment-825426480


   Does the `query key` you discuss here differ from the chart `cache_key`? Can we reuse the cache key logic here?


-- 
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.

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


[GitHub] [superset] etr2460 commented on issue #14112: Async query deduplication and cancellation support

Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #14112:
URL: https://github.com/apache/superset/issues/14112#issuecomment-827263164


   Nope, that makes sense to me! Simply wanted to make sure that you knew we already had logic to generate this key that could be reused for this use case


-- 
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.

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


[GitHub] [superset] benjreinhart commented on issue #14112: Async query deduplication and cancellation support

Posted by GitBox <gi...@apache.org>.
benjreinhart commented on issue #14112:
URL: https://github.com/apache/superset/issues/14112#issuecomment-825892057


   @etr2460 I agree that consistency here is desired. I think we wanted to leave the existing cache _values_ as they are, but as for the logic that generates the cache _key_, the idea is for it to be the same hashing logic, but with a different key prefix.
   
   Does that make sense, or are you thinking something different?


-- 
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.

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