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"; + } +}