Limit fetching index patterns #56603

Dosant · 2020-02-03T12:02:43Z

Summary

Should address #56352.

I did a look up and it seems like only id and title a really used in case that savedObjectsCache is used. So I simply limited that request to fetch only title

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

It relies in implementation details, but better then nothing

elasticmachine · 2020-02-03T13:58:02Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

tylersmalley · 2020-02-05T01:10:56Z

ack, reviewing this evening.

tylersmalley · 2020-02-05T05:44:35Z

Thanks for jumping on this so quickly! At first I was excited by the limited scope required for this fix, but the more I dug into it the more I believe the change required to be a bit more involved. Looking at the get method alone in the IndexPatterns class, it's referenced in a dozen or so places. This method returns directly from the cache the object, which defined by the type, has multiple non-optional fields.

src/plugins/data/public/index_patterns/index_patterns/index_patterns.ts 
  129, 3:   get = async (id: string): Promise<IndexPattern> => { 
  123, 25:    return await this.get(defaultIndexPatternId); 
src/plugins/data/public/actions/apply_filter_action.ts 
  71, 39: return getIndexPatterns().get(filter.meta.index!); 
src/legacy/ui/public/saved_objects/helpers/hydrate_index_pattern.ts 
  52, 40:  await indexPatterns.get(index); 
src/legacy/core_plugins/visualizations/public/np_ready/public/expressions/visualization_function.ts 
  93, 64: await getIndexPatterns().get(args.index) : null; 
src/legacy/core_plugins/vis_type_timelion/public/helpers/arg_value_suggestions.ts 
  71, 32: return await indexPatterns.get(indexPatternSavedObject.id); 
src/legacy/core_plugins/kibana/public/discover/np_ready/components/doc/use_es_doc_search.ts 
  67, 60: await indexPatternService.get(indexPatternId); 
src/legacy/core_plugins/kibana/public/discover/np_ready/angular/context/api/context.ts 
  73, 46:  await indexPatterns.get(indexPatternId); 
src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.ts 
  136, 60: dataPluginStart.indexPatterns.get(controlParams.indexPattern); 
src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.ts 
  209, 60: dataPluginStart.indexPatterns.get(controlParams.indexPattern); 
src/legacy/core_plugins/input_control_vis/public/components/editor/controls_tab.tsx 
  70, 47: startDeps.data.indexPatterns.get(indexPatternId); 
src/legacy/core_plugins/data/public/search/expressions/esaggs.ts 
  263, 46:  await indexPatterns.get(args.index);

There might be something I am missing here, as I am unable to find a broken path and there do not appear to be any functional tests effected.

Dosant · 2020-02-05T10:45:25Z

@tylersmalley,

As I understood it, there are 2 different types of cache.

is simple savedObjectCache which I was fixing. It is used in couple places for pulling an info about all index patterns. And as I understood, only id and title are used. This cache is prefetched. This one is used in methods like getTitles, getIds and getFields (which is only used in one place for both id and title).
Then there is an indexPattern cache, which caches complete IndexPattern objects. This cache is populated lazily. This one is used in methods like get

Dosant · 2020-02-05T11:40:33Z

@elasticmachine merge upstream

kibanamachine · 2020-02-05T13:00:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1eb5ad6

History

💚 Build #23984 succeeded bcab597

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

src/plugins/data/public/index_patterns/index_patterns/index_patterns.ts

alexwizp

Looking like these changes should help, two minor nit's

tylersmalley · 2020-02-05T16:27:42Z

@Dosant, multiple index pattern cache types for the same class, that's wild - knew I was missing something, thanks for the explanation.

lukeelmers

This looks good to me. Tested Chrome OSX and nothing seems broken from what I can tell.

@Dosant is correct; There are two types of caches and the one that was touched here has very limited use (mainly for the index pattern select dropdown in discover AFAIK).

lukeelmers · 2020-02-05T18:46:30Z

src/plugins/data/public/index_patterns/index_patterns/index_patterns.test.ts

+    expect(savedObjectsClient.find).toHaveBeenCalledTimes(1);
+  });
+
+  test('can refresh the saved objects caches', async () => {


Thanks for adding these 🙏

Should address elastic#56352. I did a look up and it seems like only id and title a really used in case that savedObjectsCache is used. So I simply limited that request to fetch only title

Should address #56352. I did a look up and it seems like only id and title a really used in case that savedObjectsCache is used. So I simply limited that request to fetch only title Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Should address #56352.

Should address elastic#56352. I did a look up and it seems like only id and title a really used in case that savedObjectsCache is used. So I simply limited that request to fetch only title

Dosant added 4 commits February 3, 2020 12:23

cache only title

176658d

rename type

329cf04

add simple unit tests for caching saved objects

e6c3246

It relies in implementation details, but better then nothing

ues strict matching

bcab597

Dosant changed the title ~~[wip] Optimize pre-fetching index patterns~~ Optimize loading all index patterns Feb 3, 2020

Dosant changed the title ~~Optimize loading all index patterns~~ Limit fetching index patterns Feb 3, 2020

Dosant added release_note:fix v8.0.0 v7.7.0 review Team:AppArch labels Feb 3, 2020

Dosant added the Feature:Index Management Index and index templates UI label Feb 3, 2020

Dosant marked this pull request as ready for review February 3, 2020 13:59

Dosant requested a review from a team as a code owner February 3, 2020 13:59

Dosant requested review from mattkime and tylersmalley February 3, 2020 13:59

mattkime approved these changes Feb 4, 2020

View reviewed changes

Dosant requested a review from alexwizp February 5, 2020 10:48

Merge branch 'master' into dev/optimize-fetching-index-patterns

1eb5ad6

alexwizp reviewed Feb 5, 2020

View reviewed changes

src/plugins/data/public/index_patterns/index_patterns/index_patterns.ts Show resolved Hide resolved

alexwizp reviewed Feb 5, 2020

View reviewed changes

src/plugins/data/public/index_patterns/index_patterns/index_patterns.ts Show resolved Hide resolved

alexwizp reviewed Feb 5, 2020

View reviewed changes

tylersmalley approved these changes Feb 5, 2020

View reviewed changes

Dosant requested a review from lukeelmers February 5, 2020 17:55

lukeelmers approved these changes Feb 5, 2020

View reviewed changes

Dosant mentioned this pull request Feb 5, 2020

IndexPatterns service refactoring #56900

Closed

Dosant merged commit dcaee36 into elastic:master Feb 5, 2020

Dosant deleted the dev/optimize-fetching-index-patterns branch February 5, 2020 19:21

Dosant added the v7.6.1 label Feb 5, 2020

Dosant mentioned this pull request Feb 5, 2020

[7.x] Limit fetching index patterns (#56603) #56901

Merged

Dosant mentioned this pull request Feb 5, 2020

[7.6] Limit fetching index patterns (#56603) #56904

Merged

Dosant mentioned this pull request Feb 5, 2020

[6.8] Limit fetching index patterns (#56603) #56840

Merged

tylersmalley added the v6.8.6 label Feb 5, 2020

tylersmalley pushed a commit that referenced this pull request Feb 6, 2020

[6.8] Limit fetching index patterns (#56603) (#56840)

5aeb484

Should address #56352.

LeeDr added v7.6.0 and removed v7.6.1 labels Feb 6, 2020

alexh97 added v6.8.7 v7.6.1 and removed v6.8.6 v7.6.0 labels Feb 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit fetching index patterns #56603

Limit fetching index patterns #56603

Dosant commented Feb 3, 2020 •

edited

Loading

elasticmachine commented Feb 3, 2020

tylersmalley commented Feb 5, 2020

tylersmalley commented Feb 5, 2020

Dosant commented Feb 5, 2020 •

edited

Loading

Dosant commented Feb 5, 2020

kibanamachine commented Feb 5, 2020

alexwizp left a comment

tylersmalley commented Feb 5, 2020

lukeelmers left a comment

lukeelmers Feb 5, 2020

Limit fetching index patterns #56603

Limit fetching index patterns #56603

Conversation

Dosant commented Feb 3, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Feb 3, 2020

tylersmalley commented Feb 5, 2020

tylersmalley commented Feb 5, 2020

Dosant commented Feb 5, 2020 • edited Loading

Dosant commented Feb 5, 2020

kibanamachine commented Feb 5, 2020

💚 Build Succeeded

History

alexwizp left a comment

Choose a reason for hiding this comment

tylersmalley commented Feb 5, 2020

lukeelmers left a comment

Choose a reason for hiding this comment

lukeelmers Feb 5, 2020

Choose a reason for hiding this comment

Dosant commented Feb 3, 2020 •

edited

Loading

Dosant commented Feb 5, 2020 •

edited

Loading