Keep the search engine configured on an index when deploying index definitions

src/ServiceControl.Audit.Persistence.RavenDB/DatabaseSetup.cs

@@ -107,7 +107,7 @@ internal static async Task CreateIndexes(IDocumentStore documentStore, bool enab
             await documentStore.Maintenance.SendAsync(new DeleteIndexOperation(MessagesViewIndexWithFulltextSearchName), cancellationToken);
         }
 
-        await IndexCreation.CreateIndexesAsync(indexList, documentStore, null, null, cancellationToken);
+        await IndexDeployment.CreateIndexesAsync(indexList, documentStore, cancellationToken);
     }
 
     async Task ConfigureExpiration(IDocumentStore documentStore, CancellationToken cancellationToken)

src/ServiceControl.Audit.Persistence.Tests.RavenDB/IndexSetupTests.cs

@@ -5,6 +5,7 @@ namespace ServiceControl.Audit.Persistence.Tests;
 using NUnit.Framework;
 using Persistence.RavenDB;
 using Persistence.RavenDB.Indexes;
+using Raven.Client;
 using Raven.Client.Documents.Indexes;
 using Raven.Client.Documents.Operations.Indexes;
 using Raven.Client.Exceptions;
@@ -37,7 +38,7 @@ public async Task Startup_check_should_not_report_corax_indexes_for_new_database
     [Test]
     public async Task Startup_check_should_report_indexes_using_corax()
     {
-        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { ["Indexing.Static.SearchEngineType"] = SearchEngineType.Corax.ToString() } };
+        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { [IndexDeployment.StaticSearchEngineTypeKey] = SearchEngineType.Corax.ToString() } };
 
         await UpdateIndex(index);
 
@@ -69,59 +70,147 @@ public async Task Free_text_search_index_can_be_opted_out_from()
     }
 
     [Test]
-    public async Task Indexes_should_be_reset_on_setup()
+    public async Task New_indexes_should_be_created_with_lucene()
     {
-        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { ["Indexing.Static.SearchEngineType"] = SearchEngineType.Corax.ToString() } };
+        var index = new FailedAuditImportIndex();
 
-        var indexWithCustomConfigStats = await UpdateIndex(index);
-
-        Assert.That(indexWithCustomConfigStats.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+        await configuration.DocumentStore.Maintenance.SendAsync(new DeleteIndexOperation(index.IndexName), TestTimeoutCancellationToken);
 
         await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken);
 
-        await WaitForIndexDefinitionUpdate(indexWithCustomConfigStats);
+        var definition = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexOperation(index.IndexName), TestTimeoutCancellationToken);
+
+        // The search engine is set on the index, not inherited. It then also applies to databases that default to Corax.
+        Assert.That(definition.Configuration, Does.ContainKey(IndexDeployment.StaticSearchEngineTypeKey).WithValue(SearchEngineType.Lucene.ToString()));
+    }
+
+    [TestCase(true, TestName = "Search engine set in the index definition through SearchEngineType should take precedence over the one on the server")]
+    [TestCase(false, TestName = "Search engine set in the index definition through Configuration should take precedence over the one on the server")]
+    public async Task Search_engine_set_in_the_index_definition_should_take_precedence_over_the_one_on_the_server(bool useSearchEngineTypeProperty)
+    {
+        var statsBefore = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(nameof(FailedAuditImportIndex)), TestTimeoutCancellationToken);
 
-        var indexAfterResetStats = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName));
+        Assert.That(statsBefore.SearchEngineType, Is.EqualTo(SearchEngineType.Lucene));
 
-        Assert.That(indexAfterResetStats.SearchEngineType, Is.EqualTo(SearchEngineType.Lucene));
+        await IndexDeployment.CreateIndexesAsync([new FailedAuditImportIndexPinnedToCorax(useSearchEngineTypeProperty)], configuration.DocumentStore, TestTimeoutCancellationToken);
+
+        var statsAfter = await WaitForIndexDefinitionUpdate(statsBefore);
+
+        Assert.That(statsAfter.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
     }
 
     [Test]
-    public async Task Indexes_should_not_be_reset_on_setup_when_locked_as_ignore()
+    public async Task Search_engine_configured_on_the_index_should_be_preserved_on_setup()
     {
-        var index = new MessagesViewIndexWithFullTextSearch
-        {
-            Configuration = { ["Indexing.Static.SearchEngineType"] = SearchEngineType.Corax.ToString() },
-            LockMode = IndexLockMode.LockedIgnore
-        };
+        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { [IndexDeployment.StaticSearchEngineTypeKey] = SearchEngineType.Corax.ToString() } };
 
         var indexStatsBefore = await UpdateIndex(index);
 
         Assert.That(indexStatsBefore.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
 
         await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken);
 
-        // raven will ignore the update since index was locked, so best we can do is wait a bit and check that settings hasn't changed
-        await Task.Delay(1000);
+        var replacement = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexOperation(Constants.Documents.Indexing.SideBySideIndexNamePrefix + index.IndexName), TestTimeoutCancellationToken);
+        var indexStatsAfter = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName), TestTimeoutCancellationToken);
 
-        var indexStatsAfter = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName));
+        Assert.That(replacement, Is.Null, "Setup should not trigger a rebuild of an index whose only difference is the configured search engine");
+        Assert.That(indexStatsAfter.CreatedTimestamp, Is.EqualTo(indexStatsBefore.CreatedTimestamp));
         Assert.That(indexStatsAfter.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
     }
 
     [Test]
-    public async Task Indexes_should_not_be_reset_on_setup_when_locked_as_error()
+    public async Task Indexes_should_be_reset_on_setup_keeping_the_configured_search_engine()
     {
-        var index = new MessagesViewIndexWithFullTextSearch
+        var customizedStats = await PutCustomizedIndex(SearchEngineType.Corax);
+
+        Assert.That(customizedStats.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+
+        await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken);
+
+        var resetStats = await WaitForIndexDefinitionUpdate(customizedStats);
+        var resetDefinition = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexOperation(customizedStats.Name), TestTimeoutCancellationToken);
+
+        Assert.That(resetDefinition.Fields, Does.Not.ContainKey(CustomizedField), "Customizations made to the index definition should be reset");
+        Assert.That(resetStats.SearchEngineType, Is.EqualTo(SearchEngineType.Corax), "The search engine configured on the index should be kept when the index is rebuilt");
+    }
+
+    [Test]
+    public async Task Pending_replacement_using_the_database_default_should_be_discarded_in_favor_of_the_configured_search_engine()
+    {
+        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { [IndexDeployment.StaticSearchEngineTypeKey] = SearchEngineType.Corax.ToString() } };
+        var replacementName = Constants.Documents.Indexing.SideBySideIndexNamePrefix + index.IndexName;
+
+        var originalStats = await UpdateIndex(index);
+
+        // Stop indexing so that the replacement cannot catch up and swap. A large database under load shows the same behavior.
+        await configuration.DocumentStore.Maintenance.SendAsync(new StopIndexingOperation(), TestTimeoutCancellationToken);
+
+        try
         {
-            Configuration = { ["Indexing.Static.SearchEngineType"] = SearchEngineType.Corax.ToString() },
-            LockMode = IndexLockMode.LockedError
-        };
+            // Versions before the fix deployed the definition without the configured search engine.
+            // This creates a replacement that uses the database default.
+            await IndexCreation.CreateIndexesAsync([new MessagesViewIndexWithFullTextSearch()], configuration.DocumentStore, null, null, TestTimeoutCancellationToken);
 
-        await UpdateIndex(index);
+            var defaultReplacement = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(replacementName), TestTimeoutCancellationToken);
+            Assert.That(defaultReplacement.SearchEngineType, Is.EqualTo(SearchEngineType.Lucene));
+
+            await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken);
+
+            var replacementAfterSetup = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexOperation(replacementName), TestTimeoutCancellationToken);
+            var originalAfterSetup = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName), TestTimeoutCancellationToken);
+
+            Assert.That(replacementAfterSetup, Is.Null, "The definition matches the existing index again, so the pending replacement should be discarded");
+            Assert.That(originalAfterSetup.CreatedTimestamp, Is.EqualTo(originalStats.CreatedTimestamp));
+            Assert.That(originalAfterSetup.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+        }
+        finally
+        {
+            await configuration.DocumentStore.Maintenance.SendAsync(new StartIndexingOperation(), TestTimeoutCancellationToken);
+        }
+    }
+
+    [Test]
+    public async Task Indexes_should_not_be_reset_on_setup_when_locked_as_ignore()
+    {
+        var customizedStats = await PutCustomizedIndex(SearchEngineType.Corax, IndexLockMode.LockedIgnore);
+
+        await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken);
+
+        // RavenDB ignores the update because the index is locked. Wait a moment and then make sure that the definition did not change.
+        await Task.Delay(1000);
+
+        var definitionAfter = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexOperation(customizedStats.Name), TestTimeoutCancellationToken);
+        var indexStatsAfter = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(customizedStats.Name), TestTimeoutCancellationToken);
+
+        Assert.That(definitionAfter.Fields, Does.ContainKey(CustomizedField));
+        Assert.That(indexStatsAfter.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+    }
+
+    [Test]
+    public async Task Indexes_should_not_be_reset_on_setup_when_locked_as_error()
+    {
+        await PutCustomizedIndex(SearchEngineType.Corax, IndexLockMode.LockedError);
 
         Assert.ThrowsAsync<IndexCreationException>(async () => await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken));
     }
 
+    // Simulates an index changed outside ServiceControl, for example in RavenDB Studio. Its definition is different from ours.
+    async Task<IndexStats> PutCustomizedIndex(SearchEngineType searchEngineType, IndexLockMode lockMode = IndexLockMode.Unlock)
+    {
+        var index = new MessagesViewIndexWithFullTextSearch { Conventions = configuration.DocumentStore.Conventions };
+        var definition = index.CreateIndexDefinition();
+        definition.Name = index.IndexName;
+        definition.LockMode = lockMode;
+        definition.Configuration[IndexDeployment.StaticSearchEngineTypeKey] = searchEngineType.ToString();
+        definition.Fields[CustomizedField] = new IndexFieldOptions { Storage = FieldStorage.Yes };
+
+        var statsBefore = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName), TestTimeoutCancellationToken);
+
+        await configuration.DocumentStore.Maintenance.SendAsync(new PutIndexesOperation(definition), TestTimeoutCancellationToken);
+
+        return await WaitForIndexDefinitionUpdate(statsBefore);
+    }
+
     async Task<IndexStats> UpdateIndex(IAbstractIndexCreationTask index)
     {
         var statsBefore = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName), TestTimeoutCancellationToken);
@@ -131,6 +220,25 @@ async Task<IndexStats> UpdateIndex(IAbstractIndexCreationTask index)
         return await WaitForIndexDefinitionUpdate(statsBefore);
     }
 
+    class FailedAuditImportIndexPinnedToCorax : FailedAuditImportIndex
+    {
+        public FailedAuditImportIndexPinnedToCorax(bool useSearchEngineTypeProperty)
+        {
+            if (useSearchEngineTypeProperty)
+            {
+                SearchEngineType = Raven.Client.Documents.Indexes.SearchEngineType.Corax;
+            }
+            else
+            {
+                Configuration[IndexDeployment.StaticSearchEngineTypeKey] = Raven.Client.Documents.Indexes.SearchEngineType.Corax.ToString();
+            }
+        }
+
+        public override string IndexName => nameof(FailedAuditImportIndex);
+    }
+
+    const string CustomizedField = nameof(MessagesViewIndex.SortAndFilterOptions.MessageId);
+
     // How many consecutive RavenExceptions from the stats query below get tolerated before letting one propagate for real.
     // RavenDB can throw a variety of transient errors for that race (seen so far: OperationCanceledException
     // from the read transaction being cancelled, and ObjectDisposedException from the old engine's index persistence being torn down).

src/ServiceControl.Persistence.RavenDB/DatabaseSetup.cs

@@ -4,7 +4,6 @@ namespace ServiceControl.Persistence.RavenDB
     using System.Threading;
     using System.Threading.Tasks;
     using Raven.Client.Documents;
-    using Raven.Client.Documents.Indexes;
     using Raven.Client.Documents.Operations.Expiration;
     using Raven.Client.Exceptions;
     using Raven.Client.ServerWide;
@@ -22,7 +21,7 @@ public async Task Execute(CancellationToken cancellationToken = default)
             await UpdateDatabaseSettings(settings.DatabaseName, cancellationToken);
             await UpdateDatabaseSettings(settings.ThroughputDatabaseName, cancellationToken);
 
-            await IndexCreation.CreateIndexesAsync(typeof(DatabaseSetup).Assembly, documentStore, null, null, cancellationToken);
+            await IndexDeployment.CreateIndexesAsync(typeof(DatabaseSetup).Assembly, documentStore, cancellationToken);
 
             await StartupChecks.WarnIfIndexesUseCorax(documentStore, settings.DatabaseName, cancellationToken);
             await StartupChecks.WarnIfIndexesUseCorax(documentStore, settings.ThroughputDatabaseName, cancellationToken);

src/ServiceControl.Persistence.Tests.RavenDB/IndexSetupTests.cs

@@ -0,0 +1,66 @@
+namespace ServiceControl.Persistence.Tests.RavenDB
+{
+    using System;
+    using System.Threading;
+    using System.Threading.Tasks;
+    using NUnit.Framework;
+    using Raven.Client;
+    using Raven.Client.Documents.Indexes;
+    using Raven.Client.Documents.Operations.Indexes;
+    using ServiceControl.Persistence.RavenDB;
+    using ServiceControl.RavenDB;
+
+    [TestFixture]
+    class IndexSetupTests : RavenPersistenceTestBase
+    {
+        [Test]
+        public async Task Search_engine_configured_on_the_index_should_be_preserved_on_setup()
+        {
+            var index = new CustomChecksIndex { Conventions = DocumentStore.Conventions };
+            var definition = index.CreateIndexDefinition();
+            definition.Name = index.IndexName;
+            definition.Configuration[IndexDeployment.StaticSearchEngineTypeKey] = SearchEngineType.Corax.ToString();
+
+            var statsBefore = await DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName));
+            await DocumentStore.Maintenance.SendAsync(new PutIndexesOperation(definition));
+            var customizedStats = await WaitForIndexDefinitionUpdate(statsBefore);
+
+            try
+            {
+                Assert.That(customizedStats.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+
+                await IndexDeployment.CreateIndexesAsync(typeof(DatabaseSetup).Assembly, DocumentStore);
+
+                var replacement = await DocumentStore.Maintenance.SendAsync(new GetIndexOperation(Constants.Documents.Indexing.SideBySideIndexNamePrefix + index.IndexName));
+                var statsAfter = await DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName));
+
+                Assert.That(replacement, Is.Null, "Setup should not trigger a rebuild of an index whose only difference is the configured search engine");
+                Assert.That(statsAfter.CreatedTimestamp, Is.EqualTo(customizedStats.CreatedTimestamp));
+                Assert.That(statsAfter.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+            }
+            finally
+            {
+                // Tests share the database. Restore the index to the state that setup creates.
+                await IndexCreation.CreateIndexesAsync([new CustomChecksIndex { Configuration = { [IndexDeployment.StaticSearchEngineTypeKey] = SearchEngineType.Lucene.ToString() } }], DocumentStore);
+                await WaitForIndexDefinitionUpdate(customizedStats);
+            }
+        }
+
+        async Task<IndexStats> WaitForIndexDefinitionUpdate(IndexStats oldStats)
+        {
+            using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(30));
+
+            while (true)
+            {
+                var newStats = await DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(oldStats.Name), timeout.Token);
+
+                if (newStats.CreatedTimestamp > oldStats.CreatedTimestamp)
+                {
+                    return newStats;
+                }
+
+                await Task.Delay(100, timeout.Token);
+            }
+        }
+    }
+}