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

Hiding a measure or dimension cancels outstanding queries #3960

Open
ericpgreen2 opened this issue Feb 1, 2024 · 0 comments
Open

Hiding a measure or dimension cancels outstanding queries #3960

ericpgreen2 opened this issue Feb 1, 2024 · 0 comments
Labels
Type:Bug Something isn't working

Comments

@ericpgreen2
Copy link
Contributor

ericpgreen2 commented Feb 1, 2024

Issue summary

We've identified that hiding measures or dimensions results in the cancellation of certain outstanding API queries. This behavior stems from two primary problems related to state management and component virtualization.

Bug report in Slack

Problem 1: coarse-grained state management

In Leaderboard.svelte, we have a TanStack Query call to the compare-toplist API. When a user changes the set of visible dimensions, any outstanding requests to this API get cancelled.

TanStack Query cancels an outstanding request because it has detected that its query parameters have changed.

In this case, TanStack Query thinks the following query parameters have changed:

  • measures (pulled from dashboardStore.visibleMeasureKeys)
  • timeRange and comparisonTimeRange (pulled from timeControlsStore, which pulls from dashboardStore)
  • where (pulled from dashboardStore.whereFilter)

Why does TanStack think that these query parameters have changed?

First, they’re deriving from the monolithic dashboardStore, which root object is changing with the new set of visible measures (dashboardStore.visibleMeasureKeys) or dimensions (dashboardStore.visibleDimensionKeys).

To attempt to prevent unnecessary updates, we could create derived stores to only subscribe to specific subsets of the root dashboardStore (IIUC, this is what the createReadablesFromSelectors() function is attempting to do in the state-managers package). However, derived stores only correctly prevent unnecessary updates when they return a simple data type (e.g. a boolean, string, or number). In this case, the above data types are complex (e.g. an array or object).

I created this Svelte REPL to understand the following behavior of derived stores:

  1. If a root store has changed, a derived store that returns a simple type will only trigger its subscribers if the returning value has changed. (As expected.)
  2. If a root store has changed, a derived store that returns a complex type will always trigger its subscribers. It appears Svelte does not run a deep check to understand whether or not the data has actually changed. (I’ve tried different ways of updating the root store, but the result is always the same. Could I be missing something?)

Solution

Ideally, we should break the monolithic root dashboardStore into smaller independent stores. At least, independent stores for the dimensions, measures, timeRange and whereFilter objects.

Unfortunately, this state management refactor is substantial.

Are there any shortcuts?

At least for the timeRange state, we could keep its state where it is, but create more granular derived stores that each return a simple value. Instead of a timeRange derived store that returns {startDate, endDate}, we could have standalone derived stores for timeRangeStartDate and timeRangeEndDate. This would avoid unnecessary updates of the timeRange query parameters whenever unrelated properties of the dashboardStore update. However, this hack won't work for the measures and where data types because they're arbitrarily long and nested. So we cannot convert them to a discrete number of granular derived stores.

Problem 2: virtualization causes props shift

Assuming we solve problem 1, the second problem is specific to hiding dimensions (not measures).

We have the following code, where each leaderboard in a virtualized grid takes a dimension name as a prop:

<!-- LeaderboardDisplay.svelte -->
<VirtualizedGrid
  items={$visibleDimensions}
  let:item={dimension}
>
  <Leaderboard dimensionName={dimension.name} />
</VirtualizedGrid>

Consider a scenario where the grid displays 5 leaderboards on screen with an additional 5 below the scroll. If the first dimension is hidden, the visibleDimensions array decreases in size, and every subsequent dimension shifts one position to the left in the grid. Because the virtualized grid reuses components to prevent unnecessary unmounting/re-remounting, all dimensions get re-assigned to the preceding leaderboard.

Inside each <Leaderboard /> component, the dimension name is used as a parameter to TanStack Query’s call to the compare-toplist API. TanStack will consider its query for the prior dimension name stale and will cancel an in-flight request. TanStack will then issue a new request with the updated dimension name parameter.

Solution

To fix this, we can change our virtualization logic so that we do not re-use leaderboard components when the visible dimensions change. Rather, every leaderboard component can be assigned a specific dimension. When a dimension is hidden, a new leaderboard will be mounted with a previously out-of-viewport dimension.

This may not be an entirely "virtualized" table because scrolling up-and-down will unmount and mount new leaderboard instances, rather than re-use the same instances. We should understand if this materially impacts rendering performance. However, this approach would still provide the data fetching benefit of virtualization: out-of-viewport leaderboards will not be added to the DOM, so their data will not be requested from the API.

@ericpgreen2 ericpgreen2 added the Type:Bug Something isn't working label Feb 1, 2024
@ericpgreen2 ericpgreen2 changed the title Deselecting dimensions cancels outstanding queries Hiding a dimension cancels outstanding queries Feb 1, 2024
@ericpgreen2 ericpgreen2 changed the title Hiding a dimension cancels outstanding queries Hiding a measure or dimension cancels outstanding queries Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant