maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[SearchProfiler] Remove sources of recursion over potentially deeply nested objects (#54015)

Browse source 
* Added max tree depth guard
Removed recursive normalizeTimes functions (one fewer iteration through the entire data structure)
Optimizied appliation of tree mutations by taking `if` out of tight loop
Cleaned up types

* Tidy up data being passed into store (and through immer)

* Fix max tree depth logic

* Remove immer from non-test code.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Jean-Louis Leysens 2020-01-10 10:56:28 +01:00 committed by GitHub
parent 599af6a8e6
commit 753eb53448
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 948 additions and 748 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/highlight_details_flyout/highlight_details_flyout.test.tsx
View file

@ -11,16 +11,8 @@ describe('Highlight Details Flyout', () => {
it('renders', async () => {
const props: Props = {
onClose: () => {},
shard: {
aggregations: [],
id: ['test', 'test', 'test'],
searches: [],
color: '#fff',
time: 123,
relative: 100,
},
shardName: '[test][test]',
operation: {
parent: null,
breakdown: [
{
color: 'test',
@ -48,8 +40,7 @@ describe('Highlight Details Flyout', () => {
query_type: 'test',
selfTime: 100,
time: 100,
children: [],
timePercentage: 100,
timePercentage: '100',
hasChildren: false,
visible: true,
absoluteColor: '123',

12
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/highlight_details_flyout/highlight_details_flyout.tsx
View file

@ -17,11 +17,11 @@ import {
import { msToPretty } from '../../utils';
import { HighlightDetailsTable } from './highlight_details_table';
import { Operation, Shard } from '../../types';
import { Operation } from '../../types';
export interface Props {
operation: Operation;
shard: Shard;
operation: Omit<Operation, 'children' | 'parent'>;
shardName: string;
indexName: string;
onClose: () => void;
}
@ -39,14 +39,12 @@ const FlyoutEntry = ({
</>
);
export const HighlightDetailsFlyout = ({ indexName, operation, shard, onClose }: Props) => {
export const HighlightDetailsFlyout = ({ indexName, operation, shardName, onClose }: Props) => {
return (
<EuiFlyout className="prfDevTool__details" onClose={() => onClose()}>
<EuiFlyoutHeader hasBorder={true}>
<EuiText size="s">{indexName}</EuiText>
<EuiText>
[{/* shard id */ shard.id[0]}][{/* shard number */ shard.id[2]}]
</EuiText>
<EuiText>{shardName}</EuiText>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<EuiText>

2
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/highlight_details_flyout/highlight_details_table.tsx
View file

@ -36,7 +36,7 @@ export const HighlightDetailsTable = ({ breakdown }: Props) => {
{
name: 'Percentage',
render: (item: BreakdownItem) => (
<PercentageBadge timePercentage={item.relative as number} label={item.relative + '%'} />
<PercentageBadge timePercentage={String(item.relative)} label={item.relative + '%'} />
),
},
];

2
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/percentage_badge.tsx
View file

@ -9,7 +9,7 @@ import { EuiBadge } from '@elastic/eui';
import classNames from 'classnames';
interface Props {
timePercentage: number;
timePercentage: string;
label: string;
valueType?: 'percent' | 'time';
}

1185
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/__tests__/fixtures/normalize_times.ts
View file

File diff suppressed because it is too large Load diff

108
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/__tests__/fixtures/search_response.ts
View file

@ -182,6 +182,114 @@ export const searchResponse: any = [
create_weight_count: 1,
build_scorer: 17681,
},
children: [
{
type: 'DocValuesFieldExistsQuery',
description: 'DocValuesFieldExistsQuery [field=_primary_term]',
time_in_nanos: 19795,
breakdown: {
set_min_competitive_score_count: 0,
match_count: 0,
shallow_advance_count: 0,
set_min_competitive_score: 0,
next_doc: 600,
match: 0,
next_doc_count: 5,
score_count: 0,
compute_max_score_count: 0,
compute_max_score: 0,
advance: 378,
advance_count: 3,
score: 0,
build_scorer_count: 6,
create_weight: 1121,
shallow_advance: 0,
create_weight_count: 1,
build_scorer: 17681,
},
children: [
{
type: 'DocValuesFieldExistsQuery',
description: 'DocValuesFieldExistsQuery [field=_primary_term]',
time_in_nanos: 19795,
breakdown: {
set_min_competitive_score_count: 0,
match_count: 0,
shallow_advance_count: 0,
set_min_competitive_score: 0,
next_doc: 600,
match: 0,
next_doc_count: 5,
score_count: 0,
compute_max_score_count: 0,
compute_max_score: 0,
advance: 378,
advance_count: 3,
score: 0,
build_scorer_count: 6,
create_weight: 1121,
shallow_advance: 0,
create_weight_count: 1,
build_scorer: 17681,
},
children: [
{
type: 'DocValuesFieldExistsQuery',
description: 'DocValuesFieldExistsQuery [field=_primary_term]',
time_in_nanos: 19795,
breakdown: {
set_min_competitive_score_count: 0,
match_count: 0,
shallow_advance_count: 0,
set_min_competitive_score: 0,
next_doc: 600,
match: 0,
next_doc_count: 5,
score_count: 0,
compute_max_score_count: 0,
compute_max_score: 0,
advance: 378,
advance_count: 3,
score: 0,
build_scorer_count: 6,
create_weight: 1121,
shallow_advance: 0,
create_weight_count: 1,
build_scorer: 17681,
},
children: [
{
type: 'DocValuesFieldExistsQuery',
description: 'DocValuesFieldExistsQuery [field=_primary_term]',
time_in_nanos: 19795,
breakdown: {
set_min_competitive_score_count: 0,
match_count: 0,
shallow_advance_count: 0,
set_min_competitive_score: 0,
next_doc: 600,
match: 0,
next_doc_count: 5,
score_count: 0,
compute_max_score_count: 0,
compute_max_score: 0,
advance: 378,
advance_count: 3,
score: 0,
build_scorer_count: 6,
create_weight: 1121,
shallow_advance: 0,
create_weight_count: 1,
build_scorer: 17681,
},
},
],
},
],
},
],
},
],
},
],
},

13
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/__tests__/init_data.test.ts
View file

@ -5,6 +5,9 @@
*/
import { ShardSerialized } from '../../../types';
jest.mock('../constants', () => ({
MAX_TREE_DEPTH: 3,
}));
import { initDataFor } from '../init_data';
import { searchResponse } from './fixtures/search_response';
@ -15,6 +18,16 @@ describe('ProfileTree init data', () => {
const input: ShardSerialized[] = searchResponse as any;
const actual = initDataFor('searches')(input);
/* prettier-ignore */
expect(
actual[1].shards[0].searches![0]
.treeRoot! // level 0
.children[0] // level 1
.children[0] // level 2
.children[0] // level 3 -- Max level!
.children, // level 4 (nothing here!)
).toEqual([]);
expect(actual[0].name).toEqual(processedResponseWithFirstShard[0].name);
const expectedFirstShard = processedResponseWithFirstShard[0].shards[0];
expect(actual[0].shards[0]).toEqual(expectedFirstShard);

7
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/__tests__/unsafe_utils.test.ts
View file

@ -17,13 +17,16 @@ describe('normalizeBreakdown', function() {
});
});
describe('normalizeTimes', function() {
describe('normalizeTime', function() {
it('returns correct normalization', function() {
const totalTime = 0.447365;
// Deep clone the object to preserve the original
const input = JSON.parse(JSON.stringify(inputTimes));
util.normalizeTimes(input, totalTime, 0);
// Simulate recursive application to the tree.
input.forEach((i: any) => util.normalizeTime(i, totalTime));
input[0].children.forEach((i: any) => util.normalizeTime(i, totalTime));
// Modifies in place, so inputTimes will change
expect(input).to.eql(normalizedTimes);

8
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/constants.ts Normal file
View file

@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
// Prevent recursive data structures from blowing up the JS call stack.
export const MAX_TREE_DEPTH = 40;

110
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/init_data.ts
View file

@ -4,18 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { produce } from 'immer';
import cloneDeep from 'lodash.clonedeep';
import { flow } from 'fp-ts/lib/function';
import { Targets, Shard, ShardSerialized } from '../../types';
import { calcTimes, normalizeTimes, initTree, normalizeIndices, sortIndices } from './unsafe_utils';
import { calcTimes, initTree, normalizeIndices, sortIndices } from './unsafe_utils';
import { IndexMap } from './types';
/**
* Functions prefixed with "mutate" change values by reference. Be careful when using these!
*
* It's recommended to us immer's `produce` functions to ensure immutability.
*/
export function mutateAggsTimesTree(shard: Shard) {
if (shard.aggregations == null) {
shard.time = 0;
@ -26,8 +23,7 @@ export function mutateAggsTimesTree(shard: Shard) {
shardTime += totalTime;
}
for (const agg of shard.aggregations!) {
normalizeTimes([agg], shardTime, 0);
initTree([agg], 0);
initTree([agg], shardTime);
}
shard.time = shardTime;
}
@ -43,66 +39,76 @@ export function mutateSearchTimesTree(shard: Shard) {
shard.rewrite_time += search.rewrite_time!;
const totalTime = calcTimes(search.query!);
shardTime += totalTime;
normalizeTimes(search.query!, totalTime, 0);
initTree(search.query!, 0);
initTree(search.query!, totalTime);
search.treeRoot = search.query![0];
// Remove this object.
search.query = null as any;
}
shard.time = shardTime;
}
const initShards = (data: ShardSerialized[]) =>
produce<ShardSerialized[], Shard[]>(data, draft => {
return draft.map(s => {
const idMatch = s.id.match(/\[([^\]\[]*?)\]/g) || [];
const ids = idMatch.map(id => {
return id.replace('[', '').replace(']', '');
});
return {
...s,
id: ids,
time: 0,
color: '',
relative: 0,
};
data.map(s => {
const idMatch = s.id.match(/\[([^\]\[]*?)\]/g) || [];
const ids = idMatch.map(id => {
return id.replace('[', '').replace(']', '');
});
return {
...s,
id: ids,
time: 0,
color: '',
relative: 0,
};
});
export const calculateShardValues = (target: Targets) => (data: Shard[]) =>
produce<Shard[]>(data, draft => {
for (const shard of draft) {
if (target === 'searches') {
mutateSearchTimesTree(shard);
} else if (target === 'aggregations') {
mutateAggsTimesTree(shard);
}
export const calculateShardValues = (target: Targets) => (data: Shard[]) => {
const mutateTimesTree =
target === 'searches'
? mutateSearchTimesTree
: target === 'aggregations'
? mutateAggsTimesTree
: null;
if (mutateTimesTree) {
for (const shard of data) {
mutateTimesTree(shard);
}
});
}
export const initIndices = (data: Shard[]) =>
produce<Shard[], IndexMap>(data, doNotChange => {
const indices: IndexMap = {};
return data;
};
for (const shard of doNotChange) {
if (!indices[shard.id[1]]) {
indices[shard.id[1]] = {
shards: [],
time: 0,
name: shard.id[1],
visible: false,
};
}
indices[shard.id[1]].shards.push(shard);
indices[shard.id[1]].time += shard.time;
export const initIndices = (data: Shard[]) => {
const indices: IndexMap = {};
for (const shard of data) {
if (!indices[shard.id[1]]) {
indices[shard.id[1]] = {
shards: [],
time: 0,
name: shard.id[1],
visible: false,
};
}
indices[shard.id[1]].shards.push(shard);
indices[shard.id[1]].time += shard.time;
}
return indices;
});
return indices;
};
export const normalize = (target: Targets) => (data: IndexMap) =>
produce<IndexMap>(data, draft => {
normalizeIndices(draft, target);
});
export const normalize = (target: Targets) => (data: IndexMap) => {
normalizeIndices(data, target);
return data;
};
export const initDataFor = (target: Targets) =>
flow(initShards, calculateShardValues(target), initIndices, normalize(target), sortIndices);
flow(
cloneDeep,
initShards,
calculateShardValues(target),
initIndices,
normalize(target),
sortIndices
);

2
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/shard_details/shard_details.tsx
View file

@ -38,7 +38,7 @@ export const ShardDetails = ({ index, shard, operations }: Props) => {
<EuiFlexItem grow={false} className="prfDevTool__profileTree__shard__header-flex-item">
<EuiText className="prfDevTool__shardDetails--dim">
<PercentageBadge
timePercentage={relative as number}
timePercentage={String(relative)}
label={msToPretty(time as number, 3)}
valueType={'time'}
/>

5
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/shard_details/shard_details_tree_node.tsx
View file

@ -28,8 +28,7 @@ const limitString = (string: string, limit: number) =>
`${string.slice(0, limit)}${string.length > limit ? '...' : ''}`;
/**
* This is a component that recursively iterates over data to render out a tree like
* structure in a flatly.
* This component recursively renders a tree
*/
export const ShardDetailsTreeNode = ({ operation, index, shard }: Props) => {
const [childrenVisible, setChildrenVisible] = useState(hasVisibleChild(operation));
@ -106,7 +105,7 @@ export const ShardDetailsTreeNode = ({ operation, index, shard }: Props) => {
</div>
{childrenVisible &&
operation.hasChildren &&
operation.children.flatMap((childOp, idx) => (
operation.children.map((childOp, idx) => (
<ShardDetailsTreeNode key={idx} operation={childOp} index={index} shard={shard} />
))}
</>

88
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/components/profile_tree/unsafe_utils.ts
View file

@ -4,13 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { produce } from 'immer';
import { i18n } from '@kbn/i18n';
import tinycolor from 'tinycolor2';
import _ from 'lodash';
import { BreakdownItem, Index, Operation, Shard, Targets } from '../../types';
import { IndexMap } from './types';
import { MAX_TREE_DEPTH } from './constants';
export const comparator = (v1: number, v2: number) => {
if (v1 < v2) {
@ -53,16 +53,16 @@ function getToolTip(key: string) {
}
}
export function timeInMilliseconds(data: any) {
export function timeInMilliseconds(data: any): number {
if (data.time_in_nanos) {
return data.time_in_nanos / 1000000;
}
if (typeof data.time === 'string') {
return data.time.replace('ms', '');
return Number(data.time.replace('ms', ''));
}
return data.time;
return Number(data.time);
}
export function calcTimes(data: any[], parentId?: string) {
@ -117,21 +117,6 @@ export function normalizeBreakdown(breakdown: Record<string, number>) {
});
}
export function normalizeTimes(data: any[], totalTime: number, depth: number) {
// Second pass to normalize
for (const child of data) {
child.timePercentage = ((timeInMilliseconds(child) / totalTime) * 100).toFixed(2);
child.absoluteColor = tinycolor.mix('#F5F5F5', '#FFAFAF', child.timePercentage).toHexString();
child.depth = depth;
if (child.children != null && child.children.length !== 0) {
normalizeTimes(child.children, totalTime, depth + 1);
}
}
data.sort((a, b) => comparator(timeInMilliseconds(a), timeInMilliseconds(b)));
}
export function normalizeIndices(indices: IndexMap, target: Targets) {
// Sort the shards per-index
let sortQueryComponents;
@ -167,7 +152,26 @@ export function normalizeIndices(indices: IndexMap, target: Targets) {
}
}
export function initTree<T>(data: Operation[], depth = 0, parent: Operation | null = null) {
export function normalizeTime(operation: Operation, totalTime: number) {
operation.timePercentage = ((timeInMilliseconds(operation) / totalTime) * 100).toFixed(2);
operation.absoluteColor = tinycolor
.mix('#F5F5F5', '#FFAFAF', +operation.timePercentage)
.toHexString();
}
export function initTree<T>(
data: Operation[],
totalTime: number,
depth = 0,
parent: Operation | null = null
) {
if (MAX_TREE_DEPTH + 1 === depth) {
if (parent) {
parent!.hasChildren = false;
parent!.children = [];
}
return;
}
for (const child of data) {
// For bwc of older profile responses
if (!child.description) {
@ -178,44 +182,28 @@ export function initTree<T>(data: Operation[], depth = 0, parent: Operation | nu
child.query_type = null;
}
// Use named function for tests.
normalizeTime(child, totalTime);
child.parent = parent;
child.time = timeInMilliseconds(child);
child.lucene = child.description;
child.query_type = child.type!.split('.').pop()!;
child.visible = child.timePercentage > 20;
child.visible = +child.timePercentage > 20;
child.depth = depth;
if (child.children != null && child.children.length !== 0) {
initTree(child.children, depth + 1, child);
initTree(child.children, totalTime, depth + 1, child);
}
}
data.sort((a, b) => comparator(timeInMilliseconds(a), timeInMilliseconds(b)));
}
export function closeNode<T = any>(node: Operation) {
const closeDraft = (draft: Operation) => {
draft.visible = false;
if (draft.children == null || draft.children.length === 0) {
return;
}
for (const child of draft.children) {
closeDraft(child);
}
};
return produce<Operation>(node, draft => {
closeDraft(draft);
});
}
export const sortIndices = (data: IndexMap) =>
produce<IndexMap, Index[]>(data, doNotChange => {
const sortedIndices: Index[] = [];
for (const index of Object.values(doNotChange)) {
sortedIndices.push(index);
}
// And now sort the indices themselves
sortedIndices.sort((a, b) => comparator(a.time, b.time));
return sortedIndices;
});
export const sortIndices = (data: IndexMap) => {
const sortedIndices: Index[] = [];
for (const index of Object.values(data)) {
sortedIndices.push(index);
}
// And now sort the indices themselves
sortedIndices.sort((a, b) => comparator(a.time, b.time));
return sortedIndices;
};

60
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/store/reducer.test.ts Normal file
View file

@ -0,0 +1,60 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { reducer } from './reducer';
import { initialState, State } from './store';
describe('Searchprofiler store reducer', () => {
let state: State;
beforeEach(() => {
state = initialState;
});
it('profiles as expected', () => {
const nextState = reducer(state, { type: 'setProfiling', value: true });
expect(nextState).toEqual({
...state,
pristine: false,
profiling: true,
} as State);
const finalState = reducer(nextState, { type: 'setProfiling', value: false });
expect(finalState).toEqual({
...nextState,
pristine: false,
profiling: false,
} as State);
});
it('highlights as expected', () => {
const op = { children: null } as any;
const shard = { id: ['a', 'b', 'c'] } as any;
const nextState = reducer(state, {
type: 'setHighlightDetails',
value: { operation: op, indexName: 'test', shard },
});
expect(nextState).toEqual({
...state,
highlightDetails: {
operation: {
/* .children no longer defined */
},
shardName: '[a][c]',
indexName: 'test',
},
});
const finalState = reducer(state, {
type: 'setHighlightDetails',
value: null,
});
expect(finalState).toEqual({ ...state, highlightDetails: null });
});
});

66
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/store/reducer.ts
View file

@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { produce } from 'immer';
import { Reducer } from 'react';
import { State } from './store';
@ -17,36 +16,49 @@ export type Action =
| { type: 'setActiveTab'; value: Targets | null }
| { type: 'setCurrentResponse'; value: ShardSerialized[] | null };
export const reducer: Reducer<State, Action> = (state, action) =>
produce<State>(state, draft => {
if (action.type === 'setProfiling') {
draft.pristine = false;
draft.profiling = action.value;
if (draft.profiling) {
draft.currentResponse = null;
draft.highlightDetails = null;
}
return;
}
export const reducer: Reducer<State, Action> = (state, action) => {
const nextState = { ...state };
if (action.type === 'setHighlightDetails') {
draft.highlightDetails = action.value;
return;
if (action.type === 'setProfiling') {
nextState.pristine = false;
nextState.profiling = action.value;
if (nextState.profiling) {
nextState.currentResponse = null;
nextState.highlightDetails = null;
}
return nextState;
}
if (action.type === 'setActiveTab') {
draft.activeTab = action.value;
return;
if (action.type === 'setHighlightDetails') {
if (action.value) {
const value = action.value;
// Exclude children to avoid unnecessary work copying a recursive structure.
const { children, parent, ...restOfOperation } = value.operation;
nextState.highlightDetails = {
indexName: value.indexName,
operation: Object.freeze(restOfOperation),
// prettier-ignore
shardName: `[${/* shard id */value.shard.id[0]}][${/* shard number */value.shard.id[2] }]`
};
} else {
nextState.highlightDetails = null;
}
return nextState;
}
if (action.type === 'setCurrentResponse') {
draft.currentResponse = action.value;
if (draft.currentResponse) {
// Default to the searches tab
draft.activeTab = 'searches';
}
return;
if (action.type === 'setActiveTab') {
nextState.activeTab = action.value;
return nextState;
}
if (action.type === 'setCurrentResponse') {
nextState.currentResponse = action.value;
if (nextState.currentResponse) {
// Default to the searches tab
nextState.activeTab = 'searches';
}
return nextState;
}
throw new Error(`Unknown action: ${action}`);
});
throw new Error(`Unknown action: ${action}`);
};

13
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/store/store.ts
View file

@ -5,13 +5,20 @@
*/
import { useReducer } from 'react';
import { reducer } from './reducer';
import { ShardSerialized, Targets } from '../types';
import { OnHighlightChangeArgs } from '../components/profile_tree';
import { Operation, ShardSerialized, Targets } from '../types';
export type OperationNoChildParent = Omit<Operation, 'children' | 'parent'>;
interface HighlightDetails {
indexName: string;
operation: OperationNoChildParent;
shardName: string;
}
export interface State {
profiling: boolean;
pristine: boolean;
highlightDetails: OnHighlightChangeArgs | null;
highlightDetails: HighlightDetails | null;
activeTab: Targets | null;
currentResponse: ShardSerialized[] | null;
}

2
x-pack/legacy/plugins/searchprofiler/public/np_ready/application/types.ts
View file

@ -42,7 +42,7 @@ export interface Operation {
hasChildren: boolean;
visible: boolean;
selfTime: number;
timePercentage: number;
timePercentage: string;
absoluteColor: string;
time: number;