From df1acb84b19ddc80c8e9bccaea087fed2f6eabaf Mon Sep 17 00:00:00 2001 From: Mark Hahnenberg Date: Wed, 29 Mar 2017 12:09:02 -0700 Subject: [PATCH] [client-app] Fix thread reindexing loop Summary: We would mark modified threads for reindexing, the thread search indexer would reindex them, which would trigger a notification that the thread had been modified, so we would mark the thread for reindexing, ... Now we keep track in memory of which threads we've marked for reindexing so we avoid re-marking them when the update notification arrives later. Test Plan: Run locally, verify same threads aren't getting continuously reindexed and that the in-memory set doesn't grow unboundedly Reviewers: spang, juan, evan Reviewed By: juan, evan Differential Revision: https://phab.nylas.com/D4289 --- .../lib/thread-search-index-store.es6 | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/client-app/internal_packages/search-index/lib/thread-search-index-store.es6 b/packages/client-app/internal_packages/search-index/lib/thread-search-index-store.es6 index 0c3dd4b80..a71994d64 100644 --- a/packages/client-app/internal_packages/search-index/lib/thread-search-index-store.es6 +++ b/packages/client-app/internal_packages/search-index/lib/thread-search-index-store.es6 @@ -16,6 +16,7 @@ class ThreadSearchIndexStore { constructor() { this.unsubscribers = [] this.indexer = SearchIndexScheduler; + this.threadsWaitingToBeIndexed = new Set(); } activate() { @@ -128,16 +129,23 @@ class ThreadSearchIndexStore { let promises = [] if (type === 'persist') { - const alreadyIndexedThreads = threads.filter(t => t.isSearchIndexed) - if (alreadyIndexedThreads.length > 0) { - alreadyIndexedThreads.forEach(thread => { + const threadsToIndex = _.uniq(threads.filter(t => !this.threadsWaitingToBeIndexed.has(t.id)), false /* isSorted */, t => t.id); + const threadsIndexed = threads.filter(t => t.isSearchIndexed && this.threadsWaitingToBeIndexed.has(t.id)); + + for (const thread of threadsIndexed) { + this.threadsWaitingToBeIndexed.delete(thread.id); + } + + if (threadsToIndex.length > 0) { + threadsToIndex.forEach(thread => { // Mark already indexed threads as unindexed so that we re-index them // with updates - thread.isSearchIndexed = false + thread.isSearchIndexed = false; + this.threadsWaitingToBeIndexed.add(thread.id); }) - await DatabaseStore.inTransaction(t => t.persistModels(alreadyIndexedThreads)) + await DatabaseStore.inTransaction(t => t.persistModels(threadsToIndex)); + this.indexer.notifyHasIndexingToDo(); } - this.indexer.notifyHasIndexingToDo(); } else if (type === 'unpersist') { promises = threads.map(thread => this.unindexThread(thread, {isBeingUnpersisted: true}))