Allow for additive mappings update without creating a new version ind…

…ex (#149326)

Fixes [#147237](#147237)

Based on the same principle as
[#147371](#147371), the goal of
this PR is to **avoid reindexing if possible**.
This time, the idea is to check whether the new mappings are still
compatible with the ones stored in ES.
To to so, we attempt to update the mappings in place in the existing
index, introducing a new `CHECK_COMPATIBLE_MAPPINGS` step:
* If the update operation fails, we assume the mappings are NOT
compatible, and we continue with the normal reindexing flow.
* If the update operation succeeds, we assume the mappings ARE
compatible, and we skip reindexing, just like
[#147371](#147371) does.


![image](https://user-images.githubusercontent.com/25349407/216979882-9fe9f034-b521-4171-b85d-50be6a13e179.png)

---------

Co-authored-by: kibanamachine <[email protected]>

packages/core/saved-objects/core-saved-objects-migration-server-internal/index.ts

@@ -28,7 +28,7 @@ export {
   cloneIndex,
   waitForTask,
   updateAndPickupMappings,
-  updateTargetMappingsMeta,
+  updateMappings,
   updateAliases,
   transformDocs,
   setWriteBlock,

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/index.ts

@@ -6,7 +6,8 @@
  * Side Public License, v 1.
  */
 
-import { type Either, right } from 'fp-ts/lib/Either';
+import type { Either } from 'fp-ts/lib/Either';
+import { right } from 'fp-ts/lib/Either';
 import type { RetryableEsClientError } from './catch_retryable_es_client_errors';
 import type { DocumentsTransformFailed } from '../core/migrate_raw_docs';
 
@@ -37,11 +38,8 @@ export type { CloneIndexResponse, CloneIndexParams } from './clone_index';
 export { cloneIndex } from './clone_index';
 
 export type { WaitForIndexStatusParams, IndexNotYellowTimeout } from './wait_for_index_status';
-import {
-  type IndexNotGreenTimeout,
-  type IndexNotYellowTimeout,
-  waitForIndexStatus,
-} from './wait_for_index_status';
+import type { IndexNotGreenTimeout, IndexNotYellowTimeout } from './wait_for_index_status';
+import { waitForIndexStatus } from './wait_for_index_status';
 
 export type { WaitForTaskResponse, WaitForTaskCompletionTimeout } from './wait_for_task';
 import { waitForTask, WaitForTaskCompletionTimeout } from './wait_for_task';
@@ -85,8 +83,6 @@ export { createIndex } from './create_index';
 
 export { checkTargetMappings } from './check_target_mappings';
 
-export { updateTargetMappingsMeta } from './update_target_mappings_meta';
-
 export const noop = async (): Promise<Either<never, 'noop'>> => right('noop' as const);
 
 export type {
@@ -95,6 +91,8 @@ export type {
 } from './update_and_pickup_mappings';
 export { updateAndPickupMappings } from './update_and_pickup_mappings';
 
+export { updateMappings } from './update_mappings';
+
 import type { UnknownDocsFound } from './check_for_unknown_docs';
 import type { IncompatibleClusterRoutingAllocation } from './initialize_action';
 import { ClusterShardLimitExceeded } from './create_index';

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/update_and_pickup_mappings.test.ts

@@ -46,7 +46,7 @@ describe('updateAndPickupMappings', () => {
     expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
   });
 
-  it('updates the _mapping properties but not the _meta information', async () => {
+  it('calls the indices.putMapping with the mapping properties as well as the _meta information', async () => {
     const task = updateAndPickupMappings({
       client,
       index: 'new_index',
@@ -82,6 +82,13 @@ describe('updateAndPickupMappings', () => {
           dynamic: false,
         },
       },
+      _meta: {
+        migrationMappingPropertyHashes: {
+          references: '7997cf5a56cc02bdc9c93361bde732b0',
+          'epm-packages': '860e23f4404fa1c33f430e6dad5d8fa2',
+          'cases-connector-mappings': '17d2e9e0e170a21a471285a5d845353c',
+        },
+      },
     });
   });
 });

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/update_and_pickup_mappings.ts

@@ -45,13 +45,11 @@ export const updateAndPickupMappings = ({
     RetryableEsClientError,
     'update_mappings_succeeded'
   > = () => {
-    // ._meta property will be updated on a later step
-    const { _meta, ...mappingsWithoutMeta } = mappings;
     return client.indices
       .putMapping({
         index,
         timeout: DEFAULT_TIMEOUT,
-        ...mappingsWithoutMeta,
+        ...mappings,
       })
       .then(() => {
         // Ignore `acknowledged: false`. When the coordinating node accepts

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/update_mappings.test.ts

@@ -0,0 +1,147 @@
+/*
+ * 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 { TransportResult } from '@elastic/elasticsearch';
+import { errors as EsErrors } from '@elastic/elasticsearch';
+import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
+import { catchRetryableEsClientErrors } from './catch_retryable_es_client_errors';
+import { updateMappings } from './update_mappings';
+import { DEFAULT_TIMEOUT } from './constants';
+
+jest.mock('./catch_retryable_es_client_errors');
+
+describe('updateMappings', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  const createErrorClient = (response: Partial<TransportResult<Record<string, any>>>) => {
+    // Create a mock client that returns the desired response
+    const apiResponse = elasticsearchClientMock.createApiResponse(response);
+    const error = new EsErrors.ResponseError(apiResponse);
+    const client = elasticsearchClientMock.createInternalClient(
+      elasticsearchClientMock.createErrorTransportRequestPromise(error)
+    );
+
+    return { client, error };
+  };
+
+  it('resolves left if the mappings are not compatible (aka 400 illegal_argument_exception from ES)', async () => {
+    const { client } = createErrorClient({
+      statusCode: 400,
+      body: {
+        error: {
+          type: 'illegal_argument_exception',
+          reason: 'mapper [action.actionTypeId] cannot be changed from type [keyword] to [text]',
+        },
+      },
+    });
+
+    const task = updateMappings({
+      client,
+      index: 'new_index',
+      mappings: {
+        properties: {
+          created_at: {
+            type: 'date',
+          },
+        },
+        _meta: {
+          migrationMappingPropertyHashes: {
+            references: '7997cf5a56cc02bdc9c93361bde732b0',
+            'epm-packages': '860e23f4404fa1c33f430e6dad5d8fa2',
+            'cases-connector-mappings': '17d2e9e0e170a21a471285a5d845353c',
+          },
+        },
+      },
+    });
+
+    const res = await task();
+
+    expect(Either.isLeft(res)).toEqual(true);
+    expect(res).toMatchInlineSnapshot(`
+      Object {
+        "_tag": "Left",
+        "left": Object {
+          "type": "incompatible_mapping_exception",
+        },
+      }
+    `);
+  });
+
+  it('calls catchRetryableEsClientErrors when the promise rejects', async () => {
+    const { client, error: retryableError } = createErrorClient({
+      statusCode: 503,
+      body: { error: { type: 'es_type', reason: 'es_reason' } },
+    });
+
+    const task = updateMappings({
+      client,
+      index: 'new_index',
+      mappings: {
+        properties: {
+          created_at: {
+            type: 'date',
+          },
+        },
+        _meta: {},
+      },
+    });
+    try {
+      await task();
+    } catch (e) {
+      /** ignore */
+    }
+
+    expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
+  });
+
+  it('updates the mapping information of the desired index', async () => {
+    const client = elasticsearchClientMock.createInternalClient();
+
+    const task = updateMappings({
+      client,
+      index: 'new_index',
+      mappings: {
+        properties: {
+          created_at: {
+            type: 'date',
+          },
+        },
+        _meta: {
+          migrationMappingPropertyHashes: {
+            references: '7997cf5a56cc02bdc9c93361bde732b0',
+            'epm-packages': '860e23f4404fa1c33f430e6dad5d8fa2',
+            'cases-connector-mappings': '17d2e9e0e170a21a471285a5d845353c',
+          },
+        },
+      },
+    });
+
+    const res = await task();
+    expect(Either.isRight(res)).toBe(true);
+    expect(client.indices.putMapping).toHaveBeenCalledTimes(1);
+    expect(client.indices.putMapping).toHaveBeenCalledWith({
+      index: 'new_index',
+      timeout: DEFAULT_TIMEOUT,
+      properties: {
+        created_at: {
+          type: 'date',
+        },
+      },
+      _meta: {
+        migrationMappingPropertyHashes: {
+          references: '7997cf5a56cc02bdc9c93361bde732b0',
+          'epm-packages': '860e23f4404fa1c33f430e6dad5d8fa2',
+          'cases-connector-mappings': '17d2e9e0e170a21a471285a5d845353c',
+        },
+      },
+    });
+  });
+});

packages/core/saved-objects/core-saved-objects-migration-server-internal/src/actions/update_mappings.ts

@@ -0,0 +1,63 @@
+/*
+ * 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 type { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
+import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal';
+import { catchRetryableEsClientErrors } from './catch_retryable_es_client_errors';
+import type { RetryableEsClientError } from './catch_retryable_es_client_errors';
+import { DEFAULT_TIMEOUT } from './constants';
+
+/** @internal */
+export interface UpdateMappingsParams {
+  client: ElasticsearchClient;
+  index: string;
+  mappings: IndexMapping;
+}
+
+/** @internal */
+export interface IncompatibleMappingException {
+  type: 'incompatible_mapping_exception';
+}
+
+/**
+ * Updates an index's mappings and runs an pickupUpdatedMappings task so that the mapping
+ * changes are "picked up". Returns a taskId to track progress.
+ */
+export const updateMappings = ({
+  client,
+  index,
+  mappings,
+}: UpdateMappingsParams): TaskEither.TaskEither<
+  RetryableEsClientError | IncompatibleMappingException,
+  'update_mappings_succeeded'
+> => {
+  return () => {
+    return client.indices
+      .putMapping({
+        index,
+        timeout: DEFAULT_TIMEOUT,
+        ...mappings,
+      })
+      .then(() => Either.right('update_mappings_succeeded' as const))
+      .catch((res) => {
+        const errorType = res?.body?.error?.type;
+        // ES throws this exact error when attempting to make incompatible updates to the mappigns
+        if (
+          res?.statusCode === 400 &&
+          (errorType === 'illegal_argument_exception' ||
+            errorType === 'strict_dynamic_mapping_exception' ||
+            errorType === 'mapper_parsing_exception')
+        ) {
+          return Either.left({ type: 'incompatible_mapping_exception' });
+        }
+        return catchRetryableEsClientErrors(res);
+      });
+  };
+};