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

[Dashboard De-Angular] Fix race condition by better debouncing URL state updates #4193

Open
Tracked by #3365
abbyhu2000 opened this issue May 31, 2023 · 0 comments
Open
Tracked by #3365
Assignees
Labels
dashboards de-angular de-angularize work

Comments

@abbyhu2000
Copy link
Member

abbyhu2000 commented May 31, 2023

Current the tile functional test is skipped due to the flaky results.

This is likely due to a race condition within the current state management: sometimes the previous app state or global state is not completely wiped out when navigating to the next page, and this will cause errors when loading the next dashboard page.

https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4502/files#r1256489047

What appears to be happening is there are multiple hooks and multiple services attempting to modify the URL. If the global state is modified within the URL before the app state is modified then the app state will not be modified. Meaning it will carry over the previous app state to the current app.

The specific functional test that catches is the tile map functional test. The previous test navigates to an existing dashboard and applies a query in the URL. The url updates the app state and is applied to the current child embeddables appropriately. Then the user navigates from this dashboard and to another dashboard really quickly and sometimes the global state is modified before the app state is and the app state remains with the previous query. This query is then applied to the child embeddables which can prevent proper display of data. The user can refresh and be back in the correct state.

tl;dr: race condition in url updates can cause the app state to carry over queries from a previous dashboard page

Requirements

  • Verify the root cause is a race condition
    • Run the test group multiple times
  • Propose implementation if the previous step is successful
    • One possible solution is to sync query state from url state better. For example, have a URL state manager to handle and debounce frequent URL state updates from both app state and global state. Utilizing a debouncer can limit to apply the current changes to state to the URL until all attempts to change it have come in and only update it after time period (try to be as low as possible but do not go above 100 ms, at which point the solution is likely incorrect). Rxjs has a debounce method
  • Tests

This task will likely require some more time: 1. recreate the issue and verify the state management is the reason that the functional test is flaky 2. investigation and explorations on different solution options.

@abbyhu2000 abbyhu2000 added dashboards de-angular de-angularize work and removed untriaged labels May 31, 2023
@abbyhu2000 abbyhu2000 changed the title [Dashboard De-Angular] Fix skipped unit tests and functional tests [Dashboard De-Angular] Fix the flaky tile functional test by handling and better debouncing URL state updates Jul 19, 2023
@kavilla kavilla changed the title [Dashboard De-Angular] Fix the flaky tile functional test by handling and better debouncing URL state updates [Dashboard De-Angular] Fix race condition by handling and better debouncing URL state updates Jul 20, 2023
@abbyhu2000 abbyhu2000 changed the title [Dashboard De-Angular] Fix race condition by handling and better debouncing URL state updates [Dashboard De-Angular] Fix race condition by better debouncing URL state updates Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dashboards de-angular de-angularize work
Projects
Development

No branches or pull requests

1 participant