Skip to content

Commit

Permalink
[Migrations] Only reindex indices if needed (elastic#147371)
Browse files Browse the repository at this point in the history
Fixes elastic#124946

## Summary

Takes a step toward optimising our migration paths by only reindexing
(an expensive operation) when needed by checking whether the current SO
mappings have "changed".

By "changed" we mean that we have detected a new md5 checksum of any
registered saved object type relative to the hashes we have stored.

## How to test

These changes are constrained to the `model.ts`, a test was added for
correctly detecting that mappings are the same during the `INIT` phase
to send us down the correct migration path.

Additionally, we have a new Jest integration test `skip_reindex.test.ts`
that runs Kibana and inspects the logs for the expected model
transitions.

Everything else should remain the same.

## Happy path for skipping reindex

```
RUN INIT

IF !versionMigrationIsComplete AND
   !kibana indexBelongsToLaterVersion AND
   !waitForMigrationCompletion AND
   mappingsAreUnchanged
THEN
  the migration operations must target the existing .kibana_x.y.z_001 index

RUN PREPARE_COMPATIBLE_MIGRATION (new state)
    we remove old version aliases (prevent old reads/writes), and set the current version alias (prevent old migrations)

SKIP LEGACY_SET_WRITE_BLOCK
SKIP ...
SKIP SET_SOURCE_WRITE_BLOCK 
SKIP ...
SKIP REFRESH_TARGET

RUN OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT
...
RUN MARK_VERSION_INDEX_READY
DONE
```

## Notes

* This optimisation will only be triggered when there are no mappings
changes AND we are upgrading to a new version. This does not cover all
cases. In future we will make this more sophisticated by checking for
incompatible changes to mappings and only reindexing when those occur.

## Related

* elastic#147503

### Checklist

- [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

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Gerard Soldevila <[email protected]>
  • Loading branch information
3 people authored Dec 23, 2022
1 parent eb2c9ce commit bcb8a13
Show file tree
Hide file tree
Showing 9 changed files with 394 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
addMustClausesToBoolQuery,
addMustNotClausesToBoolQuery,
getAliases,
buildRemoveAliasActions,
versionMigrationCompleted,
} from './helpers';

Expand Down Expand Up @@ -267,3 +268,22 @@ describe('versionMigrationCompleted', () => {
expect(versionMigrationCompleted('.current-alias', '.version-alias', {})).toBe(false);
});
});

describe('buildRemoveAliasActions', () => {
test('empty', () => {
expect(buildRemoveAliasActions('.kibana_test_123', [], [])).toEqual([]);
});
test('no exclusions', () => {
expect(buildRemoveAliasActions('.kibana_test_123', ['a', 'b', 'c'], [])).toEqual([
{ remove: { index: '.kibana_test_123', alias: 'a', must_exist: true } },
{ remove: { index: '.kibana_test_123', alias: 'b', must_exist: true } },
{ remove: { index: '.kibana_test_123', alias: 'c', must_exist: true } },
]);
});
test('with exclusions', () => {
expect(buildRemoveAliasActions('.kibana_test_123', ['a', 'b', 'c'], ['b'])).toEqual([
{ remove: { index: '.kibana_test_123', alias: 'a', must_exist: true } },
{ remove: { index: '.kibana_test_123', alias: 'c', must_exist: true } },
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
import * as Either from 'fp-ts/lib/Either';
import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal';
import type { State } from '../state';
import type { FetchIndexResponse } from '../actions';
import type { AliasAction, FetchIndexResponse } from '../actions';

/**
* A helper function/type for ensuring that all control state's are handled.
Expand Down Expand Up @@ -189,3 +189,19 @@ export function getAliases(

return Either.right(aliases);
}

/**
* Build a list of alias actions to remove the provided aliases from the given index.
*/
export function buildRemoveAliasActions(
index: string,
aliases: string[],
exclude: string[]
): AliasAction[] {
return aliases.flatMap((alias) => {
if (exclude.includes(alias)) {
return [];
}
return [{ remove: { index, alias, must_exist: true } }];
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import * as Either from 'fp-ts/lib/Either';
import * as Option from 'fp-ts/lib/Option';
import type { SavedObjectsRawDoc } from '@kbn/core-saved-objects-server';
import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal';
import type {
FatalState,
State,
Expand Down Expand Up @@ -45,6 +46,7 @@ import type {
CheckVersionIndexReadyActions,
UpdateTargetMappingsMeta,
CheckTargetMappingsState,
PrepareCompatibleMigration,
} from '../state';
import { type TransformErrorObjects, TransformSavedObjectDocumentError } from '../core';
import type { AliasAction, RetryableEsClientError } from '../actions';
Expand All @@ -53,6 +55,20 @@ import { createInitialProgress } from './progress';
import { model } from './model';

describe('migrations v2 model', () => {
const indexMapping: IndexMapping = {
properties: {
new_saved_object_type: {
properties: {
value: { type: 'text' },
},
},
},
_meta: {
migrationMappingPropertyHashes: {
new_saved_object_type: '4a11183eee21e6fbad864f7a30b39ad0',
},
},
};
const baseState: BaseState = {
controlState: '',
legacyIndex: '.kibana',
Expand All @@ -67,20 +83,7 @@ describe('migrations v2 model', () => {
discardCorruptObjects: false,
indexPrefix: '.kibana',
outdatedDocumentsQuery: {},
targetIndexMappings: {
properties: {
new_saved_object_type: {
properties: {
value: { type: 'text' },
},
},
},
_meta: {
migrationMappingPropertyHashes: {
new_saved_object_type: '4a11183eee21e6fbad864f7a30b39ad0',
},
},
},
targetIndexMappings: indexMapping,
tempIndexMappings: { properties: {} },
preMigrationScript: Option.none,
currentAlias: '.kibana',
Expand Down Expand Up @@ -265,7 +268,7 @@ describe('migrations v2 model', () => {
settings: {},
},
});
const newState = model(initState, res);
const newState = model(initState, res) as OutdatedDocumentsSearchOpenPit;

expect(newState.controlState).toEqual('OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT');
// This snapshot asserts that we merge the
Expand All @@ -292,6 +295,22 @@ describe('migrations v2 model', () => {
},
}
`);
expect(newState.targetIndexRawMappings).toEqual({
_meta: {
migrationMappingPropertyHashes: {
disabled_saved_object_type: '7997cf5a56cc02bdc9c93361bde732b0',
},
},
properties: {
disabled_saved_object_type: {
properties: {
value: {
type: 'keyword',
},
},
},
},
});
expect(newState.retryCount).toEqual(0);
expect(newState.retryDelay).toEqual(0);
});
Expand Down Expand Up @@ -498,6 +517,7 @@ describe('migrations v2 model', () => {
expect(newState.retryDelay).toEqual(2000);
});
});

describe('if waitForMigrationCompletion=false', () => {
const initState = Object.assign({}, initBaseState, {
waitForMigrationCompletion: false,
Expand Down Expand Up @@ -787,6 +807,67 @@ describe('migrations v2 model', () => {
expect(newState.retryCount).toEqual(0);
expect(newState.retryDelay).toEqual(0);
});

describe('when upgrading to a new stack version', () => {
const unchangedMappingsState: State = {
...baseState,
controlState: 'INIT',
kibanaVersion: '7.12.0', // new version!
currentAlias: '.kibana',
versionAlias: '.kibana_7.12.0',
versionIndex: '.kibana_7.11.0_001',
};
it('INIT -> PREPARE_COMPATIBLE_MIGRATION when the mappings have not changed', () => {
const res: ResponseType<'INIT'> = Either.right({
'.kibana_7.11.0_001': {
aliases: {
'.kibana': {},
'.kibana_7.11.0': {},
},
mappings: indexMapping,
settings: {},
},
});
const newState = model(unchangedMappingsState, res) as PrepareCompatibleMigration;

expect(newState.controlState).toEqual('PREPARE_COMPATIBLE_MIGRATION');
expect(newState.targetIndexRawMappings).toEqual({
_meta: {
migrationMappingPropertyHashes: {
new_saved_object_type: '4a11183eee21e6fbad864f7a30b39ad0',
},
},
properties: {
new_saved_object_type: {
properties: {
value: {
type: 'text',
},
},
},
},
});
expect(newState.versionAlias).toEqual('.kibana_7.12.0');
expect(newState.currentAlias).toEqual('.kibana');
// will point to
expect(newState.targetIndex).toEqual('.kibana_7.11.0_001');
expect(newState.preTransformDocsActions).toEqual([
{
add: {
alias: '.kibana_7.12.0',
index: '.kibana_7.11.0_001',
},
},
{
remove: {
alias: '.kibana_7.11.0',
index: '.kibana_7.11.0_001',
must_exist: true,
},
},
]);
});
});
});
});

Expand Down Expand Up @@ -1893,6 +1974,47 @@ describe('migrations v2 model', () => {
});
});

describe('PREPARE_COMPATIBLE_MIGRATIONS', () => {
const someAliasAction: AliasAction = { add: { index: '.kibana', alias: '.kibana_8.7.0' } };
const state: PrepareCompatibleMigration = {
...baseState,
controlState: 'PREPARE_COMPATIBLE_MIGRATION',
versionIndexReadyActions: Option.none,
sourceIndex: Option.some('.kibana') as Option.Some<string>,
targetIndex: '.kibana_7.11.0_001',
preTransformDocsActions: [someAliasAction],
};

it('PREPARE_COMPATIBLE_MIGRATIONS -> OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT if action succeeds', () => {
const res: ResponseType<'PREPARE_COMPATIBLE_MIGRATION'> = Either.right(
'update_aliases_succeeded'
);
const newState = model(state, res) as OutdatedDocumentsSearchOpenPit;
expect(newState.controlState).toEqual('OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT');
expect(newState.versionIndexReadyActions).toEqual(Option.none);
});

it('PREPARE_COMPATIBLE_MIGRATIONS -> OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT if action fails because the alias is not found', () => {
const res: ResponseType<'PREPARE_COMPATIBLE_MIGRATION'> = Either.left({
type: 'alias_not_found_exception',
});

const newState = model(state, res) as OutdatedDocumentsSearchOpenPit;
expect(newState.controlState).toEqual('OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT');
expect(newState.versionIndexReadyActions).toEqual(Option.none);
});

it('throws an exception if action fails with an error other than a missing alias', () => {
const res: ResponseType<'PREPARE_COMPATIBLE_MIGRATION'> = Either.left({
type: 'remove_index_not_a_concrete_index',
});

expect(() => model(state, res)).toThrowErrorMatchingInlineSnapshot(
`"PREPARE_COMPATIBLE_MIGRATION received unexpected action response: {\\"type\\":\\"remove_index_not_a_concrete_index\\"}"`
);
});
});

describe('OUTDATED_DOCUMENTS_SEARCH_OPEN_PIT', () => {
const state: OutdatedDocumentsSearchOpenPit = {
...baseState,
Expand Down
Loading

0 comments on commit bcb8a13

Please sign in to comment.