refactor(elasticsearch): support retrying failed deletion of stale indices

Signed-off-by: Phil Kuang <pkuang@factset.com>

.changeset/seven-deers-rule.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-search-backend-module-elasticsearch': patch
+---
+
+Prevent orphaned stale indices by permanently marking them for deletion so removal can be re-attempted if it failed previously

plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts

@@ -73,11 +73,19 @@ describe('ElasticSearchSearchEngineIndexer', () => {
         'routing.search': '-',
         is_write_index: '-',
       },
+      {
+        alias: 'some-type-index__search_removable',
+        index: 'some-type-index__456tobedeleted',
+        filter: '-',
+        'routing.index': '-',
+        'routing.search': '-',
+        is_write_index: '-',
+      },
     ]);
     mock.add(
       {
         method: 'GET',
-        path: '/_cat/aliases/some-type-index__search',
+        path: '/_cat/aliases/some-type-index__search%2Csome-type-index__search_removable',
       },
       catSpy,
     );
@@ -108,7 +116,7 @@ describe('ElasticSearchSearchEngineIndexer', () => {
     mock.add(
       {
         method: 'DELETE',
-        path: '/some-type-index__123tobedeleted',
+        path: '/some-type-index__123tobedeleted%2Csome-type-index__456tobedeleted',
       },
       deleteSpy,
     );
@@ -151,6 +159,15 @@ describe('ElasticSearchSearchEngineIndexer', () => {
       remove: { index: 'some-type-index__*', alias: 'some-type-index__search' },
     });
     expect(aliasActions[1]).toStrictEqual({
+      add: {
+        indices: [
+          'some-type-index__123tobedeleted',
+          'some-type-index__456tobedeleted',
+        ],
+        alias: 'some-type-index__search_removable',
+      },
+    });
+    expect(aliasActions[2]).toStrictEqual({
       add: { index: createdIndex, alias: 'some-type-index__search' },
     });
 
@@ -193,12 +210,12 @@ describe('ElasticSearchSearchEngineIndexer', () => {
     catSpy = jest.fn().mockReturnValue([]);
     mock.clear({
       method: 'GET',
-      path: '/_cat/aliases/some-type-index__search',
+      path: '/_cat/aliases/some-type-index__search%2Csome-type-index__search_removable',
     });
     mock.add(
       {
         method: 'GET',
-        path: '/_cat/aliases/some-type-index__search',
+        path: '/_cat/aliases/some-type-index__search%2Csome-type-index__search_removable',
       },
       catSpy,
     );

plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.ts

@@ -46,6 +46,7 @@ export class ElasticSearchSearchEngineIndexer extends BatchSearchEngineIndexer {
   private readonly indexPrefix: string;
   private readonly indexSeparator: string;
   private readonly alias: string;
+  private readonly removableAlias: string;
   private readonly logger: Logger;
   private readonly sourceStream: Readable;
   private readonly elasticSearchClient: Client;
@@ -60,6 +61,7 @@ export class ElasticSearchSearchEngineIndexer extends BatchSearchEngineIndexer {
     this.indexSeparator = options.indexSeparator;
     this.indexName = this.constructIndexName(`${Date.now()}`);
     this.alias = options.alias;
+    this.removableAlias = `${this.alias}_removable`;
     this.elasticSearchClient = options.elasticSearchClient;
 
     // The ES client bulk helper supports stream-based indexing, but we have to
@@ -90,12 +92,12 @@ export class ElasticSearchSearchEngineIndexer extends BatchSearchEngineIndexer {
 
     const aliases = await this.elasticSearchClient.cat.aliases({
       format: 'json',
-      name: this.alias,
+      name: [this.alias, this.removableAlias],
     });
 
-    this.removableIndices = aliases.body.map(
-      (r: Record<string, any>) => r.index,
-    );
+    this.removableIndices = [
+      ...new Set(aliases.body.map((r: Record<string, any>) => r.index)),
+    ] as string[];
 
     await this.elasticSearchClient.indices.create({
       index: this.indexName,
@@ -121,8 +123,9 @@ export class ElasticSearchSearchEngineIndexer extends BatchSearchEngineIndexer {
     // Wait for the bulk helper to finish processing.
     const result = await this.bulkResult;
 
-    // Rotate aliases upon completion. Allow errors to bubble up so that we can
-    // clean up the create index.
+    // Rotate main alias upon completion. Apply permanent secondary alias so
+    // stale indices can be referenced for deletion in case initial attempt
+    // fails. Allow errors to bubble up so that we can clean up the created index.
     this.logger.info(
       `Indexing completed for index ${this.type} in ${duration(
         this.startTimestamp,
@@ -135,8 +138,18 @@ export class ElasticSearchSearchEngineIndexer extends BatchSearchEngineIndexer {
           {
             remove: { index: this.constructIndexName('*'), alias: this.alias },
           },
-          { add: { index: this.indexName, alias: this.alias } },
-        ],
+          this.removableIndices.length
+            ? {
+                add: {
+                  indices: this.removableIndices,
+                  alias: this.removableAlias,
+                },
+              }
+            : undefined,
+          {
+            add: { index: this.indexName, alias: this.alias },
+          },
+        ].filter(Boolean),
       },
     });