[discover] fix too many queries sent #8347

kavilla · 2024-09-25T21:31:26Z

Description

Fixes the issue where too many queries were being fired. A number of things were being called because the dependency array was triggering updates and re-calls. Forcing a re-render of elements that had the possibility of updating the query manager.

Includes some tests.

Issues Resolved

n/a

Screenshot

Testing the changes

locally and tests

Changelog

fix: prevent too many queries sent from dataset selector

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

the dependency array was triggering updates and re-calls. forcing a re-render of elements that had the possibility of updating the query manager. Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

github-actions · 2024-09-25T21:32:08Z

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

codecov · 2024-09-25T21:46:58Z

Codecov Report

Attention: Patch coverage is 62.50000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 60.92%. Comparing base (d1caa92) to head (3352f97).
Report is 47 commits behind head on main.

Files with missing lines	Patch %	Lines
...ta/public/ui/dataset_selector/dataset_selector.tsx	53.33%	3 Missing and 4 partials ⚠️
.../plugins/data/public/ui/dataset_selector/index.tsx	87.50%	0 Missing and 1 partial ⚠️
...tion/view_components/utils/update_search_source.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8347      +/-   ##
==========================================
- Coverage   60.98%   60.92%   -0.07%     
==========================================
  Files        3743     3748       +5     
  Lines       88857    89016     +159     
  Branches    13859    13898      +39     
==========================================
+ Hits        54188    54229      +41     
- Misses      31314    31424     +110     
- Partials     3355     3363       +8

Flag	Coverage Δ
Linux_1	`28.85% <0.00%> (+<0.01%)`	⬆️
Linux_2	`56.34% <ø> (ø)`
Linux_3	`?`
Linux_4	`29.94% <ø> (ø)`
Windows_1	`28.87% <0.00%> (+<0.01%)`	⬆️
Windows_2	`56.29% <ø> (ø)`
Windows_3	`37.76% <62.50%> (-0.04%)`	⬇️
Windows_4	`29.94% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

sejli

Amazing

sejli · 2024-09-26T21:44:03Z

src/plugins/data/public/ui/dataset_selector/index.tsx

-  useEffect(() => {
-    const subscription = queryString.getUpdates$().subscribe((query) => {
-      setSelectedDataset(query.dataset);
-    });
-
-    return () => subscription.unsubscribe();
-  }, [queryString]);


I remember being confused why this is here. Was there a reason why we needed two places to set the selected dataset?

not positive. probably was just code that was intended to be cleaned up i think i at one point we would set the query at the configurator level instead of using the function that we passed in to set it and then when we got time to wire up and use the passed in function it caused some issue with re-rendering.

abbyhu2000 · 2024-09-26T22:03:39Z

src/plugins/data/public/query/query_string/query_string_manager.ts

@@ -68,6 +68,7 @@ export class QueryStringManager {
    const query = {
      query: defaultQuery,
      language: defaultLanguageId,
+      dataset: undefined,


why is this necessary? i see query is used to construct newQuery and dataset is set to be defaultDataset in line 81

my linter was complaining because i thought the return type was just query language but it's causing a test failure so im just gunna revert this and but a linter ignore

joshuali925

nice, lgtm

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

…OpenSearch-Dashboards-1 into kavilla/discovermultiplequeries

kavilla · 2024-09-26T23:25:17Z

failures unrelated and i think another PR is fixing it.

Fixes the issue where too many queries were being fired. A number of things were being called because the dependency array was triggering updates and re-calls. Forcing a re-render of elements that had the possibility of updating the query manager. Includes some tests. Signed-off-by: Kawika Avilla <kavilla414@gmail.com> --------- Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 0882435) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Fixes the issue where too many queries were being fired. A number of things were being called because the dependency array was triggering updates and re-calls. Forcing a re-render of elements that had the possibility of updating the query manager. Includes some tests. Signed-off-by: Kawika Avilla <kavilla414@gmail.com> --------- Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

Fixes the issue where too many queries were being fired. A number of things were being called because the dependency array was triggering updates and re-calls. Forcing a re-render of elements that had the possibility of updating the query manager. Includes some tests. --------- (cherry picked from commit 0882435) Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

…earch-project#8361) Fixes the issue where too many queries were being fired. A number of things were being called because the dependency array was triggering updates and re-calls. Forcing a re-render of elements that had the possibility of updating the query manager. Includes some tests. --------- (cherry picked from commit 0882435) Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

[data] fix issue with multiple queries being sent

2d6ce25

the dependency array was triggering updates and re-calls. forcing a re-render of elements that had the possibility of updating the query manager. Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

github-actions bot added the distinguished-contributor label Sep 25, 2024

github-actions bot added the failed changeset label Sep 25, 2024

kavilla added 2 commits September 26, 2024 08:06

remove debounced

6315e5e

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

add tests

f739c11

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

kavilla marked this pull request as ready for review September 26, 2024 09:27

github-actions bot removed the failed changeset label Sep 26, 2024

kavilla added backport 2.x discover for discover reinvent discover-next 2.17.1 backport 2.17-next labels Sep 26, 2024

AMoo-Miki added v2.18.0 and removed 2.17.1 backport 2.17-next labels Sep 26, 2024

abbyhu2000 previously approved these changes Sep 26, 2024

View reviewed changes

sejli previously approved these changes Sep 26, 2024

View reviewed changes

Merge branch 'main' into kavilla/discovermultiplequeries

6bf8299

abbyhu2000 reviewed Sep 26, 2024

View reviewed changes

joshuali925 previously approved these changes Sep 26, 2024

View reviewed changes

kavilla added 2 commits September 26, 2024 22:32

type the return

c788c99

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>

Merge branch 'kavilla/discovermultiplequeries' of github.com:kavilla/…

3352f97

…OpenSearch-Dashboards-1 into kavilla/discovermultiplequeries

kavilla dismissed stale reviews from joshuali925, sejli, and abbyhu2000 via 3352f97 September 26, 2024 22:32

sejli approved these changes Sep 26, 2024

View reviewed changes

abbyhu2000 approved these changes Sep 26, 2024

View reviewed changes

kavilla merged commit 0882435 into opensearch-project:main Sep 26, 2024
63 of 67 checks passed

opensearch-trigger-bot bot mentioned this pull request Sep 26, 2024

[Backport 2.x] [discover] fix too many queries sent #8361

Merged

SuZhou-Joe added the in-next label Oct 3, 2024

sejli mentioned this pull request Oct 8, 2024

Fix Loading and Editing Saved Searches in Discover #8306

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[discover] fix too many queries sent #8347

[discover] fix too many queries sent #8347

kavilla commented Sep 25, 2024 •

edited

Loading

github-actions bot commented Sep 25, 2024

codecov bot commented Sep 25, 2024 •

edited

Loading

sejli left a comment

sejli Sep 26, 2024

kavilla Sep 26, 2024

abbyhu2000 Sep 26, 2024

kavilla Sep 26, 2024 •

edited

Loading

joshuali925 left a comment

kavilla commented Sep 26, 2024

[discover] fix too many queries sent #8347

[discover] fix too many queries sent #8347

Conversation

kavilla commented Sep 25, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

github-actions bot commented Sep 25, 2024

❌ Empty Changelog Section

codecov bot commented Sep 25, 2024 • edited Loading

Codecov Report

sejli left a comment

Choose a reason for hiding this comment

sejli Sep 26, 2024

Choose a reason for hiding this comment

kavilla Sep 26, 2024

Choose a reason for hiding this comment

abbyhu2000 Sep 26, 2024

Choose a reason for hiding this comment

kavilla Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

joshuali925 left a comment

Choose a reason for hiding this comment

kavilla commented Sep 26, 2024

kavilla commented Sep 25, 2024 •

edited

Loading

codecov bot commented Sep 25, 2024 •

edited

Loading

kavilla Sep 26, 2024 •

edited

Loading