Skip to content

Commit

Permalink
[ZDT] Pickup updated types only
Browse files Browse the repository at this point in the history
  • Loading branch information
gsoldevila committed Jul 3, 2023
1 parent 44c7091 commit 397c14c
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ describe('checkTargetMappings', () => {
describe('when actual mappings are incomplete', () => {
it("returns 'actual_mappings_incomplete' if actual mappings are not defined", async () => {
const task = checkTargetMappings({
indexTypes,
expectedMappings,
});

Expand All @@ -58,7 +57,6 @@ describe('checkTargetMappings', () => {

it("returns 'actual_mappings_incomplete' if actual mappings do not define _meta", async () => {
const task = checkTargetMappings({
indexTypes,
expectedMappings,
actualMappings: {
properties,
Expand All @@ -72,7 +70,6 @@ describe('checkTargetMappings', () => {

it("returns 'actual_mappings_incomplete' if actual mappings do not define migrationMappingPropertyHashes", async () => {
const task = checkTargetMappings({
indexTypes,
expectedMappings,
actualMappings: {
properties,
Expand All @@ -87,7 +84,6 @@ describe('checkTargetMappings', () => {

it("returns 'actual_mappings_incomplete' if actual mappings define a different value for 'dynamic' property", async () => {
const task = checkTargetMappings({
indexTypes,
expectedMappings,
actualMappings: {
properties,
Expand All @@ -105,7 +101,6 @@ describe('checkTargetMappings', () => {
describe('and mappings do not match', () => {
it('returns the lists of changed root fields and types', async () => {
const task = checkTargetMappings({
indexTypes,
expectedMappings,
actualMappings: expectedMappings,
});
Expand All @@ -115,8 +110,7 @@ describe('checkTargetMappings', () => {
const result = await task();
const expected: ComparedMappingsChanged = {
type: 'compared_mappings_changed' as const,
updatedRootFields: ['someRootField'],
updatedTypes: ['type1', 'type2'],
updatedHashes: ['type1', 'type2', 'someRootField'],
};
expect(result).toEqual(Either.left(expected));
});
Expand All @@ -125,7 +119,6 @@ describe('checkTargetMappings', () => {
describe('and mappings match', () => {
it('returns a compared_mappings_match response', async () => {
const task = checkTargetMappings({
indexTypes,
expectedMappings,
actualMappings: expectedMappings,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { getUpdatedHashes } from '../core/build_active_mappings';

/** @internal */
export interface CheckTargetMappingsParams {
indexTypes: string[];
actualMappings?: IndexMapping;
expectedMappings: IndexMapping;
}
Expand All @@ -29,13 +28,11 @@ export interface ActualMappingsIncomplete {

export interface ComparedMappingsChanged {
type: 'compared_mappings_changed';
updatedRootFields: string[];
updatedTypes: string[];
updatedHashes: string[];
}

export const checkTargetMappings =
({
indexTypes,
actualMappings,
expectedMappings,
}: CheckTargetMappingsParams): TaskEither.TaskEither<
Expand All @@ -56,12 +53,9 @@ export const checkTargetMappings =
});

if (updatedHashes.length) {
const updatedTypes = updatedHashes.filter((field) => indexTypes.includes(field));
const updatedRootFields = updatedHashes.filter((field) => !indexTypes.includes(field));
return Either.left({
type: 'compared_mappings_changed' as const,
updatedRootFields,
updatedTypes,
updatedHashes,
});
} else {
return Either.right({ type: 'compared_mappings_match' as const });
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { buildPickupMappingsQuery } from './build_pickup_mappings_query';

describe('buildPickupMappingsQuery', () => {
describe('when no root fields have been updated', () => {
it('builds a boolean query to select the updated types', () => {
const query = buildPickupMappingsQuery({
rootFields: ['someRootField'],
updatedFields: ['type1', 'type2'],
});

expect(query).toEqual({
bool: {
should: [{ term: { type: 'type1' } }, { term: { type: 'type2' } }],
},
});
});
});

describe('when some root fields have been updated', () => {
it('returns undefined', () => {
const query = buildPickupMappingsQuery({
rootFields: ['someRootField'],
updatedFields: ['type1', 'type2', 'someRootField'],
});

expect(query).toBeUndefined();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/types';

export const buildPickupMappingsQuery = ({
rootFields,
updatedFields,
}: {
rootFields: string[];
updatedFields: string[];
}): QueryDslQueryContainer | undefined => {
if (updatedFields.some((field) => rootFields.includes(field))) {
// we are updating some root fields, update ALL documents (no filter query)
return undefined;
}

// at this point, all updated fields correspond to SO types
const updatedTypes = updatedFields;

return {
bool: {
should: updatedTypes.map((type) => ({ term: { type } })),
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -2615,8 +2615,7 @@ describe('migrations v2 model', () => {
it('CHECK_TARGET_MAPPINGS -> UPDATE_TARGET_MAPPINGS_PROPERTIES if core fields have been updated', () => {
const res: ResponseType<'CHECK_TARGET_MAPPINGS'> = Either.left({
type: 'compared_mappings_changed' as const,
updatedRootFields: ['namespaces'],
updatedTypes: ['dashboard', 'lens'],
updatedHashes: ['dashboard', 'lens', 'namespaces'],
});
const newState = model(
checkTargetMappingsState,
Expand All @@ -2631,8 +2630,7 @@ describe('migrations v2 model', () => {
it('CHECK_TARGET_MAPPINGS -> UPDATE_TARGET_MAPPINGS_PROPERTIES if only SO types have changed', () => {
const res: ResponseType<'CHECK_TARGET_MAPPINGS'> = Either.left({
type: 'compared_mappings_changed' as const,
updatedRootFields: [],
updatedTypes: ['dashboard', 'lens'],
updatedHashes: ['dashboard', 'lens'],
});
const newState = model(
checkTargetMappingsState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ import {
CLUSTER_SHARD_LIMIT_EXCEEDED_REASON,
FATAL_REASON_REQUEST_ENTITY_TOO_LARGE,
} from '../common/constants';
import { getBaseMappings } from '../core';
import { buildPickupMappingsQuery } from '../core/build_pickup_mappings_query';

export const model = (currentState: State, resW: ResponseType<AllActionStates>): State => {
// The action response `resW` is weakly typed, the type includes all action
Expand Down Expand Up @@ -1439,18 +1441,24 @@ export const model = (currentState: State, resW: ResponseType<AllActionStates>):
updatedTypesQuery: Option.none,
};
} else if (isTypeof(left, 'compared_mappings_changed')) {
if (left.updatedRootFields.length) {
const rootFields = Object.keys(getBaseMappings().properties);
const updatedRootFields = left.updatedHashes.filter((field) => rootFields.includes(field));
const updatedTypesQuery = Option.fromNullable(
buildPickupMappingsQuery({ rootFields, updatedFields: left.updatedHashes })
);

if (updatedRootFields.length) {
// 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,
updatedTypesQuery,
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.updatedRootFields}.`,
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: ${updatedRootFields}.`,
},
],
};
Expand All @@ -1460,16 +1468,12 @@ export const model = (currentState: State, resW: ResponseType<AllActionStates>):
...stateP,
controlState: 'UPDATE_TARGET_MAPPINGS_PROPERTIES',
// we can "pick-up" only the SO types that have changed
updatedTypesQuery: Option.some({
bool: {
should: left.updatedTypes.map((type) => ({ term: { type } })),
},
}),
updatedTypesQuery,
logs: [
...stateP.logs,
{
level: 'info',
message: `Kibana is performing a compatible upgrade and NO root fields have been udpated. Kibana will update the following SO types so that ES can pickup the updated mappings: ${left.updatedTypes}.`,
message: `Kibana is performing a compatible upgrade and NO root fields have been udpated. Kibana will update the following SO types so that ES can pickup the updated mappings: ${left.updatedHashes}.`,
},
],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import { createDelayFn } from './common/utils';
import type { TransformRawDocs } from './types';
import * as Actions from './actions';
import { REMOVED_TYPES } from './core';
import { getIndexTypes } from './model/helpers';

type ActionMap = ReturnType<typeof nextActionMap>;

Expand Down Expand Up @@ -202,7 +201,6 @@ export const nextActionMap = (
Actions.checkTargetMappings({
actualMappings: Option.toUndefined(state.sourceIndexMappings),
expectedMappings: state.targetIndexMappings,
indexTypes: getIndexTypes(state),
}),
UPDATE_TARGET_MAPPINGS_PROPERTIES: (state: UpdateTargetMappingsPropertiesState) =>
Actions.updateAndPickupMappings({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {
SetDocMigrationStartedState,
UpdateMappingModelVersionState,
UpdateDocumentModelVersionsState,
UpdateIndexMappingsState,
} from './state';

describe('actions', () => {
Expand Down Expand Up @@ -147,4 +148,67 @@ describe('actions', () => {
});
});
});

describe('UPDATE_INDEX_MAPPINGS', () => {
describe('when only SO types have been updated', () => {
it('calls updateAndPickupMappings with the correct parameters', () => {
const state: UpdateIndexMappingsState = {
...createPostDocInitState(),
controlState: 'UPDATE_INDEX_MAPPINGS',
additiveMappingChanges: {
someToken: {},
},
};
const action = actionMap.UPDATE_INDEX_MAPPINGS;

action(state);

expect(ActionMocks.updateAndPickupMappings).toHaveBeenCalledTimes(1);
expect(ActionMocks.updateAndPickupMappings).toHaveBeenCalledWith({
client: context.elasticsearchClient,
index: state.currentIndex,
mappings: {
properties: {
someToken: {},
},
},
batchSize: context.batchSize,
query: {
bool: {
should: [{ term: { type: 'someToken' } }],
},
},
});
});
});

describe('when core properties have been updated', () => {
it('calls updateAndPickupMappings with the correct parameters', () => {
const state: UpdateIndexMappingsState = {
...createPostDocInitState(),
controlState: 'UPDATE_INDEX_MAPPINGS',
additiveMappingChanges: {
managed: {}, // this is a root field
someToken: {},
},
};
const action = actionMap.UPDATE_INDEX_MAPPINGS;

action(state);

expect(ActionMocks.updateAndPickupMappings).toHaveBeenCalledTimes(1);
expect(ActionMocks.updateAndPickupMappings).toHaveBeenCalledWith({
client: context.elasticsearchClient,
index: state.currentIndex,
mappings: {
properties: {
managed: {},
someToken: {},
},
},
batchSize: context.batchSize,
});
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import {
setMetaDocMigrationComplete,
setMetaDocMigrationStarted,
} from './utils';
import { buildPickupMappingsQuery } from '../core/build_pickup_mappings_query';
import { getBaseMappings } from '../core';

export type ActionMap = ReturnType<typeof nextActionMap>;

Expand Down Expand Up @@ -72,6 +74,10 @@ export const nextActionMap = (context: MigratorContext) => {
index: state.currentIndex,
mappings: { properties: state.additiveMappingChanges },
batchSize: context.batchSize,
query: buildPickupMappingsQuery({
rootFields: Object.keys(getBaseMappings().properties),
updatedFields: Object.keys(state.additiveMappingChanges),
}),
}),
UPDATE_INDEX_MAPPINGS_WAIT_FOR_TASK: (state: UpdateIndexMappingsWaitForTaskState) =>
Actions.waitForPickupUpdatedMappingsTask({
Expand Down

0 comments on commit 397c14c

Please sign in to comment.