From 896ea68358271762070bf0d5e3bfb5ae35a79c31 Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Mon, 19 Apr 2021 18:17:57 -0400 Subject: [PATCH] [Lens] Faster field existence failures by adding timeouts (#97188) * [Lens] Faster field existence failures by adding timeouts * Increase shard timeout and add timeout-specific warning * Fix types * Fix import * Hide field info when in error state, but not timeout Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../datapanel.test.tsx | 25 +++++++ .../indexpattern_datasource/datapanel.tsx | 15 ++-- .../indexpattern_datasource/field_list.tsx | 3 + .../fields_accordion.tsx | 59 +++++++++++----- .../indexpattern_datasource/loader.test.ts | 52 ++++++++++++++ .../public/indexpattern_datasource/loader.ts | 6 +- .../public/indexpattern_datasource/types.ts | 1 + .../lens/server/routes/existing_fields.ts | 70 ++++++++++++------- 8 files changed, 181 insertions(+), 50 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.test.tsx index e6a38ce2bb7130..6c5116436dddb1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.test.tsx @@ -9,6 +9,7 @@ import React, { ChangeEvent, ReactElement } from 'react'; import { createMockedDragDropContext } from './mocks'; import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; import { InnerIndexPatternDataPanel, IndexPatternDataPanel, MemoizedDataPanel } from './datapanel'; +import { FieldList } from './field_list'; import { FieldItem } from './field_item'; import { NoFieldsCallout } from './no_fields_callout'; import { act } from 'react-dom/test-utils'; @@ -713,6 +714,30 @@ describe('IndexPattern Data Panel', () => { expect(wrapper.find(NoFieldsCallout).length).toEqual(2); }); + it('should not allow field details when error', () => { + const wrapper = mountWithIntl( + + ); + + expect(wrapper.find(FieldList).prop('fieldGroups')).toEqual( + expect.objectContaining({ + AvailableFields: expect.objectContaining({ hideDetails: true }), + }) + ); + }); + + it('should allow field details when timeout', () => { + const wrapper = mountWithIntl( + + ); + + expect(wrapper.find(FieldList).prop('fieldGroups')).toEqual( + expect.objectContaining({ + AvailableFields: expect.objectContaining({ hideDetails: false }), + }) + ); + }); + it('should filter down by name', () => { const wrapper = mountWithIntl(); act(() => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx index 1b7c8d64de36ed..9fd389d4e65d3e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx @@ -230,6 +230,7 @@ export function IndexPatternDataPanel({ onUpdateIndexPattern={onUpdateIndexPattern} existingFields={state.existingFields} existenceFetchFailed={state.existenceFetchFailed} + existenceFetchTimeout={state.existenceFetchTimeout} dropOntoWorkspace={dropOntoWorkspace} hasSuggestionForField={hasSuggestionForField} /> @@ -271,6 +272,7 @@ export const InnerIndexPatternDataPanel = function InnerIndexPatternDataPanel({ indexPatternRefs, indexPatterns, existenceFetchFailed, + existenceFetchTimeout, query, dateRange, filters, @@ -297,6 +299,7 @@ export const InnerIndexPatternDataPanel = function InnerIndexPatternDataPanel({ charts: ChartsPluginSetup; indexPatternFieldEditor: IndexPatternFieldEditorStart; existenceFetchFailed?: boolean; + existenceFetchTimeout?: boolean; }) { const [localState, setLocalState] = useState({ nameFilter: '', @@ -314,7 +317,8 @@ export const InnerIndexPatternDataPanel = function InnerIndexPatternDataPanel({ (type) => type in fieldTypeNames ); - const fieldInfoUnavailable = existenceFetchFailed || currentIndexPattern.hasRestrictions; + const fieldInfoUnavailable = + existenceFetchFailed || existenceFetchTimeout || currentIndexPattern.hasRestrictions; const editPermission = indexPatternFieldEditor.userPermissions.editIndexPattern(); @@ -389,7 +393,8 @@ export const InnerIndexPatternDataPanel = function InnerIndexPatternDataPanel({ }), isAffectedByGlobalFilter: !!filters.length, isAffectedByTimeFilter: true, - hideDetails: fieldInfoUnavailable, + // Show details on timeout but not failure + hideDetails: fieldInfoUnavailable && !existenceFetchTimeout, defaultNoFieldsMessage: i18n.translate('xpack.lens.indexPatterns.noAvailableDataLabel', { defaultMessage: `There are no available fields that contain data.`, }), @@ -438,11 +443,12 @@ export const InnerIndexPatternDataPanel = function InnerIndexPatternDataPanel({ return fieldGroupDefinitions; }, [ allFields, - existingFields, - currentIndexPattern, hasSyncedExistingFields, fieldInfoUnavailable, filters.length, + existenceFetchTimeout, + currentIndexPattern, + existingFields, ]); const fieldGroups: FieldGroups = useMemo(() => { @@ -794,6 +800,7 @@ export const InnerIndexPatternDataPanel = function InnerIndexPatternDataPanel({ filter={filter} currentIndexPatternId={currentIndexPatternId} existenceFetchFailed={existenceFetchFailed} + existenceFetchTimeout={existenceFetchTimeout} existFieldsInIndex={!!allFields.length} dropOntoWorkspace={dropOntoWorkspace} hasSuggestionForField={hasSuggestionForField} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/field_list.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/field_list.tsx index ceeb1f5b1caf3a..ee0011ad0390cb 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/field_list.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/field_list.tsx @@ -45,6 +45,7 @@ export const FieldList = React.memo(function FieldList({ exists, fieldGroups, existenceFetchFailed, + existenceFetchTimeout, fieldProps, hasSyncedExistingFields, filter, @@ -60,6 +61,7 @@ export const FieldList = React.memo(function FieldList({ fieldProps: FieldItemSharedProps; hasSyncedExistingFields: boolean; existenceFetchFailed?: boolean; + existenceFetchTimeout?: boolean; filter: { nameFilter: string; typeFilter: string[]; @@ -194,6 +196,7 @@ export const FieldList = React.memo(function FieldList({ ); }} showExistenceFetchError={existenceFetchFailed} + showExistenceFetchTimeout={existenceFetchTimeout} renderCallout={ boolean; showExistenceFetchError?: boolean; + showExistenceFetchTimeout?: boolean; hideDetails?: boolean; groupIndex: number; dropOntoWorkspace: DatasourceDataPanelProps['dropOntoWorkspace']; @@ -73,6 +74,7 @@ export const FieldsAccordion = memo(function InnerFieldsAccordion({ exists, hideDetails, showExistenceFetchError, + showExistenceFetchTimeout, groupIndex, dropOntoWorkspace, hasSuggestionForField, @@ -133,25 +135,44 @@ export const FieldsAccordion = memo(function InnerFieldsAccordion({ }, [label, helpTooltip]); const extraAction = useMemo(() => { - return showExistenceFetchError ? ( - - ) : hasLoaded ? ( - - {fieldsCount} - - ) : ( - - ); - }, [showExistenceFetchError, hasLoaded, isFiltered, fieldsCount]); + if (showExistenceFetchError) { + return ( + + ); + } + if (showExistenceFetchTimeout) { + return ( + + ); + } + if (hasLoaded) { + return ( + + {fieldsCount} + + ); + } + + return ; + }, [showExistenceFetchError, showExistenceFetchTimeout, hasLoaded, isFiltered, fieldsCount]); return ( { foo: 'bar', isFirstExistenceFetch: false, existenceFetchFailed: false, + existenceFetchTimeout: false, existingFields: { '1': { ip1_field_1: true, ip1_field_2: true }, '2': { ip2_field_1: true, ip2_field_2: true }, @@ -957,6 +959,56 @@ describe('loader', () => { }) as IndexPatternPrivateState; expect(newState.existenceFetchFailed).toEqual(true); + expect(newState.existenceFetchTimeout).toEqual(false); + expect(newState.existingFields['1']).toEqual({ + field1: true, + field2: true, + }); + }); + + it('should set all fields to available and existence error flag if the request times out', async () => { + const setState = jest.fn(); + const fetchJson = (jest.fn((path: string) => { + return new Promise((resolve, reject) => { + reject( + new HttpFetchError( + 'timeout', + 'name', + ({} as unknown) as Request, + ({ status: 408 } as unknown) as Response + ) + ); + }); + }) as unknown) as HttpHandler; + + const args = { + dateRange: { fromDate: '1900-01-01', toDate: '2000-01-01' }, + fetchJson, + indexPatterns: [ + { + id: '1', + title: '1', + hasRestrictions: false, + fields: [{ name: 'field1' }, { name: 'field2' }] as IndexPatternField[], + }, + ], + setState, + dslQuery, + showNoDataPopover: jest.fn(), + currentIndexPatternTitle: 'abc', + isFirstExistenceFetch: false, + }; + + await syncExistingFields(args); + + const [fn] = setState.mock.calls[0]; + const newState = fn({ + foo: 'bar', + existingFields: {}, + }) as IndexPatternPrivateState; + + expect(newState.existenceFetchFailed).toEqual(false); + expect(newState.existenceFetchTimeout).toEqual(true); expect(newState.existingFields['1']).toEqual({ field1: true, field2: true, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts index ec7ef6a37a27af..0eb661e92bb1d9 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts @@ -445,16 +445,18 @@ export async function syncExistingFields({ ...state, isFirstExistenceFetch: false, existenceFetchFailed: false, + existenceFetchTimeout: false, existingFields: emptinessInfo.reduce((acc, info) => { acc[info.indexPatternTitle] = booleanMap(info.existingFieldNames); return acc; }, state.existingFields), })); } catch (e) { - // show all fields as available if fetch failed + // show all fields as available if fetch failed or timed out setState((state) => ({ ...state, - existenceFetchFailed: true, + existenceFetchFailed: e.res?.status !== 408, + existenceFetchTimeout: e.res?.status === 408, existingFields: indexPatterns.reduce((acc, pattern) => { acc[pattern.title] = booleanMap(pattern.fields.map((field) => field.name)); return acc; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/types.ts index 18f653c588ee83..98dc767c44c7dd 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/types.ts @@ -87,6 +87,7 @@ export interface IndexPatternPrivateState { existingFields: Record>; isFirstExistenceFetch: boolean; existenceFetchFailed?: boolean; + existenceFetchTimeout?: boolean; } export interface IndexPatternRef { diff --git a/x-pack/plugins/lens/server/routes/existing_fields.ts b/x-pack/plugins/lens/server/routes/existing_fields.ts index 2e6d6128352315..d775113d83ff7b 100644 --- a/x-pack/plugins/lens/server/routes/existing_fields.ts +++ b/x-pack/plugins/lens/server/routes/existing_fields.ts @@ -68,8 +68,15 @@ export async function existingFieldsRoute(setup: CoreSetup, }), }); } catch (e) { + if (e instanceof errors.TimeoutError) { + logger.info(`Field existence check timed out on ${req.params.indexPatternId}`); + // 408 is Request Timeout + return res.customError({ statusCode: 408, body: e.message }); + } logger.info( - `Field existence check failed: ${isBoomError(e) ? e.output.payload.message : e.message}` + `Field existence check failed on ${req.params.indexPatternId}: ${ + isBoomError(e) ? e.output.payload.message : e.message + }` ); if (e instanceof errors.ResponseError && e.statusCode === 404) { return res.notFound({ body: e.message }); @@ -182,31 +189,44 @@ async function fetchIndexPatternStats({ const scriptedFields = fields.filter((f) => f.isScript); const runtimeFields = fields.filter((f) => f.runtimeField); - const { body: result } = await client.search({ - index, - body: { - size: SAMPLE_SIZE, - query, - sort: timeFieldName && fromDate && toDate ? [{ [timeFieldName]: 'desc' }] : [], - fields: ['*'], - _source: false, - runtime_mappings: runtimeFields.reduce((acc, field) => { - if (!field.runtimeField) return acc; - // @ts-expect-error @elastic/elasticsearch StoredScript.language is required - acc[field.name] = field.runtimeField; - return acc; - }, {} as Record), - script_fields: scriptedFields.reduce((acc, field) => { - acc[field.name] = { - script: { - lang: field.lang!, - source: field.script!, - }, - }; - return acc; - }, {} as Record), + const { body: result } = await client.search( + { + index, + body: { + size: SAMPLE_SIZE, + query, + // Sorted queries are usually able to skip entire shards that don't match + sort: timeFieldName && fromDate && toDate ? [{ [timeFieldName]: 'desc' }] : [], + fields: ['*'], + _source: false, + runtime_mappings: runtimeFields.reduce((acc, field) => { + if (!field.runtimeField) return acc; + // @ts-expect-error @elastic/elasticsearch StoredScript.language is required + acc[field.name] = field.runtimeField; + return acc; + }, {} as Record), + script_fields: scriptedFields.reduce((acc, field) => { + acc[field.name] = { + script: { + lang: field.lang!, + source: field.script!, + }, + }; + return acc; + }, {} as Record), + // Small improvement because there is overhead in counting + track_total_hits: false, + // Per-shard timeout, must be lower than overall. Shards return partial results on timeout + timeout: '4500ms', + }, }, - }); + { + // Global request timeout. Will cancel the request if exceeded. Overrides the elasticsearch.requestTimeout + requestTimeout: '5000ms', + // Fails fast instead of retrying- default is to retry + maxRetries: 0, + } + ); return result.hits.hits; }