From 789f2d94a668d9e34cc366935e36714eed3fabfd Mon Sep 17 00:00:00 2001 From: Mauro Servienti Date: Fri, 2 Oct 2026 15:47:40 +0200 Subject: [PATCH] Keep the search engine configured on an index when deploying index definitions Operators migrating indexes from Corax to Lucene in RavenDB Studio store the search engine in the index configuration. ServiceControl deployed its index definitions without it, so RavenDB built a side-by-side replacement that fell back to the database default, which is Corax for databases created before 6.20. Unless the index was locked, every start-up reset migrated indexes back to Corax. Index definitions are now deployed carrying over the search engine configured on the existing index (or on its pending replacement). A migrated index no longer needs to be locked, still receives definition changes, and a pending Corax replacement created by earlier versions is discarded at start-up. Co-Authored-By: Claude Opus 5.5 --- .../DatabaseSetup.cs | 2 +- .../IndexSetupTests.cs | 109 ++++++++++++++---- .../DatabaseSetup.cs | 3 +- .../IndexSetupTests.cs | 66 +++++++++++ src/ServiceControl.RavenDB/IndexDeployment.cs | 57 +++++++++ 5 files changed, 210 insertions(+), 27 deletions(-) create mode 100644 src/ServiceControl.Persistence.Tests.RavenDB/IndexSetupTests.cs create mode 100644 src/ServiceControl.RavenDB/IndexDeployment.cs diff --git a/src/ServiceControl.Audit.Persistence.RavenDB/DatabaseSetup.cs b/src/ServiceControl.Audit.Persistence.RavenDB/DatabaseSetup.cs index 2560fc128e..f937b0c2fa 100644 --- a/src/ServiceControl.Audit.Persistence.RavenDB/DatabaseSetup.cs +++ b/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) diff --git a/src/ServiceControl.Audit.Persistence.Tests.RavenDB/IndexSetupTests.cs b/src/ServiceControl.Audit.Persistence.Tests.RavenDB/IndexSetupTests.cs index 3df471801d..824d71ce0e 100644 --- a/src/ServiceControl.Audit.Persistence.Tests.RavenDB/IndexSetupTests.cs +++ b/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(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 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 UpdateIndex(IAbstractIndexCreationTask index) { var statsBefore = await configuration.DocumentStore.Maintenance.SendAsync(new GetIndexStatisticsOperation(index.IndexName), TestTimeoutCancellationToken); @@ -131,6 +190,8 @@ async Task 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). diff --git a/src/ServiceControl.Persistence.RavenDB/DatabaseSetup.cs b/src/ServiceControl.Persistence.RavenDB/DatabaseSetup.cs index 73c6812e09..7fe37cdb8e 100644 --- a/src/ServiceControl.Persistence.RavenDB/DatabaseSetup.cs +++ b/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); diff --git a/src/ServiceControl.Persistence.Tests.RavenDB/IndexSetupTests.cs b/src/ServiceControl.Persistence.Tests.RavenDB/IndexSetupTests.cs new file mode 100644 index 0000000000..eea504a28f --- /dev/null +++ b/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 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); + } + } + } +} diff --git a/src/ServiceControl.RavenDB/IndexDeployment.cs b/src/ServiceControl.RavenDB/IndexDeployment.cs new file mode 100644 index 0000000000..573feff258 --- /dev/null +++ b/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 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 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"; + } +}