From 35af08d9a03afea0584a951e9d4395ff462315e0 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 29 Nov 2022 15:14:30 +0100 Subject: [PATCH] Update elastic engine to preserve pre-existing indices if indexer receives 0 documents Signed-off-by: Eric Peterson --- .../src/engines/ElasticSearchSearchEngine.ts | 11 ++++++---- .../ElasticSearchSearchEngineIndexer.test.ts | 20 +++++++++++++++++++ .../ElasticSearchSearchEngineIndexer.ts | 18 +++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts index f3db46956d..f17ec1b01d 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngine.ts @@ -252,6 +252,7 @@ export class ElasticSearchSearchEngine implements SearchEngine { async getIndexer(type: string) { const alias = this.constructSearchAlias(type); + const indexerLogger = this.logger.child({ documentType: type }); const indexer = new ElasticSearchSearchEngineIndexer({ type, @@ -259,13 +260,13 @@ export class ElasticSearchSearchEngine implements SearchEngine { indexSeparator: this.indexSeparator, alias, elasticSearchClientWrapper: this.elasticSearchClientWrapper, - logger: this.logger, + logger: indexerLogger, batchSize: this.batchSize, }); // Attempt cleanup upon failure. indexer.on('error', async e => { - this.logger.error(`Failed to index documents for type ${type}`, e); + indexerLogger.error(`Failed to index documents for type ${type}`, e); let cleanupError: Error | undefined; // In some cases, a failure may have occurred before the indexer was able @@ -296,11 +297,13 @@ export class ElasticSearchSearchEngine implements SearchEngine { }); if (cleanupError) { - this.logger.error( + indexerLogger.error( `Unable to clean up elastic index ${indexer.indexName}: ${cleanupError}`, ); } else { - this.logger.info(`Removed partial, failed index ${indexer.indexName}`); + indexerLogger.info( + `Removed partial, failed index ${indexer.indexName}`, + ); } }); diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts index 155a6bbf32..1e9e98dbaf 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts @@ -176,6 +176,26 @@ describe('ElasticSearchSearchEngineIndexer', () => { expect(deleteSpy).toHaveBeenCalled(); }); + it('handles when no documents are received', async () => { + await TestPipeline.fromIndexer(indexer).withDocuments([]).execute(); + + // Older indices should have been queried for. + expect(catSpy).toHaveBeenCalled(); + + // A new index should have been created. + const createdIndex = createSpy.mock.calls[0][0].path.slice(1); + expect(createdIndex).toContain('some-type-index__'); + + // No documents should have been sent + expect(bulkSpy).not.toHaveBeenCalled(); + + // Alias should not have been rotated. + expect(aliasesSpy).not.toHaveBeenCalled(); + + // Old index should not be cleaned up. + expect(deleteSpy).not.toHaveBeenCalled(); + }); + it('handles bulk and batching during indexing', async () => { const documents = range(550).map(i => ({ title: `Hello World ${i}`, diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.ts index e32c49559d..1b1d47bb9b 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.ts @@ -131,6 +131,24 @@ export class ElasticSearchSearchEngineIndexer extends BatchSearchEngineIndexer { // Wait for the bulk helper to finish processing. const result = await this.bulkResult; + // Warn that no documents were indexed, early return so that alias swapping + // does not occur, and clean up the empty index we just created. + if (this.processed === 0) { + this.logger.warn( + `Index for ${this.type} was not ${ + this.removableIndices.length ? 'replaced' : 'created' + }: indexer received 0 documents`, + ); + try { + await this.elasticSearchClientWrapper.deleteIndex({ + index: this.indexName, + }); + } catch (error) { + this.logger.error(`Unable to clean up elastic index: ${error}`); + } + return; + } + // 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.