[Upgrade Assistant] Reindexing optimizations

x-pack/plugins/upgrade_assistant/README.md

@@ -40,16 +40,18 @@ Elasticsearch deprecations can be handled in a number of ways:
 * **Reindexing.** When a user's index contains deprecations (e.g. mappings) a reindex solves them. The Upgrade Assistant only automates reindexing for indices. For example, if you are currently on 7.x, and want to migrate to 8.0, but you still have indices that were created on 6.x. For this scenario, the user will see a "Reindex" button that they can click, which will perform the reindex.
   * Reindexing is an idempotent action in Upgrade Assistant. It works like this ([overview in code](https://github.com/elastic/kibana/blob/b320a37d8b703b2fa101a93b6971b36ee2c37f06/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts#L498-L540)):
     1. Set a write-block on the original index, no new data can be written during reindexing.
-    2. Create a target index with the following name `reindexed-v{majorVersion}-{originalIndex}`. E.g., if `my-index` is the original, the target will be named `reindexed-v8-my-index`.
+    2. Create a target index with the following name `reindexed-v{majorVersion}-{originalIndex}`. E.g., if `my-index` is the original, the target will be named `reindexed-v8-my-index`.    
+      **NOTE:** It overrides the index settings `number_of_replicas` and `refresh_interval` for reindexing performance. They are restored after completion.
     3. [Reindex](https://www.elastic.co/guide/en/elasticsearch/reference/master/docs-reindex.html) from the original index to the target index. Kibana will continuously report reindexing status.
-    4. Once reindexing is done, in one atomic operation via the aliases API:
-      1. Create an alias from the original index to the target index. All existing aliases referencing the original index will be re-pointed to the target index. E.g., `my-index` will be an alias referencing `reindexed-v8-my-index`.
-      2. Delete the original index.
-      3. **NOTE:** writing/indexing will effectively be re-enabled at this point via the alias, unless the original was write-blocked by users. This and other index settings are inherited from the original.
-    5. The Upgrade Assistant's reindex action is complete at this point.
+    4. Restores the original `number_of_replicas` and `refresh_interval` index settings.
+    5. Once reindexing is done, in one atomic operation via the aliases API:
+       1. Create an alias from the original index to the target index. All existing aliases referencing the original index will be re-pointed to the target index. E.g., `my-index` will be an alias referencing `reindexed-v8-my-index`.
+       2. Delete the original index.
+       3. **NOTE:** writing/indexing will effectively be re-enabled at this point via the alias, unless the original was write-blocked by users. This and other index settings are inherited from the original.
+    6. The Upgrade Assistant's reindex action is complete at this point.
        1. If the original index was closed before reindexing, the new target will also be closed at this point.
 
-    Currently reindexing deprecations are only enabled for major version upgrades  by setting the config `featureSet.reindexCorrectiveActions` to `true` on the `x.last` version of the stack.
+    Currently, reindexing deprecations are only enabled for major version upgrades by setting the config `featureSet.reindexCorrectiveActions` to `true` on the `x.last` version of the stack.
 
 Reindexing at the moment includes some logic that is specific to the [8.0 upgrade](https://github.com/elastic/kibana/blob/main/x-pack/plugins/upgrade_assistant/server/lib/reindexing/index_settings.ts). End users could get into a bad situation if this is enabled before this logic is fixed.
 * **Removing settings.** Some index and cluser settings are deprecated and need to be removed. The Upgrade Assistant provides a way to auto-resolve these settings via a "Remove deprecated settings" button. Migrating system indices should only be enabled for major version upgrades. This is controlled by the config `featureSet.migrateSystemIndices` which hides the second step from the UA UI for migrating system indices.

x-pack/plugins/upgrade_assistant/common/types.ts

@@ -28,6 +28,7 @@ export enum ReindexStep {
   newIndexCreated = 30,
   reindexStarted = 40,
   reindexCompleted = 50,
+  indexSettingsRestored = 55,
   aliasCreated = 60,
   originalIndexDeleted = 70,
   existingAliasesUpdated = 80,
@@ -109,6 +110,16 @@ export interface ReindexOperation {
   // This field is only used for the singleton IndexConsumerType documents.
   runningReindexCount: number | null;
 
+  /**
+   * The original index settings to set after reindex is completed.
+   * The target index is created with other defaults to improve reindexing performance.
+   * https://github.com/elastic/kibana/issues/201605
+   */
+  backupSettings?: {
+    'index.number_of_replicas'?: number;
+    'index.refresh_interval'?: number;
+  };
+
   /**
    * Options for the reindexing strategy.
    *

x-pack/plugins/upgrade_assistant/public/application/components/es_deprecations/deprecation_types/reindex/flyout/progress.test.tsx

@@ -104,6 +104,23 @@ describe('ReindexProgress', () => {
                   }
                 />,
               },
+              Object {
+                "status": "incomplete",
+                "title": <Memo(MemoizedFormattedMessage)
+                  defaultMessage="Copy original index settings from {indexName} to {reindexName}."
+                  id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexingChecklist.indexSettingsRestoredStepTitle"
+                  values={
+                    Object {
+                      "indexName": <EuiCode>
+                        foo
+                      </EuiCode>,
+                      "reindexName": <EuiCode>
+                        reindexed-foo
+                      </EuiCode>,
+                    }
+                  }
+                />,
+              },
               Object {
                 "status": "incomplete",
                 "title": <Memo(MemoizedFormattedMessage)

x-pack/plugins/upgrade_assistant/public/application/components/es_deprecations/deprecation_types/reindex/flyout/progress.tsx

@@ -178,6 +178,28 @@ const getStepTitle = (
     );
   }
 
+  if (step === ReindexStep.indexSettingsRestored) {
+    return inProgress ? (
+      <FormattedMessage
+        id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexingChecklist.inProgress.indexSettingsRestoredStepTitle"
+        defaultMessage="Copying original index settings from {indexName} to {reindexName}."
+        values={{
+          indexName: <EuiCode>{meta.indexName}</EuiCode>,
+          reindexName: <EuiCode>{meta.reindexName}</EuiCode>,
+        }}
+      />
+    ) : (
+      <FormattedMessage
+        id="xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexingChecklist.indexSettingsRestoredStepTitle"
+        defaultMessage="Copy original index settings from {indexName} to {reindexName}."
+        values={{
+          indexName: <EuiCode>{meta.indexName}</EuiCode>,
+          reindexName: <EuiCode>{meta.reindexName}</EuiCode>,
+        }}
+      />
+    );
+  }
+
   if (step === ReindexStep.aliasCreated) {
     return inProgress ? (
       <FormattedMessage
@@ -339,6 +361,7 @@ export const ReindexProgress: React.FunctionComponent<Props> = (props) => {
     getProgressStep(ReindexStep.readonly),
     getProgressStep(ReindexStep.newIndexCreated),
     reindexingDocsStep,
+    getProgressStep(ReindexStep.indexSettingsRestored),
     getProgressStep(ReindexStep.aliasCreated),
     getProgressStep(ReindexStep.originalIndexDeleted),
   ];

x-pack/plugins/upgrade_assistant/public/application/lib/utils.test.ts

@@ -62,6 +62,10 @@ describe('getReindexProgressLabel', () => {
     expect(getReindexProgressLabel(1, ReindexStep.reindexStarted)).toBe('90%');
   });
 
+  it('returns 92% when index settings have been restored', () => {
+    expect(getReindexProgressLabel(null, ReindexStep.indexSettingsRestored)).toBe('92%');
+  });
+
   it('returns 95% when alias has been created', () => {
     expect(getReindexProgressLabel(null, ReindexStep.aliasCreated)).toBe('95%');
   });
@@ -85,6 +89,12 @@ describe('getReindexProgressLabel', () => {
       );
     });
 
+    it('returns 87% when alias has been created', () => {
+      expect(
+        getReindexProgressLabel(null, ReindexStep.indexSettingsRestored, withExistingAliases)
+      ).toBe('87%');
+    });
+
     it('returns 90% when alias has been created', () => {
       expect(getReindexProgressLabel(null, ReindexStep.aliasCreated, withExistingAliases)).toBe(
         '90%'

x-pack/plugins/upgrade_assistant/public/application/lib/utils.ts

@@ -78,18 +78,23 @@ export const getReindexProgressLabel = (
       percentsComplete = hasExistingAliases ? 85 : 90;
       break;
     }
-    case ReindexStep.aliasCreated: {
+    case ReindexStep.indexSettingsRestored: {
       // step 4 completed
+      percentsComplete = hasExistingAliases ? 87 : 92;
+      break;
+    }
+    case ReindexStep.aliasCreated: {
+      // step 5 completed
       percentsComplete = hasExistingAliases ? 90 : 95;
       break;
     }
     case ReindexStep.originalIndexDeleted: {
-      // step 5 completed
+      // step 6 completed
       percentsComplete = hasExistingAliases ? 95 : 100;
       break;
     }
     case ReindexStep.existingAliasesUpdated: {
-      // step 6 completed, 100% progress
+      // step 7 completed, 100% progress
       percentsComplete = 100;
       break;
     }

x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts

@@ -465,11 +465,11 @@ describe('reindexService', () => {
         expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.newIndexCreated);
         expect(clusterClient.asCurrentUser.indices.create).toHaveBeenCalledWith({
           index: 'myIndex-reindex-0',
-          body: {
-            // index.blocks.write should be removed from the settings for the new index.
-            settings: { 'index.number_of_replicas': 7 },
-            mappings: settingsMappings.mappings,
-          },
+          // index.blocks.write should be removed from the settings for the new index.
+          // index.number_of_replicas and index.refresh_interval are stored to be set at a later stage.
+          // Setting to 0 and -1, respectively, right now.
+          settings: { 'index.number_of_replicas': 0, 'index.refresh_interval': -1 },
+          mappings: settingsMappings.mappings,
         });
       });
 
@@ -509,7 +509,7 @@ describe('reindexService', () => {
         // Original index should have been set back to allow reads.
         expect(clusterClient.asCurrentUser.indices.putSettings).toHaveBeenCalledWith({
           index: 'myIndex',
-          body: { blocks: { write: false } },
+          settings: { blocks: { write: false } },
         });
       });
     });
@@ -539,10 +539,9 @@ describe('reindexService', () => {
         expect(clusterClient.asCurrentUser.reindex).toHaveBeenLastCalledWith({
           refresh: true,
           wait_for_completion: false,
-          body: {
-            source: { index: 'myIndex' },
-            dest: { index: 'myIndex-reindex-0' },
-          },
+          source: { index: 'myIndex' },
+          dest: { index: 'myIndex-reindex-0' },
+          slices: 'auto',
         });
       });
 
@@ -725,6 +724,72 @@ describe('reindexService', () => {
           ...defaultAttributes,
           lastCompletedStep: ReindexStep.reindexCompleted,
           reindexOptions: { openAndClose: false },
+          backupSettings: {},
+        },
+      } as ReindexSavedObject;
+
+      it('restores the settings (both to null), and updates lastCompletedStep', async () => {
+        clusterClient.asCurrentUser.indices.putSettings.mockResponseOnce({ acknowledged: true });
+        const updatedOp = await service.processNextStep(reindexOp);
+        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.indexSettingsRestored);
+        expect(clusterClient.asCurrentUser.indices.putSettings).toHaveBeenCalledWith({
+          index: reindexOp.attributes.newIndexName,
+          settings: {
+            'index.number_of_replicas': null,
+            'index.refresh_interval': null,
+          },
+        });
+      });
+
+      it('restores the original settings, and updates lastCompletedStep', async () => {
+        clusterClient.asCurrentUser.indices.putSettings.mockResponseOnce({ acknowledged: true });
+        const reindexOpWithBackupSettings = {
+          ...reindexOp,
+          attributes: {
+            ...reindexOp.attributes,
+            backupSettings: {
+              'index.number_of_replicas': 7,
+              'index.refresh_interval': 1,
+            },
+          },
+        };
+        const updatedOp = await service.processNextStep(reindexOpWithBackupSettings);
+        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.indexSettingsRestored);
+        expect(clusterClient.asCurrentUser.indices.putSettings).toHaveBeenCalledWith({
+          index: reindexOp.attributes.newIndexName,
+          settings: {
+            'index.number_of_replicas': 7,
+            'index.refresh_interval': 1,
+          },
+        });
+      });
+
+      it('fails if the request is not acknowledged', async () => {
+        clusterClient.asCurrentUser.indices.putSettings.mockResponseOnce({ acknowledged: false });
+        const updatedOp = await service.processNextStep(reindexOp);
+        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted);
+        expect(updatedOp.attributes.status).toEqual(ReindexStatus.failed);
+        expect(updatedOp.attributes.errorMessage).not.toBeNull();
+        expect(log.error).toHaveBeenCalledWith(expect.any(String));
+      });
+
+      it('fails if the request fails', async () => {
+        clusterClient.asCurrentUser.indices.putSettings.mockRejectedValueOnce(new Error('blah!'));
+        const updatedOp = await service.processNextStep(reindexOp);
+        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted);
+        expect(updatedOp.attributes.status).toEqual(ReindexStatus.failed);
+        expect(updatedOp.attributes.errorMessage).not.toBeNull();
+        expect(log.error).toHaveBeenCalledWith(expect.any(String));
+      });
+    });
+
+    describe('indexSettingsRestored', () => {
+      const reindexOp = {
+        id: '1',
+        attributes: {
+          ...defaultAttributes,
+          lastCompletedStep: ReindexStep.indexSettingsRestored,
+          reindexOptions: { openAndClose: false },
         },
       } as ReindexSavedObject;
 
@@ -734,12 +799,10 @@ describe('reindexService', () => {
         const updatedOp = await service.processNextStep(reindexOp);
         expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.aliasCreated);
         expect(clusterClient.asCurrentUser.indices.updateAliases).toHaveBeenCalledWith({
-          body: {
-            actions: [
-              { add: { index: 'myIndex-reindex-0', alias: 'myIndex' } },
-              { remove_index: { index: 'myIndex' } },
-            ],
-          },
+          actions: [
+            { add: { index: 'myIndex-reindex-0', alias: 'myIndex' } },
+            { remove_index: { index: 'myIndex' } },
+          ],
         });
       });
 
@@ -758,27 +821,25 @@ describe('reindexService', () => {
         const updatedOp = await service.processNextStep(reindexOp);
         expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.aliasCreated);
         expect(clusterClient.asCurrentUser.indices.updateAliases).toHaveBeenCalledWith({
-          body: {
-            actions: [
-              { add: { index: 'myIndex-reindex-0', alias: 'myIndex' } },
-              { remove_index: { index: 'myIndex' } },
-              { add: { index: 'myIndex-reindex-0', alias: 'myAlias' } },
-              {
-                add: {
-                  index: 'myIndex-reindex-0',
-                  alias: 'myFilteredAlias',
-                  filter: { term: { https: true } },
-                },
+          actions: [
+            { add: { index: 'myIndex-reindex-0', alias: 'myIndex' } },
+            { remove_index: { index: 'myIndex' } },
+            { add: { index: 'myIndex-reindex-0', alias: 'myAlias' } },
+            {
+              add: {
+                index: 'myIndex-reindex-0',
+                alias: 'myFilteredAlias',
+                filter: { term: { https: true } },
               },
-            ],
-          },
+            },
+          ],
         });
       });
 
       it('fails if switching aliases is not acknowledged', async () => {
         clusterClient.asCurrentUser.indices.updateAliases.mockResponseOnce({ acknowledged: false });
         const updatedOp = await service.processNextStep(reindexOp);
-        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted);
+        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.indexSettingsRestored);
         expect(updatedOp.attributes.status).toEqual(ReindexStatus.failed);
         expect(updatedOp.attributes.errorMessage).not.toBeNull();
         expect(log.error).toHaveBeenCalledWith(expect.any(String));
@@ -787,7 +848,7 @@ describe('reindexService', () => {
       it('fails if switching aliases fails', async () => {
         clusterClient.asCurrentUser.indices.updateAliases.mockRejectedValueOnce(new Error('blah!'));
         const updatedOp = await service.processNextStep(reindexOp);
-        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted);
+        expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.indexSettingsRestored);
         expect(updatedOp.attributes.status).toEqual(ReindexStatus.failed);
         expect(updatedOp.attributes.errorMessage).not.toBeNull();
         expect(log.error).toHaveBeenCalledWith(expect.any(String));