[Discover] [Group By] Support group by fetching across tabs by davismcphee · Pull Request #251154 · elastic/kibana

davismcphee · 2026-01-31T03:38:11Z

Summary

This PR implements support for group by fetching across Discover tabs, by detaching the data fetching logic from the React tree and consolidating it in a new CascadedDocumentsFetcher. This allows for the following behaviours:

The active fetch is cancelled when a group is expanded then collapsed while the fetch is running.
The active fetch is not cancelled when switching Discover tabs while the fetch is running, allowing it to run to completion in the background.
When returning to a Discover tab and re-expanding a previously fetched group, its records are no longer re-fetched and instead served from a cache.
All fetched groups are cleared when triggering a top-level Discover fetch, ensuring we don't show stale data in groups.
All active fetches are cancelled when leaving Discover or triggering a top-level Discover fetch, ensuring they aren't accidentally left running.

Group by UI state is not currently retained when switching Discover tabs, that will be handled in a followup.

Resolves #249455.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
Review the backport guidelines and apply applicable backport:* labels.

…e dependencies on global services, which can trigger when switching tabs

kibanamachine · 2026-02-11T15:37:37Z

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#10718

[✅] src/platform/test/functional/apps/discover/cascade_layout/config.ts: 25/25 tests passed.

see run history

davismcphee · 2026-02-11T16:39:31Z

+const EMPTY_FILTERS: Filter[] = [];
+
+export const selectTabCombinedFilters = createSelector(
+  [
+    (tab: TabState) => tab.globalState.filters,
+    (tab: TabState) => tab.appState.filters,
+    (tab: TabState) => tab.appState.query,
+  ],
+  (globalFilters, appFilters, query) => {
+    if (isOfAggregateQueryType(query)) {
+      return EMPTY_FILTERS;
+    }
+    const allFilters = [...(globalFilters ?? []), ...(appFilters ?? [])];
+    return allFilters.length ? cloneDeep(allFilters) : EMPTY_FILTERS;
+  }
+);


I encountered an issue when switching Discover tabs where an additional fetch was triggered for the expanded group because we were relying on global services for filters, which are updated during tab change transition.

Technically it was fixed by stabilizing the handlers in useDataCascadeRowExpansionHandlers, but I had already made this change and figured it would be a good idea to standardize on a non-global service approach to selecting all filters to prevent similar issues in the future. I believe I updated all areas in Discover where this is done to use the new memoized selector.

Thanks for cleaning up the filters setup!

elasticmachine · 2026-02-11T16:49:46Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

AlexGPlay · 2026-02-13T09:09:44Z

+    dataView,
+    timeRange,
+  }: FetchCascadedDocumentsParams) {
+    this.cancelFetch(nodeId);


q1: this is because we can't have multiple cascades open at the same time so we cancel the current one?
q2: if we go back to a cascade that was already fetched this will just remove the abort controller from the map but nothing will really happen?

this is because we can't have multiple cascades open at the same time so we cancel the current one?

Yes only one group at a time, but cancelling in-flight requests should already be handled via external cancelFetch calls. This is mainly just a safeguard against duplicate fetch attempts (shouldn't really happen though).

if we go back to a cascade that was already fetched this will just remove the abort controller from the map but nothing will really happen?

Exactly, it should be a no-op for already fetched groups. However, the abort controller is removed after the first fetch, so it would just check the map and return when it doesn't find one.

AlexGPlay · 2026-02-13T09:15:59Z

+    const abortController = this.abortControllers.get(nodeId);
+
+    if (abortController) {
+      abortController.abort();


really small thing: i guess it comes from this part, right now when aborting we get an error in the console

Yeah unfortunately I'm not sure we can fix this on our end atm. I originally caught errors and returned an empty array, but it caused the cascade components to cache the empty results. cc @eokoneyo in case we want to consider ways to improve the "failed fetch" case.

I'd take a look at this

AlexGPlay

manually tested + code lgtm but i'm not an expert on this so you may want a second review 🙈

not related to this task but it'd be nice to keep the cascades open between tabs navigations so when i come back from another tab the one i was taking a look at is still open

jughosta

LGTM and works great 👍

jughosta · 2026-02-13T10:31:09Z

+const EMPTY_FILTERS: Filter[] = [];
+
+export const selectTabCombinedFilters = createSelector(
+  [
+    (tab: TabState) => tab.globalState.filters,
+    (tab: TabState) => tab.appState.filters,
+    (tab: TabState) => tab.appState.query,
+  ],
+  (globalFilters, appFilters, query) => {
+    if (isOfAggregateQueryType(query)) {
+      return EMPTY_FILTERS;
+    }
+    const allFilters = [...(globalFilters ?? []), ...(appFilters ?? [])];
+    return allFilters.length ? cloneDeep(allFilters) : EMPTY_FILTERS;
+  }
+);


Thanks for cleaning up the filters setup!

jughosta · 2026-02-13T10:32:32Z

+      await expectCascadeRequestTimestampToChange(baseline, false);
+    });
+  });
+}


We could also extend by checking how many requests were triggered via discover.expectSearchRequestCount helper.

For context, I went with the inspector approach here so we also have e2e coverage for the custom request adapter changes. I took a shot at making it work with expectSearchRequestCount too, but was running into flakiness due to editor requests when changing tabs. I'm gonna merge this now to get it in, but open to revisiting and exploring expectSearchRequestCount again after.

elasticmachine · 2026-02-18T01:41:25Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 3203041

Failed CI Steps

Scout: [ platform / workflows_management ] plugin

Test Failures

[job] [logs] Scout: [ platform / workflows_management ] plugin / local-stateful-classic - Workflow execution - Foreach iterations - should display scrollable step executions with many iterations

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`discover`	2078	2080	+2

Async chunks

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

id	before	after	diff
`discover`	1.6MB	1.6MB	+341.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`discover`	25.1KB	25.1KB	-13.0B

Unknown metric groups

async chunk count

id	before	after	diff
`discover`	44	43	-1

History

💔 Build #396438 failed 23ed3c1
💔 Build #396098 failed 1c86e7f
💛 Build #395692 was flaky ab1285c
💛 Build #393319 was flaky fa8e8bb
💔 Build #390215 failed 20a3d07

cc @davismcphee

…251154) ## Summary This PR implements support for group by fetching across Discover tabs, by detaching the data fetching logic from the React tree and consolidating it in a new `CascadedDocumentsFetcher`. This allows for the following behaviours: - The active fetch is cancelled when a group is expanded then collapsed while the fetch is running. - The active fetch is not cancelled when switching Discover tabs while the fetch is running, allowing it to run to completion in the background. - When returning to a Discover tab and re-expanding a previously fetched group, its records are no longer re-fetched and instead served from a cache. - All fetched groups are cleared when triggering a top-level Discover fetch, ensuring we don't show stale data in groups. - All active fetches are cancelled when leaving Discover or triggering a top-level Discover fetch, ensuring they aren't accidentally left running. Group by UI state is not currently retained when switching Discover tabs, that will be handled in a followup. Resolves elastic#249455. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels.

davismcphee self-assigned this Jan 31, 2026

davismcphee added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// labels Jan 31, 2026

davismcphee force-pushed the discover-group-by-fetching branch 3 times, most recently from c499ba2 to 20a3d07 Compare February 5, 2026 04:06

davismcphee added 10 commits February 10, 2026 19:07

Add CascadedDocumentsFetcher and split fetching from rendering

a74f932

Clean up cascadedDocumentsStateManager

c036730

Add selectTabCombinedFilters to unify selecting tab filters and remov…

00eb6c5

…e dependencies on global services, which can trigger when switching tabs

Don't cancel cascade fetches when switching tabs / unmounting

10172b6

Use stable handlers for cascade data fetching

69f03f5

Remove EMPTY_FILTERS

862ed34

Add fetcher Jest tests

4cf3ba3

TODOs

e132530

Uncomment tests

d1a5d39

Fix tests

c3e922f

davismcphee force-pushed the discover-group-by-fetching branch from 20a3d07 to c3e922f Compare February 10, 2026 23:41

davismcphee added 6 commits February 10, 2026 19:45

Update translation

6e2b4fd

Add missing Jest test

b3a699e

Re-add request adapter handling

ff7b55d

Fix fetcher abort handling

a05a925

FTR tests

e4775a7

Merge branch 'main' into discover-group-by-fetching

fa8e8bb

davismcphee commented Feb 11, 2026

View reviewed changes

davismcphee marked this pull request as ready for review February 11, 2026 16:49

davismcphee requested a review from a team as a code owner February 11, 2026 16:49

Merge branch 'main' into discover-group-by-fetching

6af8813

AlexGPlay reviewed Feb 13, 2026

View reviewed changes

AlexGPlay approved these changes Feb 13, 2026

View reviewed changes

jughosta approved these changes Feb 13, 2026

View reviewed changes

davismcphee added 4 commits February 16, 2026 21:59

Merge branch 'main' into discover-group-by-fetching

ab1285c

Merge branch 'main' into discover-group-by-fetching

1c86e7f

Merge branch 'main' into discover-group-by-fetching

23ed3c1

Merge branch 'main' into discover-group-by-fetching

3203041

davismcphee merged commit a64911a into elastic:main Feb 18, 2026
16 checks passed

davismcphee deleted the discover-group-by-fetching branch February 18, 2026 01:42

kibanamachine added the v9.4.0 label Feb 18, 2026

davismcphee mentioned this pull request Feb 26, 2026

[Discover] [Group By] Redundant requests fired when switching tabs while nested table loading #249321

Closed

florent-leborgne mentioned this pull request Mar 11, 2026

[not ready] Document grouped view for ES|QL STATS results in Discover elastic/docs-content#5457

Closed

2 tasks

Conversation

davismcphee commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Feb 11, 2026

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#10718

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexGPlay left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 18, 2026

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

davismcphee commented Jan 31, 2026 •

edited

Loading

AlexGPlay left a comment •

edited

Loading