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

feat: cancel dashboard queries when changing filters #13041

Closed
wants to merge 20 commits into from

Conversation

pauldambra
Copy link
Member

@pauldambra pauldambra commented Nov 30, 2022

covers hitting refresh while refreshing but not changing filters while refreshing

Problem

Dashboards can have expensive insight queries. users may change filters or get frustrated/distracted and navigate away. We don't cancel the queries, so ClickHouse does all of the work.

Changes

Adds insight query cancellation to dashboards

TODO

  • verify this is really cancelling queries
  • tidy it up 😊

punt to a follow-up

  • changing filters currently waits for active refresh to finish and then loads the new set

How did you test this code?

🤷 I didn't yet 🤣

covers hitting refresh while refreshing but not changing filters while refreshing
@pauldambra pauldambra changed the title beginning to cancel dashboard queries. feat: cancel dashboard queries when unmounting Nov 30, 2022
@pauldambra pauldambra changed the title feat: cancel dashboard queries when unmounting feat: cancel dashboard queries when changing filters Dec 1, 2022
@pauldambra
Copy link
Member Author

I'm still working on this. Needs more testing (so there are still console logs and things)

But this is mostly there so thought I'd for an early(ish) sense-check.

@macobo
Copy link
Contributor

macobo commented Dec 1, 2022

It feels that AbortController and the KILL backend route are begging to be coupled together somehow...

Perhaps we should have a KillLogic which receives the latest query id that got kicked off, supplies methodOptions and kills running queries via an action?

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@pauldambra
Copy link
Member Author

This has been superceded by other work

#13178 and #13414

@pauldambra pauldambra closed this Dec 20, 2022
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 this pull request may close these issues.

None yet

3 participants