-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Reduce startup time by skipping update mappings step when possible (#…
…145604) The goal of this PR is to reduce the startup times of Kibana server by improving the migration logic. Fixes #145743 Related #144035) The migration logic is run systematically at startup, whether the customers are upgrading or not. Historically, these steps have been very quick, but we recently found out about some customers that have more than **one million** Saved Objects stored, making the overall startup process slow, even when there are no migrations to perform. This PR specifically targets the case where there are no migrations to perform, aka a Kibana node is started against an ES cluster that is already up to date wrt stack version and list of plugins. In this scenario, we aim at skipping the `UPDATE_TARGET_MAPPINGS` step of the migration logic, which internally runs the `updateAndPickupMappings` method, which turns out to be expensive if the system indices contain lots of SO. I locally tested the following scenarios too: - **Fresh install.** The step is not even run, as the `.kibana` index did not exist ✅ - **Stack version + list of plugins up to date.** Simply restarting Kibana after the fresh install. The step is run and leads to `DONE`, as the md5 hashes match those stored in `.kibana._mapping._meta` ✅ - **Faking re-enabling an old plugin.** I manually removed one of the MD5 hashes from the stored .kibana._mapping._meta through `curl`, and then restarted Kibana. The step is run and leads to `UPDATE_TARGET_MAPPINGS` as it used to before the PR ✅ - **Faking updating a plugin.** Same as the previous one, but altering an existing md5 stored in the metas. ✅ And that is the curl command used to tamper with the stored _meta: ```bash curl -X PUT "kibana:changeme@localhost:9200/.kibana/_mapping?pretty" -H 'Content-Type: application/json' -d' { "_meta": { "migrationMappingPropertyHashes": { "references": "7997cf5a56cc02bdc9c93361bde732b0", } } } ' ```
- Loading branch information
1 parent
185993c
commit b1e18a0
Showing
16 changed files
with
810 additions
and
71 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
79 changes: 79 additions & 0 deletions
79
...ts/core-saved-objects-migration-server-internal/src/actions/check_target_mappings.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| /* | ||
| * 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 * as Either from 'fp-ts/lib/Either'; | ||
| import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal'; | ||
| import { checkTargetMappings } from './check_target_mappings'; | ||
| import { diffMappings } from '../core/build_active_mappings'; | ||
|
|
||
| jest.mock('../core/build_active_mappings'); | ||
|
|
||
| const diffMappingsMock = diffMappings as jest.MockedFn<typeof diffMappings>; | ||
|
|
||
| const sourceIndexMappings: IndexMapping = { | ||
| properties: { | ||
| field: { type: 'integer' }, | ||
| }, | ||
| }; | ||
|
|
||
| const targetIndexMappings: IndexMapping = { | ||
| properties: { | ||
| field: { type: 'long' }, | ||
| }, | ||
| }; | ||
|
|
||
| describe('checkTargetMappings', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('returns match=false if source mappings are not defined', async () => { | ||
| const task = checkTargetMappings({ | ||
| targetIndexMappings, | ||
| }); | ||
|
|
||
| const result = await task(); | ||
| expect(diffMappings).not.toHaveBeenCalled(); | ||
| expect(result).toEqual(Either.right({ match: false })); | ||
| }); | ||
|
|
||
| it('calls diffMappings() with the source and target mappings', async () => { | ||
| const task = checkTargetMappings({ | ||
| sourceIndexMappings, | ||
| targetIndexMappings, | ||
| }); | ||
|
|
||
| await task(); | ||
| expect(diffMappings).toHaveBeenCalledTimes(1); | ||
| expect(diffMappings).toHaveBeenCalledWith(sourceIndexMappings, targetIndexMappings); | ||
| }); | ||
|
|
||
| it('returns match=true if diffMappings() match', async () => { | ||
| diffMappingsMock.mockReturnValueOnce(undefined); | ||
|
|
||
| const task = checkTargetMappings({ | ||
| sourceIndexMappings, | ||
| targetIndexMappings, | ||
| }); | ||
|
|
||
| const result = await task(); | ||
| expect(result).toEqual(Either.right({ match: true })); | ||
| }); | ||
|
|
||
| it('returns match=false if diffMappings() finds differences', async () => { | ||
| diffMappingsMock.mockReturnValueOnce({ changedProp: 'field' }); | ||
|
|
||
| const task = checkTargetMappings({ | ||
| sourceIndexMappings, | ||
| targetIndexMappings, | ||
| }); | ||
|
|
||
| const result = await task(); | ||
| expect(result).toEqual(Either.right({ match: false })); | ||
| }); | ||
| }); |
36 changes: 36 additions & 0 deletions
36
...objects/core-saved-objects-migration-server-internal/src/actions/check_target_mappings.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| /* | ||
| * 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 * as Either from 'fp-ts/lib/Either'; | ||
| import * as TaskEither from 'fp-ts/lib/TaskEither'; | ||
|
|
||
| import { IndexMapping } from '@kbn/core-saved-objects-base-server-internal'; | ||
| import { diffMappings } from '../core/build_active_mappings'; | ||
|
|
||
| /** @internal */ | ||
| export interface CheckTargetMappingsParams { | ||
| sourceIndexMappings?: IndexMapping; | ||
| targetIndexMappings: IndexMapping; | ||
| } | ||
|
|
||
| /** @internal */ | ||
| export interface TargetMappingsCompareResult { | ||
| match: boolean; | ||
| } | ||
|
|
||
| export const checkTargetMappings = | ||
| ({ | ||
| sourceIndexMappings, | ||
| targetIndexMappings, | ||
| }: CheckTargetMappingsParams): TaskEither.TaskEither<never, TargetMappingsCompareResult> => | ||
| async () => { | ||
| if (!sourceIndexMappings) { | ||
| return Either.right({ match: false }); | ||
| } | ||
| const diff = diffMappings(sourceIndexMappings, targetIndexMappings); | ||
| return Either.right({ match: !diff }); | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.