Be persistent in attempting to clean up indices on error

Signed-off-by: Eric Peterson <ericpeterson@spotify.com>

.changeset/search-lieutenant-dangle.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-search-backend-module-elasticsearch': patch
+---
+
+Fixed a bug that prevented indices from being cleaned up under some circumstances, which could have led to shard exhaustion.

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

@@ -68,6 +68,14 @@ const customIndexTemplate = {
   },
 };
 
+const advanceTimersByNTimes = async (n = 1, time = 1000) => {
+  for (let i = 0; i < n; i++) {
+    await Promise.resolve();
+    jest.advanceTimersByTime(time);
+    await Promise.resolve();
+  }
+};
+
 describe('ElasticSearchSearchEngine', () => {
   let testSearchEngine: ElasticSearchSearchEngine;
   let inspectableSearchEngine: ElasticSearchSearchEngineForTranslatorTests;
@@ -855,35 +863,33 @@ describe('ElasticSearchSearchEngine', () => {
       });
 
       it('should check for and delete expected index', async () => {
-        const existsSpy = jest.fn().mockReturnValue('truthy value');
         const deleteSpy = jest.fn().mockReturnValue({});
-        mock.add({ method: 'HEAD', path: '/expected-index-name' }, existsSpy);
         mock.add({ method: 'DELETE', path: '/expected-index-name' }, deleteSpy);
 
         await errorHandler(error);
 
         // Check and delete HTTP requests were made.
-        expect(existsSpy).toHaveBeenCalled();
         expect(deleteSpy).toHaveBeenCalled();
       });
 
-      it('should not delete index if none exists', async () => {
-        // Exists call returns 404 on no index.
-        const existsSpy = jest.fn().mockReturnValue(
+      it('should retry delete index up to 5 times', async () => {
+        // Delete call returns 404
+        const deleteSpy = jest.fn().mockReturnValue(
           new errors.ResponseError({
             statusCode: 404,
             body: { status: 404 },
           } as unknown as any),
         );
-        const deleteSpy = jest.fn().mockReturnValue({});
-        mock.add({ method: 'HEAD', path: '/expected-index-name' }, existsSpy);
         mock.add({ method: 'DELETE', path: '/expected-index-name' }, deleteSpy);
 
-        await errorHandler(error);
+        // Call the error handler and advance timers
+        jest.useFakeTimers();
+        errorHandler(error);
+        await advanceTimersByNTimes(10);
+        jest.useRealTimers();
 
-        // Check request was made, but no delete request was made.
-        expect(existsSpy).toHaveBeenCalled();
-        expect(deleteSpy).not.toHaveBeenCalled();
+        // Check request was made 5 times
+        expect(deleteSpy).toHaveBeenCalledTimes(5);
       });
     });
   });

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

@@ -266,19 +266,41 @@ export class ElasticSearchSearchEngine implements SearchEngine {
     // Attempt cleanup upon failure.
     indexer.on('error', async e => {
       this.logger.error(`Failed to index documents for type ${type}`, e);
-      try {
-        const response = await this.elasticSearchClientWrapper.indexExists({
-          index: indexer.indexName,
-        });
-        const indexCreated = response.body;
-        if (indexCreated) {
-          this.logger.info(`Removing created index ${indexer.indexName}`);
-          await this.elasticSearchClientWrapper.deleteIndex({
-            index: indexer.indexName,
-          });
+      let cleanupError: Error | undefined;
+
+      // In some cases, a failure may have occurred before the indexer was able
+      // to complete initialization. Try up to 5 times to remove the dangling
+      // index.
+      await new Promise<void>(async done => {
+        const maxAttempts = 5;
+        let attempts = 0;
+
+        while (attempts < maxAttempts) {
+          try {
+            await this.elasticSearchClientWrapper.deleteIndex({
+              index: indexer.indexName,
+            });
+
+            attempts = maxAttempts;
+            cleanupError = undefined;
+            done();
+          } catch (err) {
+            cleanupError = err;
+          }
+
+          // Wait 1 second between retries.
+          await new Promise(okay => setTimeout(okay, 1000));
+
+          attempts++;
         }
-      } catch (error) {
-        this.logger.error(`Unable to clean up elastic index: ${error}`);
+      });
+
+      if (cleanupError) {
+        this.logger.error(
+          `Unable to clean up elastic index ${indexer.indexName}: ${cleanupError}`,
+        );
+      } else {
+        this.logger.info(`Removed partial, failed index ${indexer.indexName}`);
       }
     });