From f92d335a0db50e85df2c62c1ba1d2b3c085c1cf2 Mon Sep 17 00:00:00 2001 From: Yulia Cech Date: Thu, 10 Sep 2020 17:11:25 +0200 Subject: [PATCH 1/2] [ILM] Add forcemerge action to hot phase with a rollover enabled --- .../__jest__/components/edit_policy.test.tsx | 46 +++++++-- .../common/types/policies.ts | 18 +++- .../public/application/constants/policy.ts | 2 + .../edit_policy/components/forcemerge.tsx | 99 +++++++++++++++++++ .../sections/edit_policy/components/index.ts | 1 + .../sections/edit_policy/phases/hot_phase.tsx | 10 ++ .../edit_policy/phases/warm_phase.tsx | 70 ++----------- .../services/policies/hot_phase.ts | 28 ++++++ .../api/policies/register_create_route.ts | 13 ++- 9 files changed, 210 insertions(+), 77 deletions(-) create mode 100644 x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx index 28b25c3eb4530..a9de6c6f536c2 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx +++ b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx @@ -275,6 +275,40 @@ describe('edit policy', () => { save(rendered); expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); }); + test('should show forcemerge input when rollover enabled', () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + expect(findTestSubject(rendered, 'hot-forceMergeSwitch').exists()).toBeTruthy(); + }); + test('should hide forcemerge input when rollover is disabled', () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + noRollover(rendered); + rendered.update(); + expect(findTestSubject(rendered, 'hot-forceMergeSwitch').exists()).toBeFalsy(); + }); + test('should show positive number required above zero error when trying to save hot phase with 0 for force merge', async () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + findTestSubject(rendered, 'hot-forceMergeSwitch').simulate('click'); + rendered.update(); + const forcemergeInput = rendered.find('input#hot-selectedForceMergeSegments'); + forcemergeInput.simulate('change', { target: { value: '0' } }); + rendered.update(); + save(rendered); + expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + }); + test('should show positive number above 0 required error when trying to save hot phase with -1 for force merge', async () => { + const rendered = mountWithIntl(component); + setPolicyName(rendered, 'mypolicy'); + findTestSubject(rendered, 'hot-forceMergeSwitch').simulate('click'); + rendered.update(); + const forcemergeInput = rendered.find('input#hot-selectedForceMergeSegments'); + forcemergeInput.simulate('change', { target: { value: '-1' } }); + rendered.update(); + save(rendered); + expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + }); test('should show positive number required error when trying to save with -1 for index priority', () => { const rendered = mountWithIntl(component); noRollover(rendered); @@ -364,10 +398,10 @@ describe('edit policy', () => { setPolicyName(rendered, 'mypolicy'); await activatePhase(rendered, 'warm'); setPhaseAfter(rendered, 'warm', '1'); - findTestSubject(rendered, 'forceMergeSwitch').simulate('click'); + findTestSubject(rendered, 'warm-forceMergeSwitch').simulate('click'); rendered.update(); - const shrinkInput = rendered.find('input#warm-selectedForceMergeSegments'); - shrinkInput.simulate('change', { target: { value: '0' } }); + const forcemergeInput = rendered.find('input#warm-selectedForceMergeSegments'); + forcemergeInput.simulate('change', { target: { value: '0' } }); rendered.update(); save(rendered); expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); @@ -378,10 +412,10 @@ describe('edit policy', () => { setPolicyName(rendered, 'mypolicy'); await activatePhase(rendered, 'warm'); setPhaseAfter(rendered, 'warm', '1'); - findTestSubject(rendered, 'forceMergeSwitch').simulate('click'); + findTestSubject(rendered, 'warm-forceMergeSwitch').simulate('click'); rendered.update(); - const shrinkInput = rendered.find('input#warm-selectedForceMergeSegments'); - shrinkInput.simulate('change', { target: { value: '-1' } }); + const forcemergeInput = rendered.find('input#warm-selectedForceMergeSegments'); + forcemergeInput.simulate('change', { target: { value: '-1' } }); rendered.update(); save(rendered); expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); diff --git a/x-pack/plugins/index_lifecycle_management/common/types/policies.ts b/x-pack/plugins/index_lifecycle_management/common/types/policies.ts index d88d5b5021a25..97effee44533a 100644 --- a/x-pack/plugins/index_lifecycle_management/common/types/policies.ts +++ b/x-pack/plugins/index_lifecycle_management/common/types/policies.ts @@ -41,6 +41,9 @@ export interface SerializedHotPhase extends SerializedPhase { max_age?: string; max_docs?: number; }; + forcemerge?: { + max_num_segments: number; + }; set_priority?: { priority: number | null; }; @@ -131,7 +134,15 @@ export interface PhaseWithIndexPriority { phaseIndexPriority: string; } -export interface HotPhase extends CommonPhaseSettings, PhaseWithIndexPriority { +export interface PhaseWithForcemergeAction { + forceMergeEnabled: boolean; + selectedForceMergeSegments: string; +} + +export interface HotPhase + extends CommonPhaseSettings, + PhaseWithIndexPriority, + PhaseWithForcemergeAction { rolloverEnabled: boolean; selectedMaxSizeStored: string; selectedMaxSizeStoredUnits: string; @@ -144,12 +155,11 @@ export interface WarmPhase extends CommonPhaseSettings, PhaseWithMinAge, PhaseWithAllocationAction, - PhaseWithIndexPriority { + PhaseWithIndexPriority, + PhaseWithForcemergeAction { warmPhaseOnRollover: boolean; shrinkEnabled: boolean; selectedPrimaryShardCount: string; - forceMergeEnabled: boolean; - selectedForceMergeSegments: string; } export interface ColdPhase diff --git a/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts b/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts index 4fd74da06f1b3..f11860d36faf8 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/constants/policy.ts @@ -20,6 +20,8 @@ export const defaultNewHotPhase: HotPhase = { selectedMaxAgeUnits: 'd', selectedMaxSizeStored: '50', selectedMaxSizeStoredUnits: 'gb', + forceMergeEnabled: false, + selectedForceMergeSegments: '', phaseIndexPriority: '100', selectedMaxDocuments: '', }; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx new file mode 100644 index 0000000000000..aa39b8e75e2de --- /dev/null +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx @@ -0,0 +1,99 @@ +/* + * 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 { FormattedMessage } from '@kbn/i18n/react'; +import { + EuiDescribedFormGroup, + EuiFieldNumber, + EuiSpacer, + EuiSwitch, + EuiTextColor, +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; +import { LearnMoreLink } from './learn_more_link'; +import { ErrableFormRow } from './form_errors'; +import { Phases, PhaseWithForcemergeAction } from '../../../../../common/types'; +import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; + +const forcemergeLabel = i18n.translate('xpack.indexLifecycleMgmt.warmPhase.forceMergeDataLabel', { + defaultMessage: 'Force merge data', +}); + +interface Props { + errors?: PhaseValidationErrors; + phase: keyof Phases & string; + phaseData: T; + setPhaseData: (dataKey: keyof T & string, value: boolean | string) => void; + isShowingErrors: boolean; +} +export const Forcemerge = ({ + errors, + phaseData, + phase, + setPhaseData, + isShowingErrors, +}: React.PropsWithChildren>) => { + const phaseForcemergeEnabledProperty = propertyof('forceMergeEnabled'); + const phaseForcemergeSegmentsProperty = propertyof('selectedForceMergeSegments'); + return ( + + + + } + description={ + + {' '} + + + } + titleSize="xs" + fullWidth + > + { + setPhaseData(phaseForcemergeEnabledProperty, e.target.checked); + }} + aria-controls="forcemergeContent" + /> + + +
+ {phaseData.forceMergeEnabled ? ( + + { + setPhaseData(phaseForcemergeSegmentsProperty, e.target.value); + }} + min={1} + /> + + ) : null} +
+ + ); +}; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts index e933c46e98491..4410c4bb38397 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts @@ -15,3 +15,4 @@ export { PhaseErrorMessage } from './phase_error_message'; export { PolicyJsonFlyout } from './policy_json_flyout'; export { SetPriorityInput } from './set_priority_input'; export { SnapshotPolicies } from './snapshot_policies'; +export { Forcemerge } from './forcemerge'; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx index 59949ad93fa5d..893d7500f28e2 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx @@ -27,6 +27,7 @@ import { PhaseErrorMessage, ErrableFormRow, SetPriorityInput, + Forcemerge, } from '../components'; const maxSizeStoredUnits = [ @@ -313,6 +314,15 @@ export class HotPhase extends PureComponent { ) : null} + {phaseData.rolloverEnabled ? ( + + phase={'hot'} + phaseData={phaseData} + setPhaseData={setPhaseData} + isShowingErrors={isShowingErrors} + errors={errors} + /> + ) : null} errors={errors} phaseData={phaseData} diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx index 71286475bcfe9..903cdc7edba96 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx @@ -29,6 +29,7 @@ import { SetPriorityInput, NodeAllocation, MinAgeInput, + Forcemerge, } from '../components'; const shrinkLabel = i18n.translate('xpack.indexLifecycleMgmt.warmPhase.shrinkIndexLabel', { @@ -42,10 +43,6 @@ const moveToWarmPhaseOnRolloverLabel = i18n.translate( } ); -const forcemergeLabel = i18n.translate('xpack.indexLifecycleMgmt.warmPhase.forceMergeDataLabel', { - defaultMessage: 'Force merge data', -}); - const warmProperty: keyof Phases = 'warm'; const phaseProperty = (propertyName: keyof WarmPhaseInterface) => propertyName; @@ -262,64 +259,13 @@ export class WarmPhase extends PureComponent { - - - - } - description={ - - {' '} - - - } - titleSize="xs" - fullWidth - > - { - setPhaseData(phaseProperty('forceMergeEnabled'), e.target.checked); - }} - aria-controls="forcemergeContent" - /> - - -
- {phaseData.forceMergeEnabled ? ( - - { - setPhaseData(phaseProperty('selectedForceMergeSegments'), e.target.value); - }} - min={1} - /> - - ) : null} -
- + + phase={'warm'} + phaseData={phaseData} + setPhaseData={setPhaseData} + isShowingErrors={isShowingErrors} + errors={errors} + /> errors={errors} phaseData={phaseData} diff --git a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/hot_phase.ts b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/hot_phase.ts index fb7f74efeb66e..4ef4890b5ffe3 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/services/policies/hot_phase.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/services/policies/hot_phase.ts @@ -24,6 +24,8 @@ const hotPhaseInitialization: HotPhase = { selectedMaxAgeUnits: 'd', selectedMaxSizeStored: '', selectedMaxSizeStoredUnits: 'gb', + forceMergeEnabled: false, + selectedForceMergeSegments: '', phaseIndexPriority: '', selectedMaxDocuments: '', }; @@ -58,6 +60,12 @@ export const hotPhaseFromES = (phaseSerialized?: SerializedHotPhase): HotPhase = } } + if (actions.forcemerge) { + const forcemerge = actions.forcemerge; + phase.forceMergeEnabled = true; + phase.selectedForceMergeSegments = forcemerge.max_num_segments.toString(); + } + if (actions.set_priority) { phase.phaseIndexPriority = actions.set_priority.priority ? actions.set_priority.priority.toString() @@ -93,8 +101,19 @@ export const hotPhaseToES = ( if (isNumber(phase.selectedMaxDocuments)) { esPhase.actions.rollover.max_docs = parseInt(phase.selectedMaxDocuments, 10); } + if (phase.forceMergeEnabled && isNumber(phase.selectedForceMergeSegments)) { + esPhase.actions.forcemerge = { + max_num_segments: parseInt(phase.selectedForceMergeSegments, 10), + }; + } else { + delete esPhase.actions.forcemerge; + } } else { delete esPhase.actions.rollover; + // forcemerge is only allowed if rollover is enabled + if (esPhase.actions.forcemerge) { + delete esPhase.actions.forcemerge; + } } if (isNumber(phase.phaseIndexPriority)) { @@ -147,6 +166,15 @@ export const validateHotPhase = (phase: HotPhase): PhaseValidationErrors Date: Tue, 15 Sep 2020 15:26:45 +0200 Subject: [PATCH 2/2] [ILM] Add PR suggestions --- .../__jest__/components/edit_policy.test.tsx | 8 +++---- .../edit_policy/components/forcemerge.tsx | 24 +++++++++---------- .../sections/edit_policy/phases/hot_phase.tsx | 2 +- .../edit_policy/phases/warm_phase.tsx | 2 +- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx index a9de6c6f536c2..3867c30655379 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx +++ b/x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx @@ -292,7 +292,7 @@ describe('edit policy', () => { setPolicyName(rendered, 'mypolicy'); findTestSubject(rendered, 'hot-forceMergeSwitch').simulate('click'); rendered.update(); - const forcemergeInput = rendered.find('input#hot-selectedForceMergeSegments'); + const forcemergeInput = findTestSubject(rendered, 'hot-selectedForceMergeSegments'); forcemergeInput.simulate('change', { target: { value: '0' } }); rendered.update(); save(rendered); @@ -303,7 +303,7 @@ describe('edit policy', () => { setPolicyName(rendered, 'mypolicy'); findTestSubject(rendered, 'hot-forceMergeSwitch').simulate('click'); rendered.update(); - const forcemergeInput = rendered.find('input#hot-selectedForceMergeSegments'); + const forcemergeInput = findTestSubject(rendered, 'hot-selectedForceMergeSegments'); forcemergeInput.simulate('change', { target: { value: '-1' } }); rendered.update(); save(rendered); @@ -400,7 +400,7 @@ describe('edit policy', () => { setPhaseAfter(rendered, 'warm', '1'); findTestSubject(rendered, 'warm-forceMergeSwitch').simulate('click'); rendered.update(); - const forcemergeInput = rendered.find('input#warm-selectedForceMergeSegments'); + const forcemergeInput = findTestSubject(rendered, 'warm-selectedForceMergeSegments'); forcemergeInput.simulate('change', { target: { value: '0' } }); rendered.update(); save(rendered); @@ -414,7 +414,7 @@ describe('edit policy', () => { setPhaseAfter(rendered, 'warm', '1'); findTestSubject(rendered, 'warm-forceMergeSwitch').simulate('click'); rendered.update(); - const forcemergeInput = rendered.find('input#warm-selectedForceMergeSegments'); + const forcemergeInput = findTestSubject(rendered, 'warm-selectedForceMergeSegments'); forcemergeInput.simulate('change', { target: { value: '-1' } }); rendered.update(); save(rendered); diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx index aa39b8e75e2de..50cc7fd8b3274 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/forcemerge.tsx @@ -17,28 +17,26 @@ import React from 'react'; import { LearnMoreLink } from './learn_more_link'; import { ErrableFormRow } from './form_errors'; import { Phases, PhaseWithForcemergeAction } from '../../../../../common/types'; -import { PhaseValidationErrors, propertyof } from '../../../services/policies/policy_validation'; +import { PhaseValidationErrors } from '../../../services/policies/policy_validation'; const forcemergeLabel = i18n.translate('xpack.indexLifecycleMgmt.warmPhase.forceMergeDataLabel', { defaultMessage: 'Force merge data', }); -interface Props { - errors?: PhaseValidationErrors; +interface Props { + errors?: PhaseValidationErrors; phase: keyof Phases & string; - phaseData: T; - setPhaseData: (dataKey: keyof T & string, value: boolean | string) => void; + phaseData: PhaseWithForcemergeAction; + setPhaseData: (dataKey: keyof PhaseWithForcemergeAction, value: boolean | string) => void; isShowingErrors: boolean; } -export const Forcemerge = ({ +export const Forcemerge: React.FunctionComponent = ({ errors, phaseData, phase, setPhaseData, isShowingErrors, -}: React.PropsWithChildren>) => { - const phaseForcemergeEnabledProperty = propertyof('forceMergeEnabled'); - const phaseForcemergeSegmentsProperty = propertyof('selectedForceMergeSegments'); +}) => { return ( ({ aria-label={forcemergeLabel} checked={phaseData.forceMergeEnabled} onChange={(e) => { - setPhaseData(phaseForcemergeEnabledProperty, e.target.checked); + setPhaseData('forceMergeEnabled', e.target.checked); }} aria-controls="forcemergeContent" /> @@ -76,7 +74,7 @@ export const Forcemerge = ({
{phaseData.forceMergeEnabled ? ( ({ errors={errors?.selectedForceMergeSegments} > { - setPhaseData(phaseForcemergeSegmentsProperty, e.target.value); + setPhaseData('selectedForceMergeSegments', e.target.value); }} min={1} /> diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx index 893d7500f28e2..7682b92488086 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/hot_phase.tsx @@ -315,7 +315,7 @@ export class HotPhase extends PureComponent { ) : null} {phaseData.rolloverEnabled ? ( - + {
- +