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,117 @@ 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 Search_engine_configured_on_the_index_should_be_preserved_on_setup()
     {
-        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { ["Indexing.Static.SearchEngineType"] = SearchEngineType.Corax.ToString() } };
+        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { [IndexDeployment.StaticSearchEngineTypeKey] = SearchEngineType.Corax.ToString() } };
 
-        var indexWithCustomConfigStats = await UpdateIndex(index);
+        var indexStatsBefore = await UpdateIndex(index);
 
-        Assert.That(indexWithCustomConfigStats.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+        Assert.That(indexStatsBefore.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
 
         await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken);
 
-        await WaitForIndexDefinitionUpdate(indexWithCustomConfigStats);
+        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);
+
+        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_be_reset_on_setup_keeping_the_configured_search_engine()
+    {
+        var customizedStats = await PutCustomizedIndex(SearchEngineType.Corax);
+
+        Assert.That(customizedStats.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+
+        await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken);
 
-        var indexAfterResetStats = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName));
+        var resetStats = await WaitForIndexDefinitionUpdate(customizedStats);
+        var resetDefinition = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexOperation(customizedStats.Name), TestTimeoutCancellationToken);
 
-        Assert.That(indexAfterResetStats.SearchEngineType, Is.EqualTo(SearchEngineType.Lucene));
+        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 Indexes_should_not_be_reset_on_setup_when_locked_as_ignore()
+    public async Task Pending_replacement_using_the_database_default_should_be_discarded_in_favor_of_the_configured_search_engine()
     {
-        var index = new MessagesViewIndexWithFullTextSearch
+        var index = new MessagesViewIndexWithFullTextSearch { Configuration = { [IndexDeployment.StaticSearchEngineTypeKey] = SearchEngineType.Corax.ToString() } };
+        var replacementName = Constants.Documents.Indexing.SideBySideIndexNamePrefix + index.IndexName;
+
+        var originalStats = await UpdateIndex(index);
+
+        // Keep replacements from catching up and swapping, like a large database under load would
+        await configuration.DocumentStore.Maintenance.SendAsync(new StopIndexingOperation(), TestTimeoutCancellationToken);
+
+        try
         {
-            Configuration = { ["Indexing.Static.SearchEngineType"] = SearchEngineType.Corax.ToString() },
-            LockMode = IndexLockMode.LockedIgnore
-        };
+            // What versions before the fix did: deploy the definition without the configured search engine,
+            // creating a replacement that uses the database default
+            await IndexCreation.CreateIndexesAsync([new MessagesViewIndexWithFullTextSearch()], configuration.DocumentStore, null, null, TestTimeoutCancellationToken);
 
-        var indexStatsBefore = await UpdateIndex(index);
+            var defaultReplacement = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(replacementName), TestTimeoutCancellationToken);
+            Assert.That(defaultReplacement.SearchEngineType, Is.EqualTo(SearchEngineType.Lucene));
 
-        Assert.That(indexStatsBefore.SearchEngineType, Is.EqualTo(SearchEngineType.Corax));
+            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);
 
-        // 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
+        // raven will ignore the update since index was locked, so best we can do is wait a bit and check that the definition hasn't changed
         await Task.Delay(1000);
 
-        var indexStatsAfter = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName));
+        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()
     {
-        var index = new MessagesViewIndexWithFullTextSearch
-        {
-            Configuration = { ["Indexing.Static.SearchEngineType"] = SearchEngineType.Corax.ToString() },
-            LockMode = IndexLockMode.LockedError
-        };
-
-        await UpdateIndex(index);
+        await PutCustomizedIndex(SearchEngineType.Corax, IndexLockMode.LockedError);
 
         Assert.ThrowsAsync<IndexCreationException>(async () => await DatabaseSetup.CreateIndexes(configuration.DocumentStore, true, TestTimeoutCancellationToken));
     }
 
+    // Simulates an index modified outside ServiceControl, e.g. through RavenDB Studio, with a definition that differs 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 +190,8 @@ async Task<IndexStats> UpdateIndex(IAbstractIndexCreationTask index)
         return await WaitForIndexDefinitionUpdate(statsBefore);
     }
 
+    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
+            {
+                // The database is shared across tests, restore the index to the database default
+                await IndexCreation.CreateIndexesAsync([new CustomChecksIndex()], 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);
+            }
+        }
+    }
+}

src/ServiceControl.RavenDB/IndexDeployment.cs

@@ -0,0 +1,57 @@
+namespace ServiceControl.RavenDB
+{
+    using System.Reflection;
+    using System.Threading;
+    using Raven.Client;
+    using Raven.Client.Documents;
+    using Raven.Client.Documents.Indexes;
+    using Raven.Client.Documents.Operations.Indexes;
+
+    public static class IndexDeployment
+    {
+        public static Task CreateIndexesAsync(Assembly assembly, IDocumentStore store, CancellationToken cancellationToken = default)
+        {
+            var indexes = assembly.GetTypes()
+                .Where(t => t.IsClass && !t.IsAbstract && t.IsSubclassOf(typeof(AbstractIndexCreationTask)))
+                .Select(t => (AbstractIndexCreationTask)Activator.CreateInstance(t));
+
+            return CreateIndexesAsync(indexes, store, cancellationToken);
+        }
+
+        public static async Task CreateIndexesAsync(IEnumerable<AbstractIndexCreationTask> indexes, IDocumentStore store, CancellationToken cancellationToken = default)
+        {
+            var indexList = indexes.ToList();
+
+            // Operators can switch individual indexes from Corax to Lucene in RavenDB Studio, which stores the search engine
+            // in the index configuration. Our definitions don't carry it, so deploying them as-is would make RavenDB build a
+            // side-by-side replacement that falls back to the database default, which is Corax for databases created before
+            // Lucene became the default. Carry the existing choice over so the definitions match and nothing gets rebuilt.
+            var existingDefinitions = await store.Maintenance.SendAsync(new GetIndexesOperation(0, int.MaxValue), cancellationToken);
+            var existingByName = existingDefinitions.ToDictionary(d => d.Name, StringComparer.OrdinalIgnoreCase);
+
+            foreach (var index in indexList)
+            {
+                // A pending replacement reflects the latest change made by the operator, so it takes precedence
+                if (TryGetSearchEngineType(existingByName, Constants.Documents.Indexing.SideBySideIndexNamePrefix + index.IndexName, out var searchEngineType)
+                    || TryGetSearchEngineType(existingByName, index.IndexName, out searchEngineType))
+                {
+                    index.Configuration[StaticSearchEngineTypeKey] = searchEngineType;
+                }
+            }
+
+            await IndexCreation.CreateIndexesAsync(indexList, store, null, null, cancellationToken);
+        }
+
+        static bool TryGetSearchEngineType(Dictionary<string, IndexDefinition> definitions, string indexName, out string searchEngineType)
+        {
+            searchEngineType = null;
+
+            return definitions.TryGetValue(indexName, out var definition)
+                && definition.Configuration != null
+                && definition.Configuration.TryGetValue(StaticSearchEngineTypeKey, out searchEngineType)
+                && !string.IsNullOrEmpty(searchEngineType);
+        }
+
+        public const string StaticSearchEngineTypeKey = "Indexing.Static.SearchEngineType";
+    }
+}