Skip to content

Commit

Permalink
[8.14] [ML] Fix runtime fields occasionally cause redundant refetch o…
Browse files Browse the repository at this point in the history
…f data in Index data visualizer (elastic#181853) (elastic#181906)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[ML] Fix runtime fields occasionally cause redundant refetch of data
in Index data visualizer
(elastic#181853)](elastic#181853)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Quynh Nguyen
(Quinn)","email":"43350163+qn895@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-04-26T17:29:51Z","message":"[ML]
Fix runtime fields occasionally cause redundant refetch of data in Index
data visualizer (elastic#181853)\n\n## Summary\r\n\r\nThe runtime fields object
occasionally cause redundant update to the\r\nhook, which triggers a
refetch of data in Index data visualizer. This PR\r\nfixes
that.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"7d94898976a335fd36fc30e097d238a815e56b2c","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug",":ml","release_note:skip","v8.14.0","v8.15.0"],"title":"[ML]
Fix runtime fields occasionally cause redundant refetch of data in Index
data
visualizer","number":181853,"url":"https://github.com/elastic/kibana/pull/181853","mergeCommit":{"message":"[ML]
Fix runtime fields occasionally cause redundant refetch of data in Index
data visualizer (elastic#181853)\n\n## Summary\r\n\r\nThe runtime fields object
occasionally cause redundant update to the\r\nhook, which triggers a
refetch of data in Index data visualizer. This PR\r\nfixes
that.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"7d94898976a335fd36fc30e097d238a815e56b2c"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181853","number":181853,"mergeCommit":{"message":"[ML]
Fix runtime fields occasionally cause redundant refetch of data in Index
data visualizer (elastic#181853)\n\n## Summary\r\n\r\nThe runtime fields object
occasionally cause redundant update to the\r\nhook, which triggers a
refetch of data in Index data visualizer. This PR\r\nfixes
that.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"7d94898976a335fd36fc30e097d238a815e56b2c"}}]}]
BACKPORT-->

Co-authored-by: Quynh Nguyen (Quinn) <43350163+qn895@users.noreply.github.com>
  • Loading branch information
kibanamachine and qn895 authored Apr 26, 2024
1 parent e5cf41a commit 5cee001
Showing 1 changed file with 36 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { getProcessedFields } from '@kbn/ml-data-grid';
import { buildBaseFilterCriteria } from '@kbn/ml-query-utils';
import { isDefined } from '@kbn/ml-is-defined';
import type { FieldSpec } from '@kbn/data-views-plugin/common';
import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { useDataVisualizerKibana } from '../../kibana_context';
import type {
AggregatableFieldOverallStats,
Expand Down Expand Up @@ -51,6 +52,18 @@ import {
fetchDataWithTimeout,
rateLimitingForkJoin,
} from '../search_strategy/requests/fetch_utils';

const getPopulatedFieldsInIndex = (
populatedFieldsInIndexWithoutRuntimeFields: Set<string> | undefined | null,
runtimeFieldMap: MappingRuntimeFields | undefined
): Set<string> | undefined | null => {
if (!populatedFieldsInIndexWithoutRuntimeFields) return undefined;
const runtimeFields = runtimeFieldMap ? Object.keys(runtimeFieldMap) : undefined;
return runtimeFields && runtimeFields?.length > 0
? new Set([...Array.from(populatedFieldsInIndexWithoutRuntimeFields), ...runtimeFields])
: populatedFieldsInIndexWithoutRuntimeFields;
};

export function useOverallStats<TParams extends OverallStatsSearchStrategyParams>(
esql = false,
searchStrategyParams: TParams | undefined,
Expand All @@ -68,7 +81,7 @@ export function useOverallStats<TParams extends OverallStatsSearchStrategyParams
} = useDataVisualizerKibana();

const [stats, setOverallStats] = useState<OverallStats>(getDefaultPageState().overallStats);
const [populatedFieldsInIndex, setPopulatedFieldsInIndex] = useState<
const [populatedFieldsInIndexWithoutRuntimeFields, setPopulatedFieldsInIndex] = useState<
| Set<string>
// request to fields caps has not been made yet
| undefined
Expand All @@ -92,10 +105,9 @@ export function useOverallStats<TParams extends OverallStatsSearchStrategyParams

// If null, that means we tried to fetch populated fields already but it timed out
// so don't try again
if (!searchStrategyParams || populatedFieldsInIndex === null) return;
if (!searchStrategyParams || populatedFieldsInIndexWithoutRuntimeFields === null) return;

const { index, searchQuery, timeFieldName, earliest, latest, runtimeFieldMap } =
searchStrategyParams;
const { index, searchQuery, timeFieldName, earliest, latest } = searchStrategyParams;

const fetchPopulatedFields = async () => {
populatedFieldsAbortCtrl.current.abort();
Expand Down Expand Up @@ -131,15 +143,7 @@ export function useOverallStats<TParams extends OverallStatsSearchStrategyParams

if (!unmounted) {
if (Array.isArray(nonEmptyFields)) {
setPopulatedFieldsInIndex(
new Set([
...nonEmptyFields.map((field) => field.name),
// Field caps API don't know about runtime fields
// so by default we expect runtime fields to be populated
// so we can later check as needed
...Object.keys(runtimeFieldMap ?? {}),
])
);
setPopulatedFieldsInIndex(new Set([...nonEmptyFields.map((field) => field.name)]));
} else {
setPopulatedFieldsInIndex(null);
}
Expand All @@ -154,25 +158,33 @@ export function useOverallStats<TParams extends OverallStatsSearchStrategyParams
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[
data.dataViews,
searchStrategyParams?.timeFieldName,
searchStrategyParams?.earliest,
searchStrategyParams?.latest,
searchStrategyParams?.searchQuery,
searchStrategyParams?.index,
searchStrategyParams?.runtimeFieldMap,
]
);

const startFetch = useCallback(async () => {
try {
searchSubscription$.current?.unsubscribe();
abortCtrl.current.abort();
abortCtrl.current = new AbortController();

if (!searchStrategyParams || lastRefresh === 0 || populatedFieldsInIndex === undefined) {
if (
!searchStrategyParams ||
lastRefresh === 0 ||
populatedFieldsInIndexWithoutRuntimeFields === undefined
) {
return;
}

const populatedFieldsInIndex = getPopulatedFieldsInIndex(
populatedFieldsInIndexWithoutRuntimeFields,
searchStrategyParams.runtimeFieldMap
);

setFetchState({
...getInitialProgress(),
isRunning: true,
Expand Down Expand Up @@ -391,7 +403,14 @@ export function useOverallStats<TParams extends OverallStatsSearchStrategyParams
displayError(toasts, searchStrategyParams!.index, extractErrorProperties(error));
}
}
}, [data, searchStrategyParams, toasts, lastRefresh, probability, populatedFieldsInIndex]);
}, [
data,
searchStrategyParams,
toasts,
lastRefresh,
probability,
populatedFieldsInIndexWithoutRuntimeFields,
]);

const cancelFetch = useCallback(() => {
searchSubscription$.current?.unsubscribe();
Expand Down

0 comments on commit 5cee001

Please sign in to comment.