Do not update+pickup docs if SO types mappings have not changed (#179937)

## Summary

Align V2 behavior with ZDT after
https://github.com/elastic/kibana/pull/179595

Under the assumption that whenever we want to add new root fields (e.g.
[created_by](https://github.com/elastic/kibana/pull/179344/)), we will
systematically add mappings for them, we can skip the
`updateAndPickupMappings` operation iif ONLY root fields' mappings have
changed during an upgrade (aka if none of the SO types have updated
their mappings).

Up until now, the logic was updating ALL SO types whenever a `root`
field was added / updated.
This is expensive and unnecessary, and can cause a noticeable impact to
large customers during migrations.
This commit is contained in:
Gerard Soldevila 2024-04-09 11:29:31 +02:00 committed by GitHub
parent d91e410a0e
commit 9046abbe9a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 100 additions and 118 deletions

View file

@ -456,7 +456,7 @@ Once transformed we use an index operation to overwrite the outdated document wi
### Next action
`checkTargetMappings`
`checkTargetTypesMappings`
Compare the calculated mappings' hashes against those stored in the `<index>.mappings._meta`.

View file

@ -9,7 +9,7 @@
import * as Either from 'fp-ts/lib/Either';
import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal';
import type { SavedObjectsMappingProperties } from '@kbn/core-saved-objects-server';
import { checkTargetMappings } from './check_target_mappings';
import { checkTargetTypesMappings } from './check_target_mappings';
import { getBaseMappings } from '../core';
const indexTypes = ['type1', 'type2', 'type3'];
@ -60,14 +60,14 @@ const appMappings: IndexMapping = {
},
};
describe('checkTargetMappings', () => {
describe('checkTargetTypesMappings', () => {
beforeEach(() => {
jest.clearAllMocks();
});
describe('when index mappings are missing required properties', () => {
it("returns 'index_mappings_incomplete' if index mappings are not defined", async () => {
const task = checkTargetMappings({
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
latestMappingsVersions,
@ -78,7 +78,7 @@ describe('checkTargetMappings', () => {
});
it("returns 'index_mappings_incomplete' if index mappings do not define _meta", async () => {
const task = checkTargetMappings({
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: {
@ -93,7 +93,7 @@ describe('checkTargetMappings', () => {
});
it("returns 'index_mappings_incomplete' if index mappings do not define migrationMappingPropertyHashes nor mappingVersions", async () => {
const task = checkTargetMappings({
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: {
@ -109,7 +109,7 @@ describe('checkTargetMappings', () => {
});
it("returns 'index_mappings_incomplete' if index mappings define a different value for 'dynamic' property", async () => {
const task = checkTargetMappings({
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: {
@ -128,7 +128,7 @@ describe('checkTargetMappings', () => {
describe('when index mappings have all required properties', () => {
describe('when some core properties (aka root fields) have changed', () => {
it('returns the list of fields that have changed', async () => {
const task = checkTargetMappings({
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: {
@ -149,8 +149,9 @@ describe('checkTargetMappings', () => {
const result = await task();
expect(result).toEqual(
Either.left({
type: 'root_fields_changed' as const,
updatedFields: ['references'],
type: 'types_changed' as const,
// types are flagged as changed cause we have not provided a hashToVersionMap
updatedTypes: ['type1', 'type2'],
})
);
});
@ -159,8 +160,8 @@ describe('checkTargetMappings', () => {
describe('when core properties have NOT changed', () => {
describe('when index mappings ONLY contain the legacy hashes', () => {
describe('and legacy hashes match the current model versions', () => {
it('returns a compared_mappings_match response', async () => {
const task = checkTargetMappings({
it('returns a types_match response', async () => {
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: legacyMappings,
@ -175,7 +176,7 @@ describe('checkTargetMappings', () => {
const result = await task();
expect(result).toEqual(
Either.right({
type: 'compared_mappings_match' as const,
type: 'types_match' as const,
})
);
});
@ -183,7 +184,7 @@ describe('checkTargetMappings', () => {
describe('and legacy hashes do NOT match the current model versions', () => {
it('returns the list of updated SO types', async () => {
const task = checkTargetMappings({
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: legacyMappings,
@ -207,8 +208,8 @@ describe('checkTargetMappings', () => {
describe('when index mappings contain the mappingVersions', () => {
describe('and mappingVersions match', () => {
it('returns a compared_mappings_match response', async () => {
const task = checkTargetMappings({
it('returns a types_match response', async () => {
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: {
@ -228,7 +229,7 @@ describe('checkTargetMappings', () => {
const result = await task();
expect(result).toEqual(
Either.right({
type: 'compared_mappings_match' as const,
type: 'types_match' as const,
})
);
});
@ -236,7 +237,7 @@ describe('checkTargetMappings', () => {
describe('and mappingVersions do NOT match', () => {
it('returns the list of updated SO types', async () => {
const task = checkTargetMappings({
const task = checkTargetTypesMappings({
indexTypes,
appMappings,
indexMappings: {

View file

@ -9,10 +9,10 @@ import * as Either from 'fp-ts/lib/Either';
import * as TaskEither from 'fp-ts/lib/TaskEither';
import type { IndexMapping, VirtualVersionMap } from '@kbn/core-saved-objects-base-server-internal';
import { getUpdatedRootFields, getUpdatedTypes } from '../core/compare_mappings';
import { getUpdatedTypes } from '../core/compare_mappings';
/** @internal */
export interface CheckTargetMappingsParams {
export interface CheckTargetTypesMappingsParams {
indexTypes: string[];
indexMappings?: IndexMapping;
appMappings: IndexMapping;
@ -21,34 +21,31 @@ export interface CheckTargetMappingsParams {
}
/** @internal */
export interface ComparedMappingsMatch {
type: 'compared_mappings_match';
}
export interface IndexMappingsIncomplete {
type: 'index_mappings_incomplete';
}
export interface RootFieldsChanged {
type: 'root_fields_changed';
updatedFields: string[];
/** @internal */
export interface TypesMatch {
type: 'types_match';
}
/** @internal */
export interface TypesChanged {
type: 'types_changed';
updatedTypes: string[];
}
export const checkTargetMappings =
export const checkTargetTypesMappings =
({
indexTypes,
indexMappings,
appMappings,
latestMappingsVersions,
hashToVersionMap = {},
}: CheckTargetMappingsParams): TaskEither.TaskEither<
IndexMappingsIncomplete | RootFieldsChanged | TypesChanged,
ComparedMappingsMatch
}: CheckTargetTypesMappingsParams): TaskEither.TaskEither<
IndexMappingsIncomplete | TypesChanged,
TypesMatch
> =>
async () => {
if (
@ -59,15 +56,6 @@ export const checkTargetMappings =
return Either.left({ type: 'index_mappings_incomplete' as const });
}
const updatedFields = getUpdatedRootFields(indexMappings);
if (updatedFields.length) {
return Either.left({
type: 'root_fields_changed',
updatedFields,
});
}
const updatedTypes = getUpdatedTypes({
indexTypes,
indexMeta: indexMappings?._meta,
@ -81,6 +69,6 @@ export const checkTargetMappings =
updatedTypes,
});
} else {
return Either.right({ type: 'compared_mappings_match' as const });
return Either.right({ type: 'types_match' as const });
}
};

View file

@ -87,7 +87,7 @@ export { synchronizeMigrators } from './synchronize_migrators';
export { createIndex } from './create_index';
export { checkTargetMappings } from './check_target_mappings';
export { checkTargetTypesMappings } from './check_target_mappings';
export const noop = async (): Promise<Either<never, 'noop'>> => right('noop' as const);
@ -108,11 +108,7 @@ import type { UnknownDocsFound } from './check_for_unknown_docs';
import type { IncompatibleClusterRoutingAllocation } from './initialize_action';
import type { ClusterShardLimitExceeded } from './create_index';
import type { SynchronizationFailed } from './synchronize_migrators';
import type {
IndexMappingsIncomplete,
RootFieldsChanged,
TypesChanged,
} from './check_target_mappings';
import type { IndexMappingsIncomplete, TypesChanged } from './check_target_mappings';
export type {
CheckForUnknownDocsParams,
@ -187,7 +183,6 @@ export interface ActionErrorTypeMap {
es_response_too_large: EsResponseTooLargeError;
synchronization_failed: SynchronizationFailed;
index_mappings_incomplete: IndexMappingsIncomplete;
root_fields_changed: RootFieldsChanged;
types_changed: TypesChanged;
operation_not_supported: OperationNotSupported;
}

View file

@ -10,10 +10,18 @@ import type { IndexMapping, VirtualVersionMap } from '@kbn/core-saved-objects-ba
import { getUpdatedRootFields, getUpdatedTypes } from './compare_mappings';
/**
* Diffs the actual vs expected mappings. The properties are compared using md5 hashes stored in _meta, because
* actual and expected mappings *can* differ, but if the md5 hashes stored in actual._meta.migrationMappingPropertyHashes
* match our expectations, we don't require a migration. This allows ES to tack on additional mappings that Kibana
* doesn't know about or expect, without triggering continual migrations.
* Diffs the stored vs app mappings.
* On one hand, it compares changes in root fields, by deep comparing the actual mappings.
* On the other hand, it compares changes in SO types mappings:
* Historically, this comparison was done using md5 hashes.
* Currently, and in order to be FIPS compliant, this has been replaced by comparing model versions.
* The `getUpdatedTypes` uses a map to handle the transition md5 => modelVersion
* @param indexMappings The mappings stored in the SO index
* @param appMappings The current Kibana mappings, computed from the typeRegistry
* @param indexTypes A list of the SO types that are bound to the SO index
* @param latestMappingsVersions A map containing the latest version in which each type has updated its mappings
* @param hashToVersionMap Map that holds md5 => modelVersion equivalence, to smoothly transition away from hashes
*/
export function diffMappings({
indexMappings,

View file

@ -19,7 +19,7 @@ import type {
BaseState,
CalculateExcludeFiltersState,
UpdateSourceMappingsPropertiesState,
CheckTargetMappingsState,
CheckTargetTypesMappingsState,
CheckUnknownDocumentsState,
CheckVersionIndexReadyActions,
CleanupUnknownAndExcluded,
@ -2592,7 +2592,7 @@ describe('migrations v2 model', () => {
it('OUTDATED_DOCUMENTS_SEARCH_CLOSE_PIT -> CHECK_TARGET_MAPPINGS if action succeeded', () => {
const res: ResponseType<'OUTDATED_DOCUMENTS_SEARCH_CLOSE_PIT'> = Either.right({});
const newState = model(state, res) as CheckTargetMappingsState;
const newState = model(state, res) as CheckTargetTypesMappingsState;
expect(newState.controlState).toBe('CHECK_TARGET_MAPPINGS');
// @ts-expect-error pitId shouldn't leak outside
expect(newState.pitId).toBe(undefined);
@ -2600,7 +2600,7 @@ describe('migrations v2 model', () => {
});
describe('CHECK_TARGET_MAPPINGS', () => {
const checkTargetMappingsState: CheckTargetMappingsState = {
const checkTargetTypesMappingsState: CheckTargetTypesMappingsState = {
...postInitState,
controlState: 'CHECK_TARGET_MAPPINGS',
versionIndexReadyActions: Option.none,
@ -2614,7 +2614,7 @@ describe('migrations v2 model', () => {
type: 'index_mappings_incomplete' as const,
});
const newState = model(
checkTargetMappingsState,
checkTargetTypesMappingsState,
res
) as UpdateTargetMappingsPropertiesState;
expect(newState.controlState).toBe('UPDATE_TARGET_MAPPINGS_PROPERTIES');
@ -2623,28 +2623,14 @@ describe('migrations v2 model', () => {
});
describe('compatible migration', () => {
it('CHECK_TARGET_MAPPINGS -> UPDATE_TARGET_MAPPINGS_PROPERTIES if core fields have been updated', () => {
const res: ResponseType<'CHECK_TARGET_MAPPINGS'> = Either.left({
type: 'root_fields_changed' as const,
updatedFields: ['references'],
});
const newState = model(
checkTargetMappingsState,
res
) as UpdateTargetMappingsPropertiesState;
expect(newState.controlState).toBe('UPDATE_TARGET_MAPPINGS_PROPERTIES');
// since a core field has been updated, we must pickup ALL SOs.
// Thus, we must NOT define a filter query.
expect(Option.isNone(newState.updatedTypesQuery)).toEqual(true);
});
it('CHECK_TARGET_MAPPINGS -> UPDATE_TARGET_MAPPINGS_PROPERTIES if only SO types have changed', () => {
it('CHECK_TARGET_MAPPINGS -> UPDATE_TARGET_MAPPINGS_PROPERTIES if SO types have changed', () => {
const res: ResponseType<'CHECK_TARGET_MAPPINGS'> = Either.left({
type: 'types_changed' as const,
updatedFields: [],
updatedTypes: ['dashboard', 'lens'],
});
const newState = model(
checkTargetMappingsState,
checkTargetTypesMappingsState,
res
) as UpdateTargetMappingsPropertiesState;
expect(newState.controlState).toBe('UPDATE_TARGET_MAPPINGS_PROPERTIES');
@ -2668,11 +2654,14 @@ describe('migrations v2 model', () => {
});
});
it('CHECK_TARGET_MAPPINGS -> CHECK_VERSION_INDEX_READY_ACTIONS if mappings match', () => {
it('CHECK_TARGET_MAPPINGS -> CHECK_VERSION_INDEX_READY_ACTIONS if types match (there might be additions in core fields)', () => {
const res: ResponseType<'CHECK_TARGET_MAPPINGS'> = Either.right({
type: 'compared_mappings_match' as const,
type: 'types_match' as const,
});
const newState = model(checkTargetMappingsState, res) as CheckVersionIndexReadyActions;
const newState = model(
checkTargetTypesMappingsState,
res
) as CheckVersionIndexReadyActions;
expect(newState.controlState).toBe('CHECK_VERSION_INDEX_READY_ACTIONS');
});
});

View file

@ -1429,40 +1429,33 @@ export const model = (currentState: State, resW: ResponseType<AllActionStates>):
} else if (stateP.controlState === 'CHECK_TARGET_MAPPINGS') {
const res = resW as ResponseType<typeof stateP.controlState>;
if (Either.isRight(res)) {
// The mappings have NOT changed, no need to pick up changes in any documents
// The types mappings have NOT changed, no need to pick up changes in any documents
return {
...stateP,
controlState: 'CHECK_VERSION_INDEX_READY_ACTIONS',
logs: [
...stateP.logs,
{
level: 'info',
message:
'There are no changes in the mappings of any of the SO types, skipping UPDATE_TARGET_MAPPINGS steps.',
},
],
};
} else {
const left = res.left;
if (isTypeof(left, 'index_mappings_incomplete')) {
// reindex migration
// some top-level properties have changed, e.g. 'dynamic' or '_meta' (see checkTargetMappings())
// some top-level properties have changed, e.g. 'dynamic' or '_meta' (see checkTargetTypesMappings())
// we must "pick-up" all documents on the index (by not providing a query)
return {
...stateP,
controlState: 'UPDATE_TARGET_MAPPINGS_PROPERTIES',
updatedTypesQuery: Option.none,
};
} else if (isTypeof(left, 'root_fields_changed')) {
// compatible migration: some core fields have been updated
return {
...stateP,
controlState: 'UPDATE_TARGET_MAPPINGS_PROPERTIES',
// we must "pick-up" all documents on the index (by not providing a query)
updatedTypesQuery: Option.none,
logs: [
...stateP.logs,
{
level: 'info',
message: `Kibana is performing a compatible upgrade and the mappings of some root fields have been changed. For Elasticsearch to pickup these mappings, all saved objects need to be updated. Updated root fields: ${left.updatedFields}.`,
},
],
};
} else if (isTypeof(left, 'types_changed')) {
// compatible migration: some fields have been updated, and they all correspond to SO types
const updatedTypesQuery = Option.fromNullable(buildPickupMappingsQuery(left.updatedTypes));
// compatible migration: the mappings of some SO types have been updated
const updatedTypesQuery = Option.some(buildPickupMappingsQuery(left.updatedTypes));
return {
...stateP,
@ -1473,7 +1466,7 @@ export const model = (currentState: State, resW: ResponseType<AllActionStates>):
...stateP.logs,
{
level: 'info',
message: `Kibana is performing a compatible upgrade and NO root fields have been updated. Kibana will update the following SO types so that ES can pickup the updated mappings: ${left.updatedTypes}.`,
message: `Documents of the following SO types will be updated, so that ES can pickup the updated mappings: ${left.updatedTypes}.`,
},
],
};

View file

@ -15,7 +15,7 @@ import type { WaitGroup } from './kibana_migrator_utils';
import type {
AllActionStates,
CalculateExcludeFiltersState,
CheckTargetMappingsState,
CheckTargetTypesMappingsState,
CheckUnknownDocumentsState,
CleanupUnknownAndExcluded,
CleanupUnknownAndExcludedWaitForTaskState,
@ -203,8 +203,8 @@ export const nextActionMap = (
}),
REFRESH_TARGET: (state: RefreshTarget) =>
Actions.refreshIndex({ client, index: state.targetIndex }),
CHECK_TARGET_MAPPINGS: (state: CheckTargetMappingsState) =>
Actions.checkTargetMappings({
CHECK_TARGET_MAPPINGS: (state: CheckTargetTypesMappingsState) =>
Actions.checkTargetTypesMappings({
indexTypes: state.indexTypes,
indexMappings: Option.toUndefined(state.sourceIndexMappings),
appMappings: state.targetIndexMappings,

View file

@ -384,7 +384,7 @@ export interface RefreshTarget extends PostInitState {
readonly targetIndex: string;
}
export interface CheckTargetMappingsState extends PostInitState {
export interface CheckTargetTypesMappingsState extends PostInitState {
readonly controlState: 'CHECK_TARGET_MAPPINGS';
}
@ -549,7 +549,7 @@ export interface LegacyDeleteState extends LegacyBaseState {
export type State = Readonly<
| CalculateExcludeFiltersState
| CheckTargetMappingsState
| CheckTargetTypesMappingsState
| CheckUnknownDocumentsState
| CheckVersionIndexReadyActions
| CleanupUnknownAndExcluded

View file

@ -184,7 +184,7 @@ describe('V2 algorithm', () => {
it('only "picks up" the types that have changed', async () => {
const logs = await readLog(logFilePath);
expect(logs).toMatch(
'Kibana is performing a compatible upgrade and NO root fields have been updated. Kibana will update the following SO types so that ES can pickup the updated mappings: another-type.'
'Documents of the following SO types will be updated, so that ES can pickup the updated mappings: another-type.'
);
});
});
@ -263,7 +263,7 @@ describe('V2 algorithm', () => {
it('only "picks up" the types that have changed', async () => {
const logs = await readLog(logFilePath);
expect(logs).toMatch(
'Kibana is performing a compatible upgrade and NO root fields have been updated. Kibana will update the following SO types so that ES can pickup the updated mappings: another-type.'
'Documents of the following SO types will be updated, so that ES can pickup the updated mappings: another-type.'
);
});
});

View file

@ -12,8 +12,8 @@ import {
clearLog,
createBaseline,
defaultKibanaIndex,
defaultLogFilePath,
getCompatibleMappingsMigrator,
getIdenticalMappingsMigrator,
getIncompatibleMappingsMigrator,
startElasticsearch,
} from '../kibana_migrator_test_kit';
@ -31,38 +31,39 @@ describe('pickupUpdatedMappings', () => {
beforeEach(async () => {
await createBaseline();
await clearLog();
await clearLog(logFilePath);
});
describe('when performing a reindexing migration', () => {
it('should pickup all documents from the index', async () => {
const { runMigrations } = await getIncompatibleMappingsMigrator();
const { runMigrations } = await getIncompatibleMappingsMigrator({ logFilePath });
await runMigrations();
const logs = await parseLogFile(defaultLogFilePath);
const logs = await parseLogFile(logFilePath);
expect(logs).not.toContainLogEntry('Documents of the following SO types will be updated');
expect(logs).not.toContainLogEntry(
'Kibana is performing a compatible upgrade and NO root fields have been updated. Kibana will update the following SO types so that ES can pickup the updated mappings'
'There are no changes in the mappings of any of the SO types, skipping UPDATE_TARGET_MAPPINGS steps.'
);
});
});
describe('when performing a compatible migration', () => {
it('should pickup only the types that have been updated', async () => {
const { runMigrations } = await getCompatibleMappingsMigrator();
const { runMigrations } = await getCompatibleMappingsMigrator({ logFilePath });
await runMigrations();
const logs = await parseLogFile(defaultLogFilePath);
const logs = await parseLogFile(logFilePath);
expect(logs).toContainLogEntry(
'Kibana is performing a compatible upgrade and NO root fields have been updated. Kibana will update the following SO types so that ES can pickup the updated mappings: complex.'
'Documents of the following SO types will be updated, so that ES can pickup the updated mappings: complex.'
);
});
it('should pickup ALL documents if any root fields have been updated', async () => {
const { runMigrations, client } = await getCompatibleMappingsMigrator();
it('should NOT pickup any documents if only root fields have been updated', async () => {
const { runMigrations, client } = await getIdenticalMappingsMigrator({ logFilePath });
// we tamper the baseline mappings to simulate some root fields changes
const baselineMappings = await client.indices.getMapping({ index: defaultKibanaIndex });
@ -74,14 +75,12 @@ describe('pickupUpdatedMappings', () => {
await runMigrations();
const logs = await parseLogFile(defaultLogFilePath);
const logs = await parseLogFile(logFilePath);
expect(logs).toContainLogEntry(
'Kibana is performing a compatible upgrade and the mappings of some root fields have been changed. For Elasticsearch to pickup these mappings, all saved objects need to be updated. Updated root fields: references.'
);
expect(logs).not.toContainLogEntry(
'Kibana is performing a compatible upgrade and NO root fields have been updated. Kibana will update the following SO types so that ES can pickup the updated mappings'
'There are no changes in the mappings of any of the SO types, skipping UPDATE_TARGET_MAPPINGS steps.'
);
expect(logs).not.toContainLogEntry('Documents of the following SO types will be updated');
});
});

View file

@ -434,15 +434,18 @@ export const createBaseline = async () => {
};
interface GetMutatedMigratorParams {
logFilePath?: string;
kibanaVersion?: string;
settings?: Record<string, any>;
}
export const getIdenticalMappingsMigrator = async ({
logFilePath = defaultLogFilePath,
kibanaVersion = nextMinor,
settings = {},
}: GetMutatedMigratorParams = {}) => {
return await getKibanaMigratorTestKit({
logFilePath,
types: baselineTypes,
kibanaVersion,
settings,
@ -450,10 +453,12 @@ export const getIdenticalMappingsMigrator = async ({
};
export const getNonDeprecatedMappingsMigrator = async ({
logFilePath = defaultLogFilePath,
kibanaVersion = nextMinor,
settings = {},
}: GetMutatedMigratorParams = {}) => {
return await getKibanaMigratorTestKit({
logFilePath,
types: baselineTypes.filter((type) => type.name !== 'deprecated'),
kibanaVersion,
settings,
@ -461,6 +466,7 @@ export const getNonDeprecatedMappingsMigrator = async ({
};
export const getCompatibleMappingsMigrator = async ({
logFilePath = defaultLogFilePath,
filterDeprecated = false,
kibanaVersion = nextMinor,
settings = {},
@ -499,6 +505,7 @@ export const getCompatibleMappingsMigrator = async ({
});
return await getKibanaMigratorTestKit({
logFilePath,
types,
kibanaVersion,
settings,
@ -506,6 +513,7 @@ export const getCompatibleMappingsMigrator = async ({
};
export const getIncompatibleMappingsMigrator = async ({
logFilePath = defaultLogFilePath,
kibanaVersion = nextMinor,
settings = {},
}: GetMutatedMigratorParams = {}) => {
@ -544,6 +552,7 @@ export const getIncompatibleMappingsMigrator = async ({
});
return await getKibanaMigratorTestKit({
logFilePath,
types,
kibanaVersion,
settings,