search

influxdata / ui

UI for InfluxDB
93 stars 42 forks source link

Dashboards: Query/QueryResult Sync Issue #1935

Open subirjolly opened 3 years ago

subirjolly commented 3 years ago

Before Before.gif

After After.gif

The Issue: In both the gifs, you can see that the cell isn't loading after you click on Submit and then the Green Check. The underlying issue, I think, is that there's no global query context and we have to keep/sync the states of the query in multiple places/components. The solution is to move towards having a global query object/context which keeps track on all the queries and we don't have to rely on different mechanisms to store query and their states.

@drdelambre @Asalem1 Thoughts?

subirjolly commented 3 years ago

https://app.zenhub.com/workspaces/applicationsui-unity-606b1041215d5f0017898847/issues/influxdata/ui/1935

asalem1 commented 3 years ago

Sorry, just reading this.

In both the gifs, you can see that the cell isn't loading after you click on Submit and then the Green Check.

This is by design. What was previously happening was any changes to a cell in the VEO was updating the cells, effectively causing the same query to run twice. By locking in the cell's context until AFTER the cell has finished changing, we guarantee that the changes to the view don't affect the cell until AFTER the modifications have finished.

For example, previously when I had opened the view editor overlay, changing the timerange would cause ALL the cells within the background view to update their results. So if a user were to toggle the timerange a couple of times, then cancel their changes (or end up where they started), we would trigger a bunch of changes to the cells even though those changes wouldn't be relevant to what the user would be doing until AFTER they had closed the overlay.

The underlying issue, I think, is that there's no global query context and we have to keep/sync the states of the query in multiple places/components.

That's partially correct, but there's an implicit need that keep the view layer and query layer separate like it currently is. @drdelambre can expound on that a bit more.

The solution is to move towards having a global query object/context which keeps track on all the queries and we don't have to rely on different mechanisms to store query and their states.

I don't think this resolves the issue you mentioned above, since that's not an issue, but it does resolve some view layer issues that we see when trying to sync view layer state with query state.

drdelambre commented 3 years ago

yeah, this is by design to reduce running the same query (just to return the same results). the need for centralizing the query status is that the "query is currently running" is predominately handled by the view layer. i remember work was done in here to explicitly share this state between the cell and the veo, but iirc it only works one way (i dont think a pending cell's request can be viewed as pending in the VEO, nor can you cancel it once in there). centralizing these affordances (i have a query, what is it's loading state, how can i cancel it, etc), should remove the issue of duplicating these mechanisms / sources of truth for the query's state, and removing it from the current redux implementation should decouple the mechanisms from their current dependence on internal time machine state

subirjolly commented 3 years ago

This note is for myself. I need to do the query caching by using global query context when we press cmd+enter instead of clicking Submit.

To summarize the work so far and my findings. I have created a global query context which can cache queries and their results for a default of 30 seconds and will clean up the queries after 10 minutes on any interaction with the context. There need to be a couple of phases for this rollout.

Phase 1: Create GlobalQueryContext and use it only when Running the queries. So, if the query result hasn't been cached in the context then the query will be run and if the same query gets run from any part of the UI then you will be getting the cached result instantaneously. In order to do that, we will have to get the data from this caching layer and insert into the respective places where the data currently is expected to reside. This change needs to happen across the UI.

Phase 2: The next phase aims at being the source of truth for the fetched resultsets. Instead of storing the data locally in their currently respective stores/places, we will be storing that data inside the Global Query Context only. Every other place which needs to run queries or access data needs to point to this context only.

russorat commented 3 years ago

@jzahnd should this be an epic?