From 7aacc490432d22ee18e3168050d4e230866e29c8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:48:28 -0400 Subject: [PATCH] fix(deployment): detach each instance's settled rows from the bulk deploy's shared unit of work after Finalize (review 2026-10-21 04/M3) --- .../Component-DeploymentManager.md | 6 +++- .../IDeploymentManagerRepository.cs | 13 +++++++ .../DeploymentManagerRepository.cs | 21 ++++++++++++ .../DeploymentService.cs | 7 ++++ .../PendingDeploymentRepositoryTests.cs | 34 +++++++++++++++++++ .../DeploySiteAsyncTests.cs | 20 +++++++++++ 6 files changed, 100 insertions(+), 1 deletion(-) diff --git a/docs/requirements/Component-DeploymentManager.md b/docs/requirements/Component-DeploymentManager.md index ececfb32..17173f37 100644 --- a/docs/requirements/Component-DeploymentManager.md +++ b/docs/requirements/Component-DeploymentManager.md @@ -167,7 +167,11 @@ one is parallel: does not reintroduce serialization. Shape mirrors `ArtifactDeploymentService.DeployCoreAsync`. 3. **Finalize (serial).** Commit terminal statuses, apply post-success side - effects, write audit rows, release each operation lock. + effects, write audit rows, release each operation lock. Each finalised + instance's settled rows (staged `PendingDeployment`, snapshot, deployment + record) are then detached from the batch's shared unit of work, so the batch + does not hold N configs' JSON in the change tracker — or re-scan them on every + later save — until it ends (review 2026-10-21 04/M3). **Disabled instances are skipped.** A bulk site deploy never touches an instance in `Disabled` — no flatten, no send, no state change — because deploy is a legal diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs index 91e6637f..24ef39bc 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs @@ -466,4 +466,17 @@ public interface IDeploymentManagerRepository /// /// The number of tracked entries detached (0 when nothing was pending). int DetachPendingChanges(); + + /// + /// Stops tracking the SETTLED deployment rows of one instance — its + /// , and + /// entries in the Unchanged state. A bulk site deploy + /// shares one scoped unit of work across every instance; without this, each finalised + /// instance's config JSON (staged copy, old and new snapshot) stays tracked for the rest of + /// the batch and every later SaveChanges re-scans it. Entries with pending writes are + /// left alone. + /// + /// The instance whose settled rows to stop tracking. + /// The number of entries detached. + int DetachSettledDeploymentRows(int instanceId); } diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs index 8c7f8ce0..255f2245 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs @@ -844,6 +844,27 @@ public class DeploymentManagerRepository : IDeploymentManagerRepository return pending.Count; } + /// + public int DetachSettledDeploymentRows(int instanceId) + { + // Enumerated into a list first: changing State mutates the tracker. + var settled = _dbContext.ChangeTracker.Entries() + .Where(e => e.State == EntityState.Unchanged + && e.Entity switch + { + PendingDeployment p => p.InstanceId == instanceId, + DeployedConfigSnapshot s => s.InstanceId == instanceId, + DeploymentRecord r => r.InstanceId == instanceId, + _ => false + }) + .ToList(); + + foreach (var entry in settled) + entry.State = EntityState.Detached; + + return settled.Count; + } + /// /// Flat shape of the deployment-record ⋈ instance join used to compose the list /// query. A named type rather than an anonymous one so diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs index 9c217a7d..12141524 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/DeploymentService.cs @@ -1051,6 +1051,13 @@ public class DeploymentService { step.LockHandle?.Dispose(); } + + // This instance is done. The batch shares ONE scoped unit of work, so + // without this its staged copy and old/new snapshot JSON would stay + // tracked (and re-scanned by every later SaveChanges) until the whole + // batch ends; dropping the step's own copy of the config lets it go too. + _repository.DetachSettledDeploymentRows(step.Instance.Id); + prepared[finalizedUpTo] = step with { ConfigJson = string.Empty }; } } catch (Exception ex) diff --git a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/PendingDeploymentRepositoryTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/PendingDeploymentRepositoryTests.cs index e97d8a02..24774dbc 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/PendingDeploymentRepositoryTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/PendingDeploymentRepositoryTests.cs @@ -3,6 +3,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories; namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests; @@ -416,4 +417,37 @@ public class PendingDeploymentRepositoryTests : IDisposable var row = await _repository.GetPendingDeploymentByIdAsync("dep-x"); Assert.Equal("{\"fresh\":true}", row!.ConfigurationJson); } + + /// + /// review 2026-10-21 04/M3: detaching an instance's settled rows stops tracking its saved + /// pending row, snapshot and deployment record, and leaves another instance's rows and any + /// entry with a pending write alone. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task DetachSettledDeploymentRows_DetachesOnlyThatInstancesUnchangedRows() + { + var mine = await SeedInstanceAsync("InstM3a"); + var other = await SeedInstanceAsync("InstM3b"); + await _repository.AddPendingDeploymentAsync(NewPending("m3-mine", mine, "{}")); + await _repository.AddPendingDeploymentAsync(NewPending("m3-other", other, "{}")); + await _repository.AddDeployedSnapshotAsync(new DeployedConfigSnapshot("m3-mine", "rev", "{}") { InstanceId = mine }); + var record = new DeploymentRecord("m3-mine", "admin") { InstanceId = mine, Status = DeploymentStatus.Success }; + await _repository.AddDeploymentRecordAsync(record); + await _repository.SaveChangesAsync(); + + var detached = _repository.DetachSettledDeploymentRows(mine); + + Assert.Equal(3, detached); + Assert.DoesNotContain(_context.ChangeTracker.Entries(), e => e.Entity.InstanceId == mine); + Assert.Empty(_context.ChangeTracker.Entries()); + Assert.Empty(_context.ChangeTracker.Entries()); + Assert.Contains(_context.ChangeTracker.Entries(), e => e.Entity.InstanceId == other); + + // A row with a pending write is never detached. + await _repository.AddDeploymentRecordAsync( + new DeploymentRecord("m3-unsaved", "admin") { InstanceId = mine, Status = DeploymentStatus.InProgress }); + Assert.Equal(0, _repository.DetachSettledDeploymentRows(mine)); + Assert.Single(_context.ChangeTracker.Entries()); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/DeploySiteAsyncTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/DeploySiteAsyncTests.cs index 27092d24..22d2d51e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/DeploySiteAsyncTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/DeploySiteAsyncTests.cs @@ -592,6 +592,26 @@ public class DeploySiteAsyncTests : TestKit Assert.All(sessions, s => Assert.Same(sessions[0], s)); } + /// + /// review 2026-10-21 04/M3: once an instance is finalised its settled deployment rows are + /// detached from the batch's shared unit of work, so N instances' config JSON is not held + /// (and re-scanned by every later save) for the rest of the batch. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task DeploySiteAsync_DetachesEachInstancesSettledRowsAfterFinalize() + { + ArrangeInstances(3); + var applied = new ConcurrentBag(); + var commActor = Sys.ActorOf(Props.Create(() => new RecordingSiteActor(applied))); + + var result = await CreateService(commActor, maxParallelism: 3).DeploySiteAsync(SiteId, "admin"); + + Assert.Equal(3, result.Value.SuccessCount); + foreach (var id in new[] { 1, 2, 3 }) + _repo.Received(1).DetachSettledDeploymentRows(id); + } + /// /// force restores the unconditional behaviour for operators whose /// site state central cannot see — a restored data volume or a rebuilt node