[discover] async query and caching #7943

kavilla · 2024-08-30T12:39:58Z

Description

Async query feature for S3 type on Discover. Due to the time to click and verify I end up implementing the cache to avoid waiting too long per every re-build.

What this PR does:

Poll for query
Cache data structures and refetches them from session storage
Poll based on the data type

What this PR does NOT do yet:

SessionId for search strategy, working fine for selector
isCacheable field property
Abort signal on server

Issues Resolved

n/a

Screenshot

Testing the changes

Changelog

feat: async query search and caching, also adding tests to related components

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

github-actions · 2024-08-30T12:40:32Z

❌ 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.

kavilla · 2024-08-30T12:41:52Z

src/plugins/data/public/plugin.ts

+    this.storage = createStorage({ engine: window.localStorage, prefix: 'opensearchDashboards.' });
+    this.sessionStorage = createStorage({
+      engine: window.sessionStorage,
+      prefix: 'opensearchDashboards.',


this prefix has to be opensearchDashboards. BWC

codecov · 2024-08-30T12:54:24Z

Codecov Report

Attention: Patch coverage is 25.67568% with 55 lines in your changes missing coverage. Please review.

Project coverage is 61.10%. Comparing base (edabe3b) to head (eeb0dec).
Report is 99 commits behind head on main.

Files with missing lines	Patch %	Lines
src/plugins/query_enhancements/common/utils.ts	20.00%	16 Missing ⚠️
...gins/query_enhancements/public/datasets/s3_type.ts	6.66%	14 Missing ⚠️
...ry/query_string/dataset_service/dataset_service.ts	52.17%	9 Missing and 2 partials ⚠️
...c/plugins/query_enhancements/server/utils/facet.ts	0.00%	8 Missing ⚠️
.../data/common/search/search_source/search_source.ts	0.00%	4 Missing ⚠️
src/plugins/data/public/plugin.ts	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7943      +/-   ##
==========================================
+ Coverage   61.03%   61.10%   +0.06%     
==========================================
  Files        3688     3691       +3     
  Lines       87232    87309      +77     
  Branches    13425    13433       +8     
==========================================
+ Hits        53241    53348     +107     
+ Misses      30763    30713      -50     
- Partials     3228     3248      +20

Flag	Coverage Δ
Linux_1	`29.32% <0.00%> (-0.02%)`	⬇️
Linux_2	`56.24% <0.00%> (-0.01%)`	⬇️
Linux_3	`?`
Linux_4	`29.59% <6.75%> (-0.01%)`	⬇️
Windows_1	`29.36% <0.00%> (-0.02%)`	⬇️
Windows_2	`56.19% <0.00%> (-0.01%)`	⬇️
Windows_3	`37.93% <45.16%> (+0.13%)`	⬆️
Windows_4	`29.59% <6.75%> (-0.01%)`	⬇️

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.

ashwin-pc · 2024-08-30T18:26:48Z

src/plugins/data/public/plugin.ts


  constructor(initializerContext: PluginInitializerContext<ConfigSchema>) {
    this.searchService = new SearchService(initializerContext);
    this.uiService = new UiService(initializerContext);
    this.queryService = new QueryService();
    this.fieldFormatsService = new FieldFormatsService();
    this.autocomplete = new AutocompleteService(initializerContext);
-    this.storage = createStorage({ engine: window.localStorage, prefix: 'opensearch_dashboards.' });
+    this.storage = createStorage({ engine: window.localStorage, prefix: 'opensearchDashboards.' });


Lets not change the key. Else users stored cache will be wiped out with a version update

correct, the original prefix was opensearchDashboards. so this was restoring it back to the older key.

ashwin-pc · 2024-08-30T18:29:16Z

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

+    const cacheKey = `${dataType}.${lastPathItem.id}`;
+
+    const cachedDataStructure = this.sessionStorage.get<CachedDataStructure>(cacheKey);
+    if (cachedDataStructure.children?.length > 0) {


What if the datastructure has no children?

should attempt the fetch correct? we could check hasNext but to me in makes sense to refetch always if no children.

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

ashwin-pc · 2024-08-30T18:58:15Z

src/plugins/query_enhancements/public/search/sql_search_interceptor.ts

-    const query = this.buildQuery(strategy);
-
-    return fetch(context, query).pipe(
+    return fetch(context, this.queryService.queryString.getQuery()).pipe(


doesnt the request object already have the whole query? why ae we fetching the query from the service?

you are right. this is an existing issue i will need to fix in a subsequent PR.

github-actions · 2024-08-30T21:31:03Z

❌ 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.

ps48 · 2024-08-30T22:14:21Z

src/plugins/query_enhancements/common/utils.ts

+export const buildQueryStatusConfig = (response: any) => {
+  return {
+    queryId: response.data.queryId,
+    sessionId: response.data.sessionId,


Can we please make sure the sessionId is stored per - MDS cluster and flint datasource?

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

LDrago27 · 2024-09-03T21:27:58Z

src/plugins/data/common/search/search_source/search_source.ts

@@ -440,11 +440,15 @@ export class SearchSource {
          await this.setDataFrame(dataFrameResponse.body as IDataFrame);
          return onResponse(searchRequest, convertResult(response as IDataFrameResponse));
        }
+        if ((response as IDataFrameResponse).type === DATA_FRAME_TYPES.POLLING) {


nit: Can we combine both the if condition
(response as IDataFrameResponse).type === DATA_FRAME_TYPES.DEFAULT) || (response as IDataFrameResponse).type === DATA_FRAME_TYPES.POLLING)
since the body of the both the condition are same.

yes good catch. i put this here but i'm planning on removing this type difference

LDrago27 · 2024-09-03T21:37:53Z

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

      if (temporaryIndexPattern) {
        this.indexPatterns?.saveToCache(dataset.id, temporaryIndexPattern);
      }
    }
  }

-  public fetchOptions(
+  public async fetchOptions(


Do we want to give an option to users to explicitly cache burst fetchOptions ?

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

joshuali925 · 2024-09-03T21:59:23Z

src/plugins/query_enhancements/public/datasets/s3_type.ts

+  try {
+    const response = await handleQueryStatus({
+      fetchStatus: () =>
+        http.fetch('../../api/enhancements/datasource/jobs', {


why do we need ../../?

kavilla · 2024-09-03T22:02:23Z

will address the comments in fast follows

opensearch-trigger-bot · 2024-09-03T22:03:25Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-7943-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8c9abe2e32ccf7a47c0998d574c68b2ead111026
# Push it to GitHub
git push --set-upstream origin backport/backport-7943-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7943-to-2.x.

opensearch-trigger-bot · 2024-09-03T22:11:11Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-7943-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8c9abe2e32ccf7a47c0998d574c68b2ead111026
# Push it to GitHub
git push --set-upstream origin backport/backport-7943-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7943-to-2.x.

Async query feature for S3 type on Discover. Due to the time to click and verify I end up implementing the cache to avoid waiting too long per every re-build. What this PR does: Poll for query Cache data structures and refetches them from session storage Poll based on the data type What this PR does NOT do yet: SessionId for search strategy, working fine for selector isCacheable field property Abort signal on server --------- 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 8c9abe2)

AMoo-Miki · 2024-09-13T16:05:01Z

Manually backported to 2.x with #7979

github-actions bot added the distinguished-contributor label Aug 30, 2024

github-actions bot added the failed changeset label Aug 30, 2024

kavilla commented Aug 30, 2024

View reviewed changes

ashwin-pc reviewed Aug 30, 2024

View reviewed changes

ps48 reviewed Aug 30, 2024

View reviewed changes

kavilla added discover for discover reinvent failed changeset v2.17.0 discover-next labels Aug 31, 2024

Merge branch 'main' into kavilla/s3_query

1d168ab

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

kavilla removed the failed changeset label Sep 3, 2024

abbyhu2000 previously approved these changes Sep 3, 2024

View reviewed changes

LDrago27 previously approved these changes Sep 3, 2024

View reviewed changes

Merge branch 'main' into kavilla/s3_query

eeb0dec

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

kavilla dismissed stale reviews from LDrago27 and abbyhu2000 via eeb0dec September 3, 2024 21:54

abbyhu2000 approved these changes Sep 3, 2024

View reviewed changes

LDrago27 approved these changes Sep 3, 2024

View reviewed changes

joshuali925 approved these changes Sep 3, 2024

View reviewed changes

joshuali925 reviewed Sep 3, 2024

View reviewed changes

kavilla merged commit 8c9abe2 into opensearch-project:main Sep 3, 2024
52 of 54 checks passed

opensearch-trigger-bot bot added the failed backport label Sep 3, 2024

kavilla added backport 2.x and removed backport 2.x failed backport labels Sep 3, 2024

opensearch-trigger-bot bot added the failed backport label Sep 3, 2024

kavilla mentioned this pull request Sep 3, 2024

[backport 2.x] [discover] async query and caching (#7943) #7979

Merged

AMoo-Miki removed the failed backport label Sep 13, 2024

AMoo-Miki added the in-next label Sep 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[discover] async query and caching #7943

[discover] async query and caching #7943

kavilla commented Aug 30, 2024 •

edited

Loading

github-actions bot commented Aug 30, 2024

kavilla Aug 30, 2024

codecov bot commented Aug 30, 2024 •

edited

Loading

ashwin-pc Aug 30, 2024

kavilla Aug 30, 2024

ashwin-pc Aug 30, 2024

kavilla Aug 30, 2024

ashwin-pc Aug 30, 2024

kavilla Aug 31, 2024

github-actions bot commented Aug 30, 2024

ps48 Aug 30, 2024

LDrago27 Sep 3, 2024

kavilla Sep 3, 2024

LDrago27 Sep 3, 2024

joshuali925 Sep 3, 2024

kavilla commented Sep 3, 2024

opensearch-trigger-bot bot commented Sep 3, 2024

opensearch-trigger-bot bot commented Sep 3, 2024

AMoo-Miki commented Sep 13, 2024

[discover] async query and caching #7943

[discover] async query and caching #7943

Conversation

kavilla commented Aug 30, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

github-actions bot commented Aug 30, 2024

❌ Empty Changelog Section

Choose a reason for hiding this comment

codecov bot commented Aug 30, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 30, 2024

❌ Empty Changelog Section

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla commented Sep 3, 2024

opensearch-trigger-bot bot commented Sep 3, 2024

opensearch-trigger-bot bot commented Sep 3, 2024

AMoo-Miki commented Sep 13, 2024

kavilla commented Aug 30, 2024 •

edited

Loading

codecov bot commented Aug 30, 2024 •

edited

Loading