Skip to content

Comments

[Controls] Fix initialization race condition#237726

Merged
ThomThomson merged 10 commits intoelastic:mainfrom
ThomThomson:controls/fixInitializationRaceCondition
Oct 14, 2025
Merged

[Controls] Fix initialization race condition#237726
ThomThomson merged 10 commits intoelastic:mainfrom
ThomThomson:controls/fixInitializationRaceCondition

Conversation

@ThomThomson
Copy link
Contributor

@ThomThomson ThomThomson commented Oct 6, 2025 •
edited by kibanamachine
Loading

Summary

Fixes a race condition in the controls initialization step. Followup to #237169.

This should make the fix for the double-refetch bug more robust as now the Dashboard panels will not load at all until the Controls have finished initializing, and the control group has finished combining their initial filters into its filters array.

There should be no user-facing changes in this PR as the previous fix worked correctly despite not fixing the underlying race condition as the panel initialization step always takes longer than the filter publish & data view fetch step.

Changes:

  1. The Options list control no longer returns an initial filter before the data view and field were ready.
  2. The Dashboard panels now begin loading after the control group combines its child filters, rather than in parallel.

@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 6, 2025

/ci

@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 7, 2025

/ci

@ThomThomson ThomThomson added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. backport:version Backport to applied version labels v8.19.0 v9.2.0 v9.3.0 v8.19.6 and removed v8.19.0 labels Oct 8, 2025
@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 8, 2025

/ci

@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 8, 2025

@elasticmachine merge upstream

@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 8, 2025

/ci

@ThomThomson ThomThomson marked this pull request as ready for review October 9, 2025 15:06
@ThomThomson ThomThomson requested a review from a team as a code owner October 9, 2025 15:06
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 9, 2025

Pinging @elastic/kibana-presentation (Team:Presentation)

nreese
nreese reviewed Oct 9, 2025
View reviewed changes
...form/plugins/shared/dashboard/public/dashboard_renderer/viewport/dashboard_viewport.test.tsx
});
});

test('should not render DashboardGrid until controls are ready', async () => {
Copy link
Contributor

@nreese nreese Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this test being removed? I don't think it should be removed

Copy link
Contributor Author

@ThomThomson ThomThomson Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, that was a holdover from when I was using pauseFetch to fix the bug in the same PR. Restored this test.

nreese
nreese reviewed Oct 9, 2025
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does filtersReadySubscription need to be clean up? The reason for the debounce is to give time for the filters to propagate to the control group. I don't think this is needed any more.

@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 10, 2025

I removed the unnecessary debounce from filtersReadySubscription, but as a follow-up I'd like to look into removing it entirely and making the initializeDataControlManager function async instead. That way we can avoid all this extra noise around filters being ready etc.

I decided not to do that in this PR because I'd like to keep this as stable as possible and that would introduce a lot of changes. Additionally, we'll be merging controlsAnywhere into main soon which will change this initialization logic. I'll attempt a clean up either in the feature branch, or after it merges.

nreese
nreese approved these changes Oct 10, 2025
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for cleaning up these initialization conditions.
code review only

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 10, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
controls 488.9KB 489.0KB +127.0B
dashboard 662.4KB 662.4KB +5.0B
total +132.0B

History

@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 14, 2025

@elasticmachine merge upstream

@ThomThomson ThomThomson merged commit e26e842 into elastic:main Oct 14, 2025
12 checks passed
@kibanamachine
Copy link
Contributor

kibanamachine commented Oct 14, 2025

Starting backport for target branches: 8.19, 9.2

https://github.com/elastic/kibana/actions/runs/18508235826

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 14, 2025
@ThomThomson
[Controls] Fix initialization race condition (elastic#237726)
85d6769 
Fixes a race condition in the controls initialization step.

(cherry picked from commit e26e842)
@kibanamachine kibanamachine mentioned this pull request Oct 14, 2025
Merged
@kibanamachine
Copy link
Contributor

kibanamachine commented Oct 14, 2025

💔 Some backports could not be created

Status Branch Result
❌ 8.19 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.19:
- [Controls] migrate scss to emotion (#216413)
✅ 9.2

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 237726

Questions ?

Please refer to the Backport tool documentation

This was referenced Oct 14, 2025
Merged
Closed
ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Oct 14, 2025
@ThomThomson
[Controls] Fix initialization race condition (elastic#237726)
b3304d8 
Fixes a race condition in the controls initialization step.

(cherry picked from commit e26e842)

# Conflicts:
#	src/platform/plugins/shared/controls/public/control_group/selections_manager.ts
#	src/platform/plugins/shared/controls/public/controls/data_controls/initialize_data_control.ts
#	src/platform/plugins/shared/controls/public/controls/data_controls/options_list_control/get_options_list_control_factory.test.tsx
#	src/platform/plugins/shared/dashboard/public/dashboard_api/control_group_manager.ts
@ThomThomson ThomThomson mentioned this pull request Oct 14, 2025
Merged
@ThomThomson
Copy link
Contributor Author

ThomThomson commented Oct 14, 2025

💚 All backports created successfully

Status Branch Result
✅ 9.1
✅ 9.0
✅ 8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Oct 14, 2025
@ThomThomson
[Controls] Fix initialization race condition (elastic#237726)
eff0c49 
Fixes a race condition in the controls initialization step.

(cherry picked from commit e26e842)

# Conflicts:
#	src/platform/plugins/shared/controls/public/control_group/selections_manager.ts
#	src/platform/plugins/shared/controls/public/controls/data_controls/data_control_manager.ts
#	src/platform/plugins/shared/controls/public/controls/data_controls/options_list_control/get_options_list_control_factory.test.tsx
ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Oct 14, 2025
@ThomThomson
[Controls] Fix initialization race condition (elastic#237726)
0636263 
Fixes a race condition in the controls initialization step.

(cherry picked from commit e26e842)

# Conflicts:
#	src/platform/plugins/shared/controls/public/control_group/selections_manager.ts
#	src/platform/plugins/shared/controls/public/controls/data_controls/data_control_manager.ts
#	src/platform/plugins/shared/controls/public/controls/data_controls/options_list_control/get_options_list_control_factory.test.tsx
kibanamachine added a commit that referenced this pull request Oct 14, 2025
@kibanamachine @ThomThomson
[9.2] [Controls] Fix initialization race condition (#237726) (#239007)
f196358 
# Backport

This will backport the following commits from `main` to `9.2`:
- [[Controls] Fix initialization race condition
(#237726)](#237726)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Devon
Thomson","email":"devon.thomson@elastic.co"},"sourceCommit":{"committedDate":"2025-10-14T19:46:49Z","message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d","branchLabelMapping":{"^v9.3.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","loe:small","release_note:skip","impact:medium","backport:version","v9.2.0","v9.3.0","v8.19.6"],"title":"[Controls]
Fix initialization race
condition","number":237726,"url":"https://github.com/elastic/kibana/pull/237726","mergeCommit":{"message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d"}},"sourceBranch":"main","suggestedTargetBranches":["9.2","8.19"],"targetPullRequestStates":[{"branch":"9.2","label":"v9.2.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.3.0","branchLabelMappingKey":"^v9.3.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/237726","number":237726,"mergeCommit":{"message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d"}},{"branch":"8.19","label":"v8.19.6","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Devon Thomson <devon.thomson@elastic.co>
ersin-erdal pushed a commit to ersin-erdal/kibana that referenced this pull request Oct 15, 2025
@ThomThomson @ersin-erdal
[Controls] Fix initialization race condition (elastic#237726)
665d00a 
Fixes a race condition in the controls initialization step.
ThomThomson added a commit that referenced this pull request Oct 15, 2025
@ThomThomson
[9.1] [Controls] Fix initialization race condition (#237726) (#239011)
10829ea 
# Backport

This will backport the following commits from `main` to `9.1`:
- [[Controls] Fix initialization race condition
(#237726)](#237726)

<!--- Backport version: 10.0.2 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Devon
Thomson","email":"devon.thomson@elastic.co"},"sourceCommit":{"committedDate":"2025-10-14T19:46:49Z","message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d","branchLabelMapping":{"^v9.3.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","loe:small","release_note:skip","impact:medium","backport:version","v9.2.0","v9.3.0","v9.1.6","v8.19.6","v9.0.9"],"title":"[Controls]
Fix initialization race
condition","number":237726,"url":"https://github.com/elastic/kibana/pull/237726","mergeCommit":{"message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19","9.0"],"targetPullRequestStates":[{"branch":"9.2","label":"v9.2.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/239007","number":239007,"state":"OPEN"},{"branch":"main","label":"v9.3.0","branchLabelMappingKey":"^v9.3.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/237726","number":237726,"mergeCommit":{"message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d"}},{"branch":"9.1","label":"v9.1.6","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.6","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.0","label":"v9.0.9","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
ThomThomson added a commit that referenced this pull request Oct 15, 2025
@ThomThomson
[8.19] [Controls] Fix initialization race condition (#237726) (#239013)
7cb1470 
# Backport

This will backport the following commits from `main` to `8.19`:
- [[Controls] Fix initialization race condition
(#237726)](#237726)

<!--- Backport version: 10.0.2 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Devon
Thomson","email":"devon.thomson@elastic.co"},"sourceCommit":{"committedDate":"2025-10-14T19:46:49Z","message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d","branchLabelMapping":{"^v9.3.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","loe:small","release_note:skip","impact:medium","backport:version","v9.2.0","v9.3.0","v9.1.6","v8.19.6","v9.0.9"],"title":"[Controls]
Fix initialization race
condition","number":237726,"url":"https://github.com/elastic/kibana/pull/237726","mergeCommit":{"message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19","9.0"],"targetPullRequestStates":[{"branch":"9.2","label":"v9.2.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/239007","number":239007,"state":"OPEN"},{"branch":"main","label":"v9.3.0","branchLabelMappingKey":"^v9.3.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/237726","number":237726,"mergeCommit":{"message":"[Controls]
Fix initialization race condition (#237726)\n\nFixes a race condition in
the controls initialization
step.","sha":"e26e842a094f95c0396bdd91f70ecd409956082d"}},{"branch":"9.1","label":"v9.1.6","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.6","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.0","label":"v9.0.9","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
mgadewoll pushed a commit to tkajtoch/kibana that referenced this pull request Oct 17, 2025
@ThomThomson @mgadewoll
[Controls] Fix initialization race condition (elastic#237726)
01b0ac8 
Fixes a race condition in the controls initialization step.
rylnd pushed a commit to rylnd/kibana that referenced this pull request Oct 17, 2025
@ThomThomson
[Controls] Fix initialization race condition (elastic#237726)
7c612e9 
Fixes a race condition in the controls initialization step.
nickpeihl pushed a commit to nickpeihl/kibana that referenced this pull request Oct 23, 2025
@ThomThomson @nickpeihl
[Controls] Fix initialization race condition (elastic#237726)
97e0b25 
Fixes a race condition in the controls initialization step.
NicholasPeretti pushed a commit to NicholasPeretti/kibana that referenced this pull request Oct 27, 2025
@ThomThomson @NicholasPeretti
[Controls] Fix initialization race condition (elastic#237726)
56acb7f 
Fixes a race condition in the controls initialization step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nreese nreese nreese approved these changes

Assignees

No one assigned

Labels

backport:version Backport to applied version labels impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.19.6 v9.0.9 v9.1.6 v9.2.0 v9.3.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ThomThomson @elasticmachine @kibanamachine @nreese