Implement Known Endpoints persistence with EF Core insert-only tables by johnsimons · Pull Request #5625 · Particular/ServiceControl

johnsimons · 2026-07-20T23:20:44Z

Introduce KnownEndpointEntity and KnownEndpointInsertOnlyEntity tables for efficient, eventual-consistent storage of discovered endpoints. A background reconciler processes new endpoint entries from the insert-only table into the main table, handling duplicates and maintaining existing 'Monitored' state.

This completes the EF Core integration for the IMonitoringDataStore.

Updates test contexts for database teardown and draining of insert-only tables.

johnsimons · 2026-07-20T23:56:39Z

+    {
+        var sql = """
+            WITH lock_check AS (
+                SELECT pg_try_advisory_xact_lock(hashtext('known_endpoints_sync')) AS acquired


We need locks so that if the instance is scalled out for some reason, the code still works correctly

johnsimons · 2026-07-20T23:58:38Z

+        // Call base first to apply entity configurations
+        base.OnModelCreating(modelBuilder);
+
+        // Apply snake_case naming convention for PostgreSQL


Using snake_case is the official de facto standard for PostgreSQL database objects because the system automatically folds all unquoted SQL identifiers to lowercase

Is there a particular reason for not using EFCore.NamingConventions to do this?

Good to know, I have modified it to use it

johnsimons · 2026-07-20T23:59:51Z

-  <ItemGroup>
-    <!-- Private=false & ExcludeAssets=runtime prevent repeatedly including binary dependencies of ServiceControl.Persistence and its dependencies in each persister directory -->
+  <ItemGroup Condition="'$(Configuration)' == 'Release'">
+    <!-- Private=false & ExcludeAssets=runtime prevent repeatedly including binary dependencies of ServiceControl.Persistence


This is important otherwise dotnet ef migrations does not work, It can't find dependencies

johnsimons · 2026-07-21T00:00:20Z

+    {
+        var sql = """
+            DECLARE @lockResult INT;
+            EXEC @lockResult = sp_getapplock @Resource = 'known_endpoints_sync', @LockMode = 'Exclusive', @LockOwner = 'Transaction', @LockTimeout = 0;


Same as PostgreSQL lock but this time for SqlServer

johnsimons · 2026-07-21T00:02:04Z

+
+public class KnownEndpointEntity
+{
+    public Guid Id { get; set; }


I think is ok to use Guid here, given that we are setting it explicitly

johnsimons · 2026-07-21T00:02:42Z

+
+public class KnownEndpointInsertOnlyEntity
+{
+    public long Id { get; set; }


This one is the insert only table, so I want things to go fast

johnsimons · 2026-07-21T00:20:09Z

        using var scope = host.Services.CreateScope();
        var db = scope.ServiceProvider.GetRequiredService<PostgreSqlServiceControlDbContext>();
-        await db.Database.EnsureCreatedAsync();
+        await db.Database.MigrateAsync();


We are using migrations now, this ensures that side is also correct.

rbev · 2026-07-21T03:46:20Z

+        // Call base first to apply entity configurations
+        base.OnModelCreating(modelBuilder);
+
+        // Apply snake_case naming convention for PostgreSQL


Is there a particular reason for not using EFCore.NamingConventions to do this?

rbev · 2026-07-21T03:50:08Z

+                Host NVARCHAR(MAX)
+            );
+
+            DELETE TOP (@batchSize) FROM KnownEndpointsInsertOnly


Can you avoid the temp table by just putting the Delete/Output into the CTE directly?

Data-modifying CTEs are a PostgreSQL feature. SQL Server doesn't allow DELETE/INSERT/UPDATE inside a WITH clause at all.

rbev · 2026-07-21T03:51:56Z

@@ -9,19 +8,19 @@ namespace ServiceControl.Persistence.EFCore.Implementation.UnitOfWork;
 public class EFIngestionUnitOfWork : IIngestionUnitOfWork


Should this and the other persistence interface implementations be internal?

we can fine tune that later, but the TL;DR; is that it doesn't really matter, this is not an API

rbev · 2026-07-21T03:55:10Z

+            }
+            catch (DbUpdateException)
+            {
+                // A concurrent insert with the same deterministic id may have won the race


should this explicitly check for the key violation error type?
should it also log a debug logline?

I have modified this to confirm that is a key violation by revalidating the data.

Introduce `KnownEndpointEntity` and `KnownEndpointInsertOnlyEntity` tables for efficient, eventual-consistent storage of discovered endpoints. A background reconciler processes new endpoint entries from the insert-only table into the main table, handling duplicates and maintaining existing 'Monitored' state. This completes the EF Core integration for the `IMonitoringDataStore`. Updates test contexts for database teardown and draining of insert-only tables.

Refactor `InsertOnlyTableReconciler` to be non-generic, simplifying its declaration. Integrate `TimeProvider` for precise control over delays during testing. Add a `ReconcileNow` method to immediately trigger reconciliation cycles, bypassing the background service's periodic timers. This enables tests to drain insert-only tables without waiting for real time. Update persistence test contexts to leverage `FakeTimeProvider` and the new `ReconcileNow` method, reducing test setup complexity and coupling to specific reconciler implementations.

Ensures that all properties of `MessageBodyFileResult` are explicitly initialized upon object creation, preventing nullability issues and improving the integrity of message body data.

Co-authored-by: Warwick Schroeder <warwick.schroeder@gmail.com>

johnsimons self-assigned this Jul 20, 2026

johnsimons requested review from abparticular, rbev and warwickschroeder July 20, 2026 23:54

johnsimons force-pushed the john/ingestion_part2 branch 2 times, most recently from e45b529 to 58b2c1f Compare July 21, 2026 01:38

johnsimons commented Jul 21, 2026

View reviewed changes

johnsimons marked this pull request as ready for review July 21, 2026 01:57

johnsimons force-pushed the john/ingestion_part2 branch from 42dc604 to 885d4d0 Compare July 21, 2026 03:39

rbev approved these changes Jul 21, 2026

View reviewed changes

johnsimons force-pushed the john/ingestion_part2 branch from 885d4d0 to 3362d69 Compare July 21, 2026 04:34

johnsimons added 6 commits July 21, 2026 15:22

Make CompleteDatabaseOperation asynchronous across persistence tests

94a272f

No longer applies

bb06d02

Make MessageBodyFileResult properties required

598e783

Ensures that all properties of `MessageBodyFileResult` are explicitly initialized upon object creation, preventing nullability issues and improving the integrity of message body data.

Small updates based on feedback

510ed9c

johnsimons force-pushed the john/ingestion_part2 branch from 3362d69 to 510ed9c Compare July 21, 2026 05:22

warwickschroeder reviewed Jul 21, 2026

View reviewed changes

Apply suggestions from code review

ffadf36

Co-authored-by: Warwick Schroeder <warwick.schroeder@gmail.com>

johnsimons enabled auto-merge July 21, 2026 21:24

johnsimons merged commit 2f66be6 into master Jul 21, 2026
33 checks passed

johnsimons deleted the john/ingestion_part2 branch July 21, 2026 21:39

		@@ -9,19 +8,19 @@ namespace ServiceControl.Persistence.EFCore.Implementation.UnitOfWork;
		public class EFIngestionUnitOfWork : IIngestionUnitOfWork

Uh oh!

Conversation

johnsimons commented Jul 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants