feat: cancel queries on navigation-ish #13414
Conversation
|
Would be great to confirm in what navigation scenarios queries get cancelled now and in which they don't. |
|
|
||
| useEffect(() => { | ||
| reportDashboardViewed() | ||
| return () => { |
There was a problem hiding this comment.
Suggestion: create a useQueryCancellation(fn) wrapping useEffect.
There was a problem hiding this comment.
The problem with wrapping (or at least the problem I've not figured out the solution to) is the insight and dashboard cancellation cases are still really different
So, we have the existing behaviour that catches changes in filters and cancels running queries. This adds cancelling queries when the components are no longer in the DOM. So the cases covered right now
|
| @@ -295,11 +296,14 @@ export const insightLogic = kea<insightLogicType>([ | |||
| // fetch this now, as it might be different when we report below | |||
| const scene = sceneLogic.isMounted() ? sceneLogic.values.scene : null | |||
|
|
|||
| // If a query is in progress, kill that query | |||
| if (cache.abortController) { | |||
There was a problem hiding this comment.
Could we dispatch abortAnyRunningQuery here? Or extract function to keep DRY as we repeat the same code many times.
…ting playwright" This reverts commit 09234ce.
* add a test that checks abort query is called * remove the feature flag so I can test the cancel call * add a test for dashboard query cancellation * separate tests a little * remove breakpoint, I think it's copy pasta
| breakpoint() | ||
| cache.abortController = null | ||
|
|
||
| if (!cancelled && refreshesFinished === insights.length) { |
There was a problem hiding this comment.
@macobo one last thing to check now I have the tests passing
I've had to add a !cancelled gate here because sometimes this conditional was executed after tests had finished running.
I think it's OK not to report time_to_see data for the dashboard load here since we'd be reporting on the cancellation but....
Problem
We have query cancellation that fires when someone changes filters while a slow query is running.
But, they may become frustrated and navigate away within the app. We would leave any ClickHouse queries running in that case.
Changes
Calls into the mounted dashboard or insight logic when the
Dashboard.tsxorInsight.tsxis unmounted and attempts to cancel queriesNB: this doesn't try to unify this cancellation mechanism between insights and dashboards, just adds the new reasons to call it
How did you test this code?
running locally and 👀