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

[Lens] Avoid index pattern remapping for each visualization #89269

Closed
flash1293 opened this issue Jan 26, 2021 · 7 comments
Closed

[Lens] Avoid index pattern remapping for each visualization #89269

flash1293 opened this issue Jan 26, 2021 · 7 comments
Labels
Feature:Lens impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. performance Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@flash1293
Copy link
Contributor

In our current setup, the datasource initializes itself when a Lens visualization is loaded on a dashboard. This includes calling loadIndexPatterns, which loads the used index patterns and builds a map for all existing fields. As this is happening for each visualization individually, a large dashboard built with large index patterns can cause performance issues:

  • Lots of memory used for storing the same field maps/lists
  • Lots of time spent mapping and indexing the field lists

There are two options how we can improve this:

  • As we never change the field list, cache the mapped index pattern and reuse it for multiple initializations (downside: how to correctly clear the cache? it seems like the index pattern object is not immutable, so we could run into consistency issues)
  • Change Lens to not require any remapping, but simply use the index pattern instance provided by the data plugin cache (downside: this requires a lot of refactoring)
@flash1293 flash1293 added bug Fixes for quality problems that affect the customer experience performance Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Jan 26, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@dej611
Copy link
Contributor

dej611 commented Apr 12, 2021

I've run some investigation in the topic and found that, maybe, we're already taking advantage of the data plugin cache:

  • in the embeddable output the component is calling indexPatternService.get(id) which internally relies on the caching system
  • in the indexpattern_datasource the loadIndexPatterns function is relying on the same indexPatternService.get(id) method

After discussing with @wylieconlon , there are still probably some open questions around performance here:

  • what happens when loading 10 Lens embeddables in a dashboard with a big indexpattern? Will the cache be useful "in time"?
  • even if the cache reduces the network time, we're still making a copy of the indexpattern for each Lens embeddable. Can this be a bottleneck in terms of performance for big indexpatterns?

@flash1293
Copy link
Contributor Author

Yes, we are hitting the data plugin index pattern cache, this issue is about the re-mapping of index patterns within Lens (

const currentIndexPattern: IndexPattern = {
). Sorry if that wasn't clear from the issue.

what happens when loading 10 Lens embeddables in a dashboard with a big indexpattern? Will the cache be useful "in time"?

The data plugin takes care of that, it will only do the work once even if the same index pattern is requested at once multiple times.

@dej611 dej611 removed their assignment Apr 22, 2021
@timductive timductive added the impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. label Jul 20, 2021
@flash1293
Copy link
Contributor Author

Chatted with @mattkime about this and the data view instances delivered from the service are not immutable and might change field lists in some cases, so we can't use it as a cache key. However I'm not sure whether refactoring as described above would be a good idea as the instances not serializable as well.

Not sure about the right way forward here... @mbondyra what do you think about putting the data view instances directly into the redux store?

@flash1293 flash1293 removed the bug Fixes for quality problems that affect the customer experience label Oct 4, 2021
@mbondyra
Copy link
Contributor

mbondyra commented Oct 7, 2021

Data views have some unserialisable data so we'd break one of the redux state rules there, but it's not the first time we'd do it anyway.

I wonder if we could load data views before creating the store and have it as a dependency same way as datasourceMap or visualizationMap. But I guess not, given that the field list can change, eg. while adding runtime fields.

@flash1293
Copy link
Contributor Author

flash1293 commented Oct 7, 2021

I wonder if we could load data views before creating the store and have it as a dependency same way as datasourceMap or visualizationMap. But I guess not, given that the field list can change, eg. while adding runtime fields.

It's too expensive to preload all data views - we need an approach to do this on the fly. I like the idea of moving this to the frame level though. Maybe we can introduce a mechanism on the datasource to report a list of data views it requires, and the frame loads them async and then calls the sync functions of the data source, passing in the list of data view instances. I'm not sure whether they need to be in the redux state, we just put them in there so the datasource can use it in all places as the state is already passed into every function.

That would mean there are no data views in the state anymore, neither re-mapped nor unmapped. It's a pretty large refactoring though, but one that would reduce some complexity as we could remove the async initialization method of data sources; we just keep that around for the data views.

@stratoula
Copy link
Contributor

Caching the fieldlist solves this problem! Will be available in 8.13!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. performance Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

No branches or pull requests

7 participants