fix dashboard index pattern race condition

[Dosant]

Summary

Fixes #72601 - "Sometimes filters on non-default-indexpattern dashboard are stuck in warning state".

Please note: it doesn't fix flash of warning state during dashboard loading: #72899 (review). It makes sure we won't stuck in this state. Separate issue to address flickering: #73286

Looks like we hard race condition there for a long time, but noticed it because of #66979 which made it easily noticeable

Problem

There was a race condition when setting index pattern in a dashboard:

Old code:

function updateIndexPattern() {
 const indexPatternsFromPanels = getIndexPatternsFromPanels();

 if (indexPatternsFromPanels.length > 0) {
    setIndexPatterns(indexPatternsFromPanels.length)
 } else {
  fetchDefaultIndexPatterns()
   .then(setIndexPatterns)
 }
} 

And this function was called a bunch of times in the beginning.
On each getOutput$() change:

updateIndexPattern();
updateIndexPattern();
updateIndexPattern(); // assume on this point indexPatternsInPanels are resolved and available
updateIndexPattern();
updateIndexPattern();

⬆️ but because code inside fetches defaults async, it could have happened that earlier calls finish last

Solution

I refactored that piece to use RxJS to get handy cancelations.

I didn't add any test, as not sure about the good way to test this. This fixes a race condition which would be hard to reliably catch in a function test. And dashboard controller need a refactor to start adding unit tests for logic there.

Release notes

Sometimes when creating filters on a dashboard suggestions from default index patterns were shown by mistake.

[elasticmachine]

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

[flash1293]

This looks pretty good already and it seems like I'm always ending up in the right state, but during loading it shows the warnings the wrong way:

Maybe we should pass a "loading" flag of some kind in to prevent "Warning" flags as longs as index patterns are still fetched?

[Dosant]

@flash1293, what if do not show filter bar until index patterns are properly loaded?
(introducing new loading state for filter bar in a sake of this hot fix seems like a risky overkill)

[flash1293]

@Dosant Just realized you want to get this into 7.9, not showing the filter bar is fine for me then. Maybe we can add a nice loading state (Maybe a Loading filters...) in a later version

[Dosant]

Hm, not sure I like this "hotfix" because of context shift:

I am thinking to leave this fix for 7.9 focused on fixing index pattern resolution race condition (no hiding filters bar) and to create a ticket for follow up with proper loading state for filters. WDYT?

[flash1293]

@Dosant Can we hold back the filters till the index patterns are loaded? So rendering an empty filter bar, then passing the filters through once the index patterns are ready?

If that doesn't fit well I would be fine with the current state.

[Dosant]

@flash1293, I'd say doesn't fit :(, filters and index patterns get into filter bar in completely different code paths.

• Index patterns via react component props
• Filters via FilterManager service which get's syncing with DashboardStateManger (url)

[flash1293]

@Dosant I'm fine with the current state in that case. @majagrubic are you fine with that approach for now as well? We can improve in 7.10

[Dosant]

@elasticmachine merge upstream

[flash1293]

@majagrubic is out this week, it seems like a pretty risky change for 7.9 at this point. @Dosant what do you think about moving this to 7.9.1?

[flash1293]

Approving as this is looking fine for me (with room for improvement as discussed) - we can already prepare the backports till we have a decision how to handle it

[lizozom]

I strongly support fixing this bug and handling loading state during 7.10.
Feel free to create an issue and assign it to me.

[lizozom]

Added some minor comments.

[Dosant]

@lizozom, created an issue to address "flickering" #73286

[Dosant]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 98dbd27

Build metrics

page load bundle size

id	value	diff	baseline
dashboard	679.5KB	+697.0B	678.9KB

History

• 💚 Build #64336 succeeded 14d8ec5
• 💚 Build #64247 succeeded 7f10d38
• 💚 Build #63669 succeeded 7b86bef

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