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..034a759c7e 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,31 +70,39 @@ 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); @@ -101,27 +110,107 @@ public async Task Indexes_should_not_be_reset_on_setup_when_locked_as_ignore() 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(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 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 +220,25 @@ async Task 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). 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..df6d49a441 --- /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 + { + // 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 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..38188f075c --- /dev/null +++ b/src/ServiceControl.RavenDB/IndexDeployment.cs @@ -0,0 +1,93 @@ +namespace ServiceControl.RavenDB +{ + using System.Reflection; + using System.Threading; + using Microsoft.Extensions.Logging; + using Raven.Client; + using Raven.Client.Documents; + using Raven.Client.Documents.Indexes; + using Raven.Client.Documents.Operations.Indexes; + using ServiceControl.Infrastructure; + + 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(); + + // Our index definitions do not set a search engine. RavenDB then uses the database default for them, + // which, starting in 6.20, ServiceControl sets to Lucene when creating new databases; existing + // databases are left untouched. + // An operator can set a different search engine on one index in RavenDB Studio, for example Lucene instead of + // Corax, as the migration guide recommends. RavenDB stores that choice in the configuration of that index only. + // RavenDB compares the configuration as part of the index definition. At the next start-up, our definition has + // no search engine and does not match the definition on the server. RavenDB then builds a side-by-side + // replacement with the database default. For databases created before Lucene became the default, that default + // is Corax. This undoes the migration and starts a full rebuild of the index. On a large database, the rebuild + // can take days. To prevent this, the search engine of each index is resolved before deployment: + 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) + { + // 1. A search engine set in our index definition always wins. It can be set through SearchEngineType or Configuration. + if (index.SearchEngineType.HasValue + || (index.Configuration.TryGetValue(StaticSearchEngineTypeKey, out var configuredSearchEngineType) && !string.IsNullOrEmpty(configuredSearchEngineType))) + { + continue; + } + + var replacementName = Constants.Documents.Indexing.SideBySideIndexNamePrefix + index.IndexName; + + // 2. Keep the search engine set on the index on the server. The definitions then match and RavenDB does not + // rebuild the index. A pending replacement is checked first. When the operator changes the search engine, + // the replacement holds the latest choice. ServiceControl 6.20.0 and 6.21.0 also created replacements when + // they reset a migrated index at start-up. Those replacements have no search engine. The search engine of + // the original index is then used. The definition matches the original index again and RavenDB discards + // the replacement. + if (TryGetSearchEngineType(existingByName, replacementName, out var searchEngineType) + || TryGetSearchEngineType(existingByName, index.IndexName, out searchEngineType)) + { + index.Configuration[StaticSearchEngineTypeKey] = searchEngineType; + Logger.LogInformation("Index {IndexName} keeps the configured {SearchEngineType} search engine", index.IndexName, searchEngineType); + } + // 3. An index that does not exist yet is created with Lucene. Lucene performs better for our workload, also + // in databases that still default to Corax. The search engine is set on the index itself. The index then + // stays on Lucene when the database default changes. + else if (!existingByName.ContainsKey(index.IndexName) && !existingByName.ContainsKey(replacementName)) + { + index.Configuration[StaticSearchEngineTypeKey] = nameof(SearchEngineType.Lucene); + Logger.LogInformation("Index {IndexName} is created with the Lucene search engine", index.IndexName); + } + + // 4. An existing index without a search engine of its own continues to use the database default. A search + // engine set now changes the definition and starts the full rebuild that this code prevents. + } + + 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"; + + static readonly ILogger Logger = LoggerUtil.CreateStaticLogger(typeof(IndexDeployment)); + } +}