Add default schemas that are unique

docs/README.md

@@ -32,6 +32,7 @@ This section points to sources that explain why ServiceControl is designed the w
 - [Handling unavailable runtime dependencies](handling-unavailable-runtime-dependencies.md) — how instances react when a dependency is unavailable
 - [Telemetry](telemetry.md) — how to configure telemetry export and which metrics instances emit
 - [Throughput collection](throughput-collection.md) — how usage data is collected and why the throughput queue has a fixed name
+- [SQL persister schemas](sql-persister-schemas.md) — which schema each SQL persister uses by default, and that sharing one between a primary and an audit instance is not recommended
 - [Usage report contents](usage-report.md): every key the usage report carries, every setting it leaves out, and why
 
 ## Decisions and rationale

docs/sql-persister-schemas.md

@@ -0,0 +1,16 @@
+# SQL persister schemas
+
+The SQL Server and PostgreSQL persisters keep all of an instance's tables in one schema, set by `Database/Schema`. A blank or absent setting selects a default:
+
+| Instance | Default schema |
+|---|---|
+| Primary (error) | `sc_primary` |
+| Audit | `sc_audit` |
+
+* ServiceControl will attempt to create the configured schema if it does not already exist. If using a connection without this permission it should be created in advance with the appropriate rights granted to the connection user.
+* A primary and an audit instance can share one schema, but sharing is not recommended.
+* The primary instance's usage report lists `Storage.Schema` as `Default` for `sc_primary` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md).
+
+ServiceControl does not rename or move existing schemas or tables. An instance that already holds data must set `Database/Schema` to the schema that contains it, such as `dbo` on SQL Server, `public` on PostgreSQL, or a custom schema. Otherwise, the instance uses the default schema instead of the schema containing its existing data.
+
+The primary instance records applied migrations in the `__ErrorMigrationsHistory` table of its schema, as the audit instance does with `__AuditMigrationsHistory`.

docs/usage-report.md

@@ -88,7 +88,7 @@ Releases up to 6.21.2 report `SqlVersion` as the full text of `@@VERSION` on SQL
 | `Storage.FullTextSearch` | `Enabled`, `Disabled` | `EnableFullTextSearchOnBodies`, RavenDB only. Always `Enabled` on SQL Server and PostgreSQL, which do not read the setting | 6.20.0 |
 | `Storage.BodyStorage.Type` | `RavenAttachments`, `FileSystem`, `AzureBlob`, `S3` | the persister | 6.20.0 |
 | `Limits.MaxBodySizeToStore` | bytes | `MaxBodySizeToStore`, SQL Server and PostgreSQL only | 6.20.0 |
-| `Storage.Schema` | `Default`, `Custom` | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased |
+| `Storage.Schema` | `Default` when the schema is `sc_primary`, `Custom` for other configured schemas | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased |
 | `Storage.LogLevel` | `None`, `Information`, `Operations` | `RavenDBLogLevel`, RavenDB only | Unreleased |
 | `Storage.CommandTimeoutSeconds` | `Default` or number | `Database/CommandTimeout`, SQL Server and PostgreSQL only | Unreleased |
 | `Storage.QueryTimeoutSeconds` | `Default` or number | `QueryTimeoutInSeconds` | Unreleased |
@@ -237,7 +237,7 @@ The ingestion and health keys follow the rules in [Error ingestion health](#erro
 | `Audit.SameMachine` | `True`, `False`, `Mixed`, `NotApplicable`, `Unknown` | the machine name hashes. `NotApplicable` when either instance runs in a container | Unreleased |
 | `Audit.DatabaseSharing` | `SameSchema`, `SameDatabase`, `SameServer`, `SeparateServer`, `NotApplicable`, `Unknown` | the storage identity hashes. `NotApplicable` when the storage engines differ. Several instances report the most shared class | Unreleased |
 
-`Audit.Storage.SizeGB` covers the whole schema. When the primary and an audit instance share a schema, both report the same size, and `Audit.DatabaseSharing` is `SameSchema`.
+`Audit.Storage.SizeGB` covers the whole schema. A primary and an audit instance on SQL Server or PostgreSQL can share a schema, which is not recommended and is not the default; see [SQL persister schemas](sql-persister-schemas.md).
 
 #### From the configuration response
 

src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest

@@ -11,7 +11,8 @@
     },
     {
       "Name": "ServiceControl.Audit/Database/Schema",
-      "Mandatory": false
+      "Mandatory": false,
+      "DefaultValue": "sc_audit"
     },
     {
       "Name": "ServiceControl.Audit/Database/CommandTimeout",

src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest

@@ -11,7 +11,8 @@
     },
     {
       "Name": "ServiceControl.Audit/Database/Schema",
-      "Mandatory": false
+      "Mandatory": false,
+      "DefaultValue": "sc_audit"
     },
     {
       "Name": "ServiceControl.Audit/Database/CommandTimeout",

src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs

@@ -41,11 +41,11 @@ public static EFPersisterSettings CreateSettings(PersistenceSettings settings)
         };
     }
 
-    static string? ReadSchema(IDictionary<string, string> specific)
+    static string ReadSchema(IDictionary<string, string> specific)
     {
         if (!specific.TryGetValue(SchemaKey, out var schema) || string.IsNullOrWhiteSpace(schema))
         {
-            return null;
+            return EFPersisterSettings.DefaultSchema;
         }
 
         try

src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs

@@ -7,6 +7,7 @@ public class EFPersisterSettings
     public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40);
 
     public const int DefaultCommandTimeout = 30;
+    public const string DefaultSchema = "sc_audit";
 
     public required string ConnectionString { get; init; }
 

src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs

@@ -7,7 +7,6 @@ namespace ServiceControl.Audit.Persistence.EFCore.DbContexts;
 
 public abstract class AuditDbContext(DbContextOptions options) : DbContext(options)
 {
-    // Not EF Core's default, so an audit instance can share a schema with a primary instance.
     public const string MigrationsHistoryTableName = "__AuditMigrationsHistory";
 
     public string? Schema { get; } = options.FindExtension<SchemaOptionsExtension>()?.Schema;

src/ServiceControl.Audit.Persistence.Tests.PostgreSql/PostgreSqlEnvironmentDataTests.cs

@@ -14,6 +14,7 @@ namespace ServiceControl.Audit.Persistence.Tests
     using ServiceControl.Audit.Persistence.EFCore.DbContexts;
     using ServiceControl.Audit.Persistence.EFCore.Entities;
     using ServiceControl.Audit.Persistence.EFCore.Infrastructure;
+    using ServiceControl.Audit.Persistence.EFCore.PostgreSql;
 
     class PostgreSqlEnvironmentDataTests : EFPersistenceTestFixture
     {
@@ -63,14 +64,15 @@ public async Task Counts_messages_across_the_partitions_once_analysed()
         [Test]
         public async Task Measures_and_identifies_the_search_path_schema_when_none_is_configured()
         {
-            var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000);
-            settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] =
-                new NpgsqlConnectionStringBuilder(configuration.ConnectionString) { SearchPath = configuration.Schema }.ConnectionString;
-
             var services = new ServiceCollection();
             services.AddLogging();
             services.AddSingleton<TimeProvider>(configuration.TimeProvider);
-            configuration.CreateConfiguration().Create(settings).AddPersistence(services);
+            new PostgreSqlPersistence(new EFPersisterSettings
+            {
+                ConnectionString = new NpgsqlConnectionStringBuilder(configuration.ConnectionString) { SearchPath = configuration.Schema }.ConnectionString,
+                AuditRetentionPeriod = TimeSpan.FromDays(1),
+                MaxBodySizeToStore = 100000
+            }).AddPersistence(services);
             await using var unconfigured = services.BuildServiceProvider();
 
             var identity = await unconfigured.GetRequiredService<IStorageIdentityProvider>().GetIdentity(TestTimeoutCancellationToken);

src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs

@@ -0,0 +1,43 @@
+namespace ServiceControl.Audit.Persistence.Tests
+{
+    using System;
+    using NUnit.Framework;
+    using ServiceControl.Audit.Persistence.EFCore.Abstractions;
+
+    [TestFixture]
+    class SchemaConfigurationTests
+    {
+        [TestCase(null)]
+        [TestCase("")]
+        [TestCase("   ")]
+        public void Absent_or_blank_schema_falls_back_to_sc_audit(string configured)
+        {
+            var settings = CreateSettings();
+            if (configured != null)
+            {
+                settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = configured;
+            }
+
+            Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("sc_audit"));
+        }
+
+        [TestCase("legacy")]
+        [TestCase("dbo")]
+        [TestCase("public")]
+        [TestCase("psw_audit")]
+        public void Configured_schema_is_used(string schema)
+        {
+            var settings = CreateSettings();
+            settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = schema;
+
+            Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo(schema));
+        }
+
+        static PersistenceSettings CreateSettings()
+        {
+            var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000);
+            settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = "Server=nowhere";
+            return settings;
+        }
+    }
+}

src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs

@@ -6,6 +6,7 @@ namespace ServiceControl.Audit.Persistence.Tests
     using Microsoft.Extensions.Hosting;
     using NUnit.Framework;
     using ServiceControl.Audit.Persistence.EFCore.Abstractions;
+    using ServiceControl.Persistence.Tests;
 
     [TestFixture]
     class SchemaMustExistTests
@@ -14,19 +15,26 @@ class SchemaMustExistTests
         public async Task Setup_fails_when_the_configured_schema_does_not_exist()
         {
             var testsConfiguration = new PersistenceTestsConfiguration();
+            var connectionString = await testsConfiguration.GetConnectionString();
             var schema = $"sc_absent_{Guid.NewGuid():n}";
 
+            var exception = Assert.ThrowsAsync<InvalidOperationException>(() => Setup(testsConfiguration, connectionString, schema));
+
+            Assert.That(exception.Message, Does.Contain(schema).And.Contain("does not exist"));
+        }
+
+        static async Task Setup(PersistenceTestsConfiguration testsConfiguration, string connectionString, string schema)
+        {
             var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000);
-            settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = await testsConfiguration.GetConnectionString();
+            settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = connectionString;
             settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = schema;
 
             var hostBuilder = Host.CreateApplicationBuilder();
             testsConfiguration.CreateConfiguration().Create(settings).AddInstaller(hostBuilder.Services);
             using var host = hostBuilder.Build();
 
-            var exception = Assert.ThrowsAsync<InvalidOperationException>(() => host.StartAsync());
-
-            Assert.That(exception.Message, Does.Contain(schema).And.Contain("does not exist"));
+            await host.StartAsync();
+            await host.StopAsync();
         }
     }
 }

src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs

@@ -50,13 +50,7 @@ void ConfigureDbContext(IServiceCollection services)
             {
                 npgsqlOptions.CommandTimeout(settings.CommandTimeout);
 
-                if (settings.Schema is not null)
-                {
-                    // Its own history table per schema, so schemas sharing a database migrate
-                    // independently. Without this EF leaves the history table unqualified and every
-                    // schema reads the same one.
-                    npgsqlOptions.MigrationsHistoryTable(HistoryRepository.DefaultTableName, settings.Schema);
-                }
+                npgsqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName, settings.Schema);
 
                 if (settings.EnableRetryOnFailure)
                 {
@@ -73,9 +67,7 @@ void ConfigureDbContext(IServiceCollection services)
                 options.ReplaceService<IMigrationsSqlGenerator, SchemaStampingNpgsqlMigrationsSqlGenerator>();
                 options.ReplaceService<IModelCacheKeyFactory, SchemaModelCacheKeyFactory>();
 
-                // HasDefaultSchema moves every table, so the model is meant to differ from the
-                // snapshot the migrations were scaffolded against. A default installation keeps the
-                // check, where a difference really would mean a migration is missing.
+                // Schema stamping intentionally differs from the schema-less snapshot; PendingModelChangesTests checks migration drift.
                 options.ConfigureWarnings(warnings => warnings.Ignore(RelationalEventId.PendingModelChangesWarning));
             }
 

src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContextFactory.cs

@@ -2,6 +2,7 @@ namespace ServiceControl.Persistence.EFCore.PostgreSql;
 
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Design;
+using ServiceControl.Persistence.EFCore.DbContexts;
 
 /// <summary>
 /// Design-time factory used by the dotnet-ef tooling (e.g. dotnet ef migrations add).
@@ -14,7 +15,7 @@ public PostgreSqlServiceControlDbContext CreateDbContext(string[] args)
             ?? "Host=localhost;Port=5432;Database=servicecontrol;Username=postgres;Password=postgres";
 
         var optionsBuilder = new DbContextOptionsBuilder<PostgreSqlServiceControlDbContext>();
-        optionsBuilder.UseNpgsql(connectionString);
+        optionsBuilder.UseNpgsql(connectionString, npgsqlOptions => npgsqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName));
 
         return new PostgreSqlServiceControlDbContext(optionsBuilder.Options);
     }

src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest

@@ -9,6 +9,11 @@
       "Name": "ServiceControl/Database/ConnectionString",
       "Mandatory": true
     },
+    {
+      "Name": "ServiceControl/Database/Schema",
+      "Mandatory": false,
+      "DefaultValue": "sc_primary"
+    },
     {
       "Name": "ServiceControl/Database/CommandTimeout",
       "Mandatory": false

src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs

@@ -50,13 +50,7 @@ void ConfigureDbContext(IServiceCollection services)
             {
                 sqlOptions.CommandTimeout(settings.CommandTimeout);
 
-                if (settings.Schema is not null)
-                {
-                    // Its own history table per schema, so schemas sharing a database migrate
-                    // independently. Without this EF leaves the history table unqualified and every
-                    // schema reads the same one.
-                    sqlOptions.MigrationsHistoryTable(HistoryRepository.DefaultTableName, settings.Schema);
-                }
+                sqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName, settings.Schema);
 
                 if (settings.EnableRetryOnFailure)
                 {
@@ -73,9 +67,7 @@ void ConfigureDbContext(IServiceCollection services)
                 options.ReplaceService<IMigrationsSqlGenerator, SchemaStampingSqlServerMigrationsSqlGenerator>();
                 options.ReplaceService<IModelCacheKeyFactory, SchemaModelCacheKeyFactory>();
 
-                // HasDefaultSchema moves every table, so the model is meant to differ from the
-                // snapshot the migrations were scaffolded against. A default installation keeps the
-                // check, where a difference really would mean a migration is missing.
+                // Schema stamping intentionally differs from the schema-less snapshot; PendingModelChangesTests checks migration drift.
                 options.ConfigureWarnings(warnings => warnings.Ignore(RelationalEventId.PendingModelChangesWarning));
             }
 

src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContextFactory.cs

@@ -2,6 +2,7 @@ namespace ServiceControl.Persistence.EFCore.SqlServer;
 
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Design;
+using ServiceControl.Persistence.EFCore.DbContexts;
 
 /// <summary>
 /// Design-time factory used by the dotnet-ef tooling (e.g. dotnet ef migrations add).
@@ -14,7 +15,7 @@ public SqlServerServiceControlDbContext CreateDbContext(string[] args)
             ?? "Server=localhost;Database=ServiceControl;Trusted_Connection=True;TrustServerCertificate=True";
 
         var optionsBuilder = new DbContextOptionsBuilder<SqlServerServiceControlDbContext>();
-        optionsBuilder.UseSqlServer(connectionString);
+        optionsBuilder.UseSqlServer(connectionString, sqlOptions => sqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName));
 
         return new SqlServerServiceControlDbContext(optionsBuilder.Options);
     }

src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest

@@ -9,6 +9,11 @@
       "Name": "ServiceControl/Database/ConnectionString",
       "Mandatory": true
     },
+    {
+      "Name": "ServiceControl/Database/Schema",
+      "Mandatory": false,
+      "DefaultValue": "sc_primary"
+    },
     {
       "Name": "ServiceControl/Database/CommandTimeout",
       "Mandatory": false

src/ServiceControl.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs

@@ -52,13 +52,13 @@ public PersistenceSettings CreateSettings(SettingsRootNamespace settingsRootName
 
     protected abstract EFPersisterSettings CreateSettings(string connectionString, BodyStorageSettings bodyStorage);
 
-    static string? ReadSchema(SettingsRootNamespace settingsRootNamespace)
+    static string ReadSchema(SettingsRootNamespace settingsRootNamespace)
     {
         var schema = SettingsReader.Read<string>(settingsRootNamespace, SchemaKey);
 
         if (string.IsNullOrWhiteSpace(schema))
         {
-            return null;
+            return EFPersisterSettings.DefaultSchema;
         }
 
         try

src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs

@@ -5,6 +5,7 @@ public abstract class EFPersisterSettings : PersistenceSettings
     public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40);
 
     public const int DefaultCommandTimeout = 30;
+    public const string DefaultSchema = "sc_primary";
     public const int DefaultExternalIntegrationsDispatchingBatchSize = 100;
     public static readonly TimeSpan DefaultEventsRetentionPeriod = TimeSpan.FromDays(14);
 
@@ -13,7 +14,7 @@ public abstract class EFPersisterSettings : PersistenceSettings
     public required string ConnectionString { get; set; }
 
     /// <summary>
-    /// The schema the persister owns, or null to use the provider default. Validated on assignment
+    /// The schema the persister owns, or null to use the provider default. Configuration falls back to <see cref="DefaultSchema"/>. Validated on assignment
     /// because it reaches raw DDL and dialect SQL by interpolation.
     /// </summary>
     public string? Schema

src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs

@@ -7,6 +7,8 @@ namespace ServiceControl.Persistence.EFCore.DbContexts;
 
 public abstract class ServiceControlDbContext(DbContextOptions options) : DbContext(options)
 {
+    public const string MigrationsHistoryTableName = "__ErrorMigrationsHistory";
+
     /// <summary>
     /// The configured schema, or null when the provider default is in use. Null keeps the model
     /// identical to the one the migrations were scaffolded against.

src/ServiceControl.Persistence.EFCore/Implementation/EFEnvironmentDataProvider.cs

@@ -43,7 +43,7 @@ public IEnumerable<EnvironmentDatum> GetData()
             Value("Storage.FullTextSearch", () => settings.EnableFullTextSearchOnBodies ? "Enabled" : "Disabled"),
             Value("Storage.BodyStorage.Type", () => BodyStorageType(settings.BodyStorage)),
             Value("Limits.MaxBodySizeToStore", () => settings.BodyStorage.MaxBodySizeToStore.ToString(CultureInfo.InvariantCulture)),
-            Value("Storage.Schema", () => settings.Schema is null ? "Default" : "Custom"),
+            Value("Storage.Schema", () => settings.Schema is null or EFPersisterSettings.DefaultSchema ? "Default" : "Custom"),
             Value("Storage.CommandTimeoutSeconds", () => WhenConfigured(EFPersistenceConfigurationBase.CommandTimeoutKey, () => Number(settings.CommandTimeout))),
             Value("Storage.QueryTimeoutSeconds", () => WhenConfigured(QueryTimeLimit.SettingName, () => Seconds(settings.QueryTimeout))),
             Value("Storage.SubscriptionCacheSeconds", () => WhenConfigured(EFPersistenceConfigurationBase.SubscriptionCacheDurationKey, () => Seconds(settings.SubscriptionCacheDuration))),