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

Race condition in dashboards makes search results not reflect current filters #30872

Open
3 tasks done
apoteet opened this issue Nov 7, 2024 · 6 comments
Open
3 tasks done
Labels
dashboard:performance Related to Dashboard performance

Comments

@apoteet
Copy link

apoteet commented Nov 7, 2024

Bug description

  1. Go to a dashboard, ideally featuring a slow table with lots of rows of data
  2. Run a search with very broad or no filters that will take a long time
  3. As soon as you start the slow search, run a much quicker search, such as by a specific id.

✅ The fast search will return and show up on the page
and then
❌ When the slow search completes, it will clobber the table content with the original search query results, putting you in a state where the data presented does not match the last thing searched.
I think when a new query is run, the old query needs to be cancelled/promise rejected.

Screenshots/recordings

Superset_race_condition.mov

Superset version

4.0.2

Python version

3.10

Node version

18 or greater

Browser

Chrome

Additional context

We have enabled EMBEDDED_SUPERSET, HORIZONTAL_FILTER_BAR, and EMBEDDABLE_CHARTS, though this video is in the superset app.
The datasource is a redshift table

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
@dosubot dosubot bot added the dashboard:performance Related to Dashboard performance label Nov 7, 2024
Copy link

dosubot bot commented Nov 7, 2024

To address the race condition in Superset dashboards where a slow search query overwrites the results of a faster search, you can implement a debouncing mechanism. This can be done using the debounce function from lodash to manage the timing of updates. The publishDataMask function in Superset already uses this approach to update URL parameters with the current state of the data mask, which is part of the filtering logic.

Here's a snippet of how this is implemented:

const publishDataMask = debounce(
  async (
    history,
    dashboardId,
    updateKey,
    dataMaskSelected: DataMaskStateWithId,
    tabId,
  ) => {
    // ... existing logic to update URL parameters
  },
  SLOW_DEBOUNCE,
);

The SLOW_DEBOUNCE constant sets the debounce delay, ensuring that only the most recent search query results are applied, preventing older, slower queries from overwriting them. Adjusting the debounce delay can help fine-tune the responsiveness and accuracy of the filter updates [1].

Additionally, Superset's frontend can handle the cancellation of ongoing queries when a new query is initiated by using an AbortController. This allows the ongoing query to be aborted if a new query is triggered, ensuring that the most recent query results are displayed [2].

To continue talking to Dosu, mention @dosu.

@apoteet
Copy link
Author

apoteet commented Nov 7, 2024

@dosu That would be true in my own application, but I do not own superset's react code.

Copy link

dosubot bot commented Nov 7, 2024

In Superset, handling query cancellations and ensuring that only the latest query results are displayed can be managed using the AbortController API. This API is used in Superset's query handling to manage and potentially abort ongoing HTTP requests. It is implemented in the superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx file. The AbortController is instantiated to create a signal that is passed to the SupersetClient.get method, allowing the request to be aborted if necessary [1].

Additionally, the sqlLabReducer in superset-frontend/src/SqlLab/reducers/sqlLab.js manages the lifecycle of queries, including starting, updating, and canceling them. The STOP_QUERY action, for example, sets the state of a query to Stopped and clears its results, which can help prevent older queries from overwriting newer results [2].

These mechanisms can help address the issue of race conditions in query execution by ensuring that only the most recent query results are displayed.

To continue talking to Dosu, mention @dosu.

@rusackas
Copy link
Member

rusackas commented Nov 8, 2024

@dosu That would be true in my own application, but I do not own superset's react code.

That's the joy of it! Nobody does! If you feel like opening a PR, we'd be plenty happy to review it.

Copy link

dosubot bot commented Nov 8, 2024

If you're interested in contributing a fix, you can fork the repository and make the necessary changes to handle query cancellations using the AbortController API. Once you've implemented and tested your solution, you can open a pull request for review. The community is generally supportive and can provide feedback to help refine your contribution. If you need guidance on setting up the development environment or have questions about the codebase, feel free to ask!

To continue talking to Dosu, mention @dosu.

@apoteet
Copy link
Author

apoteet commented Nov 8, 2024

That's the joy of it! Nobody does! If you feel like opening a PR, we'd be plenty happy to review it.

I thought my comment would get that response 😅. We'll see if the issue is as big a blocker as someone claimed and go from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dashboard:performance Related to Dashboard performance
Projects
None yet
Development

No branches or pull requests

2 participants