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

fix(insights): Fall back to dataNodeLogic results #22196

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

webjunkie
Copy link
Contributor

@webjunkie webjunkie commented May 8, 2024

Problem

We do not make requests to /query endpoint from dashboards (anymore). However, only these would run async.
The dataNodeLogic that is responsible would do it, but it gets cached results now and doesn't act even when they are null.

Goal:

  • dashboard loads, insight is empty
  • insight tries to refresh, but this times out, leading to a still empty result
  • dataNodeLogic gets triggered and loads with the async query, possibly succeeding

Changes

  • adjust conditions so that when results are null and and dataNodeLogic doesn't yet have any results, then it loads

DANGER: This is not yet checked for the bug that caused an incident.
Somewhere here this leads to the logic aborting queries (sending DELETE requests) before even starting to send the first ones.

Update: Found the culprit. For each insight that has result: null when the dashboard loads, the dataNodeLogic already triggers, which is not what we want. Because it triggers multiple times somehow, it cancels its own queries as well, leading to the DELETE requests.

Does this work well for both Cloud and self-hosted?

n/a

How did you test this code?

tbd

Copy link
Contributor

github-actions bot commented May 8, 2024

Size Change: 0 B

Total Size: 1.05 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.05 MB

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

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