perf(site-runtime): instance reconcile sweep reads metadata only and fetches config_json just for the instances it repairs (review 2026-10-21 01/SR-P1)

Adds SiteStorageService.GetAllDeployedConfigMetadataAsync. The sweep decides from the projection, then fetches single rows for its starts/restarts off- thread and re-validates them (generation guard, revision match, pipeline/ running guards), releasing the per-revision claim of any repair that does not act. Boot load unchanged.
2026-10-22 06:42:06 -04:00
parent c782744b07
commit 960c35060c
6 changed files with 415 additions and 42 deletions
@@ -0,0 +1,136 @@
+using System.Text.Json;
+using Akka.Actor;
+using Akka.TestKit.Xunit2;
+using Microsoft.Extensions.Logging.Abstractions;
+using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening;
+using ZB.MOM.WW.ScadaBridge.SiteRuntime.Actors;
+using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence;
+using ZB.MOM.WW.ScadaBridge.SiteRuntime.Scripts;
+using ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.TestSupport;
+using ZB.MOM.WW.ScadaBridge.TestSupport;
+
+namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Actors;
+
+/// <summary>
+/// Review 2026-10-21 01/SR-P1: the instance reconcile sweep compared only name / enabled /
+/// revision, yet materialised every row's <c>config_json</c> on every tick — tens of MB of LOH
+/// strings per minute on a healthy large site. It now reads a metadata-only projection and
+/// fetches a full row only for an instance it actually starts or restarts.
+/// </summary>
+public class InstanceSweepMetadataReadTests : TestKit, IDisposable
+{
+    private readonly SiteStorageService _storage;
+    private readonly ScriptCompilationService _compilationService;
+    private readonly SharedScriptLibrary _sharedScriptLibrary;
+    private readonly TestLocalDb _localDb;
+
+    private int _fullTableReads;
+    private int _singleRowReads;
+
+    /// <summary>Creates a temporary LocalDb-backed site store per test.</summary>
+    public InstanceSweepMetadataReadTests()
+    {
+        _localDb = TestLocalDb.CreateTemp("sweep-metadata-read");
+        _storage = new SiteStorageService(_localDb.Db, NullLogger<SiteStorageService>.Instance);
+        _storage.InitializeAsync().GetAwaiter().GetResult();
+        _compilationService = new ScriptCompilationService(NullLogger<ScriptCompilationService>.Instance);
+        _sharedScriptLibrary = new SharedScriptLibrary(
+            _compilationService, NullLogger<SharedScriptLibrary>.Instance);
+    }
+
+    /// <inheritdoc />
+    void IDisposable.Dispose()
+    {
+        Shutdown();
+        var path = _localDb.Path;
+        _localDb.Dispose();
+        TestLocalDb.DeleteFiles(path);
+    }
+
+    private static string ConfigJson(string name) => JsonSerializer.Serialize(new FlattenedConfiguration
+    {
+        InstanceUniqueName = name,
+        Attributes = [new ResolvedAttribute { CanonicalName = "A", Value = "1", DataType = "Int32" }]
+    });
+
+    private async Task<List<DeployedInstance>> CountingFullLoad()
+    {
+        Interlocked.Increment(ref _fullTableReads);
+        return await _storage.GetAllDeployedConfigsAsync();
+    }
+
+    private async Task<DeployedInstance?> CountingSingleRowLoad(string name)
+    {
+        Interlocked.Increment(ref _singleRowReads);
+        return await _storage.GetDeployedConfigAsync(name);
+    }
+
+    private IActorRef CreateBootedDeploymentManager()
+    {
+        var health = new DeploymentManagerStartupProbe();
+        Func<Task<List<DeployedInstance>>> fullLoad = CountingFullLoad;
+        Func<string, Task<DeployedInstance?>> rowLoad = CountingSingleRowLoad;
+        var dm = ActorOf(Props.Create(() => new DeploymentManagerActor(
+            _storage,
+            _compilationService,
+            _sharedScriptLibrary,
+            null,
+            new SiteRuntimeOptions { InstanceSweepIntervalSeconds = 1, StartupBatchDelayMs = 10 },
+            NullLogger<DeploymentManagerActor>.Instance,
+            null,
+            health,
+            null,
+            null,
+            null,
+            null,
+            fullLoad,
+            null,
+            null,
+            null,
+            rowLoad,
+            null)));
+        AwaitCondition(() => health.ReportCount > 0, TimeSpan.FromSeconds(20), TimeSpan.FromMilliseconds(25));
+        return dm;
+    }
+
+    private async Task<bool> IsRunningAsync(IActorRef dm, string name)
+    {
+        var snapshot = await dm.Ask<DebugViewSnapshot>(
+            new DebugSnapshotRequest(name, Guid.NewGuid().ToString("N")), TimeSpan.FromSeconds(5));
+        return !snapshot.InstanceNotFound;
+    }
+
+    /// <summary>A healthy site's sweep never reads a config blob: one full read (boot), zero row reads.</summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task HealthySweep_ReadsNoConfigJson()
+    {
+        await _storage.StoreDeployedConfigAsync("P1", ConfigJson("P1"), "d1", "r1", true);
+        await _storage.StoreDeployedConfigAsync("P2", ConfigJson("P2"), "d2", "r2", true);
+        var dm = CreateBootedDeploymentManager();
+        await AwaitAssertAsync(async () => Assert.True(await IsRunningAsync(dm, "P2")), TimeSpan.FromSeconds(10));
+
+        await Task.Delay(TimeSpan.FromSeconds(4)); // several sweep ticks
+
+        Assert.Equal(1, Volatile.Read(ref _fullTableReads));
+        Assert.Equal(0, Volatile.Read(ref _singleRowReads));
+    }
+
+    /// <summary>
+    /// A repair still works: a row that appears after boot (as CDC replication would deliver it)
+    /// is started, its config fetched by a single-row read.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task SweepStart_FetchesOnlyTheRowItStarts()
+    {
+        var dm = CreateBootedDeploymentManager();
+        await _storage.StoreDeployedConfigAsync("Late1", ConfigJson("Late1"), "d1", "r1", true);
+
+        await AwaitAssertAsync(async () => Assert.True(await IsRunningAsync(dm, "Late1")), TimeSpan.FromSeconds(15));
+
+        Assert.Equal(1, Volatile.Read(ref _fullTableReads));
+        Assert.Equal(1, Volatile.Read(ref _singleRowReads));
+    }
+}
@@ -532,22 +532,16 @@ public class InstanceSweepSelfHealTests : TestKit, IDisposable
    /// done since makes it stale. Connection definitions are passed as <see langword="null"/>
    /// (= "the definitions read failed, keep the overlay") so these tests exercise the CONFIG
    /// half of the snapshot in isolation.
+    /// <para>
+    /// Metadata only (review 2026-10-21 01/SR-P1): the sweep fetches the config body of an
+    /// instance it repairs from the store, so a test expecting a repair must ALSO have the row
+    /// (at the same revision) in storage.
+    /// </para>
    /// </summary>
    private static DeploymentManagerActor.InstanceSweepLoaded SweepSnapshot(
-        string instanceName, string revision, string configJson, long generationAtRead,
-        bool isEnabled = true) =>
+        string instanceName, string revision, long generationAtRead, bool isEnabled = true) =>
        new(
-            [
-                new DeployedInstance
-                {
-                    InstanceUniqueName = instanceName,
-                    ConfigJson = configJson,
-                    DeploymentId = $"dep-{revision}",
-                    RevisionHash = revision,
-                    IsEnabled = isEnabled,
-                    DeployedAt = DateTimeOffset.UtcNow.ToString("O")
-                }
-            ],
+            [new DeployedConfigMetadata(instanceName, $"dep-{revision}", revision, isEnabled)],
            null,
            ConnectionDefinitions: null,
            StoreGenerationAtRead: generationAtRead);
@@ -585,9 +579,7 @@ public class InstanceSweepSelfHealTests : TestKit, IDisposable
        var afterDeploys = await ResolveInstanceChildAsync(dm, instanceName);

        // Generation 0 = captured before either deploy mutated the store.
-        dm.Tell(SweepSnapshot(
-            instanceName, "rev-1", MakeConfigJson(instanceName, attributeValue: "111"),
-            generationAtRead: 0));
+        dm.Tell(SweepSnapshot(instanceName, "rev-1", generationAtRead: 0));

        await Task.Delay(SeveralSweeps);

@@ -625,9 +617,13 @@ public class InstanceSweepSelfHealTests : TestKit, IDisposable
        await AwaitAssertAsync(async () =>
            Assert.True(await InstanceIsRunningAsync(dm, instanceName)), TimeSpan.FromSeconds(20));

-        dm.Tell(SweepSnapshot(
-            instanceName, "rev-2", MakeConfigJson(instanceName, attributeValue: "222"),
-            generationAtRead: 0));
+        // The store is refreshed out of band (as replication would), so the sweep's single-row
+        // fetch finds the rev-2 body the metadata snapshot names. A direct storage write does not
+        // pass through the actor, so the generation stays 0.
+        await _storage.StoreDeployedConfigAsync(
+            instanceName, MakeConfigJson(instanceName, attributeValue: "222"),
+            "dep-rev-2", "rev-2", true);
+        dm.Tell(SweepSnapshot(instanceName, "rev-2", generationAtRead: 0));

        await AwaitAssertAsync(async () =>
        {
@@ -671,8 +667,7 @@ public class InstanceSweepSelfHealTests : TestKit, IDisposable
            Assert.False(await InstanceIsRunningAsync(dm, instanceName)), TimeSpan.FromSeconds(15));

        // Generation 0 = captured before the deploy and the delete mutated the store.
-        dm.Tell(SweepSnapshot(
-            instanceName, "rev-1", MakeConfigJson(instanceName), generationAtRead: 0));
+        dm.Tell(SweepSnapshot(instanceName, "rev-1", generationAtRead: 0));

        await Task.Delay(SeveralSweeps);

@@ -693,8 +688,9 @@ public class InstanceSweepSelfHealTests : TestKit, IDisposable
        var siteLog = new FakeSiteEventLogger();
        var dm = CreateBootedDeploymentManager(siteLog, sweepSeconds: 3600);

-        dm.Tell(SweepSnapshot(
-            instanceName, "rev-1", MakeConfigJson(instanceName), generationAtRead: 0));
+        await _storage.StoreDeployedConfigAsync(
+            instanceName, MakeConfigJson(instanceName), "dep-rev-1", "rev-1", true);
+        dm.Tell(SweepSnapshot(instanceName, "rev-1", generationAtRead: 0));

        await AwaitAssertAsync(async () =>
            Assert.True(await InstanceIsRunningAsync(dm, instanceName)), TimeSpan.FromSeconds(20));
@@ -147,6 +147,29 @@ public class SiteStorageServiceTests : IAsyncLifetime, IDisposable
        Assert.Null(result);
    }

+    /// <summary>
+    /// Review 2026-10-21 01/SR-P1: the all-rows metadata projection the instance sweep reads
+    /// carries the same identity as the full read, for every row, without the config blob.
+    /// </summary>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    [Fact]
+    public async Task GetAllDeployedConfigMetadataAsync_MatchesTheFullReadForEveryRow()
+    {
+        await _storage.StoreDeployedConfigAsync(
+            "Pump1", "{\"test\":true}", "dep-001", "sha256:abc", isEnabled: true);
+        await _storage.StoreDeployedConfigAsync(
+            "Pump2", "{\"other\":1}", "dep-002", "sha256:def", isEnabled: false);
+
+        var metadata = (await _storage.GetAllDeployedConfigMetadataAsync())
+            .OrderBy(m => m.InstanceUniqueName).ToList();
+        var full = (await _storage.GetAllDeployedConfigsAsync())
+            .OrderBy(c => c.InstanceUniqueName).ToList();
+
+        Assert.Equal(
+            full.Select(c => (c.InstanceUniqueName, c.DeploymentId, c.RevisionHash, c.IsEnabled)),
+            metadata.Select(m => (m.InstanceUniqueName, m.DeploymentId, m.RevisionHash, m.IsEnabled)));
+    }
+
    /// <summary>Verifies that a metadata read returns the same identity as reading all configs and picking the match.</summary>
    /// <returns>A task that represents the asynchronous operation.</returns>
    [Fact]