diff --git a/.changeset/search-lieutenant-dangle.md b/.changeset/search-lieutenant-dangle.md new file mode 100644 index 0000000000..75f1bca915 --- /dev/null +++ b/.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. diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts index 3a237664a8..602a158b98 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.test.ts +++ b/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); }); }); }); diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts index 72bab5b3b6..f3db46956d 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts +++ b/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(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}`); } });