AdminUI deployments page: sync indicator can say "In sync" on first load after a draft change #903

Closed
opened 2026-11-13 01:43:13 -05:00 by dohertj2 · 2 comments
Owner

Seen on DEV vd03 (OtOpcUa 6c4d2990) on 2026-11-13, during the lmxopcua#700 deployed gate (record docs/deploy/2026-11-13-vd03-fea7de0a-to-6c4d2990.md, 9375c9e2).

What happened. After changing the draft (temporary tags added), the deployments page sometimes showed In sync on its first load. A refresh then showed the draft as differing from the deployed config.

Why it matters. An operator can believe their change is already deployed, or that there is nothing to deploy.

To do.

  • Find how the page computes draft vs. deployed. Likely culprits are a cached revision hash, a render before the comparison finishes, or a stale scoped service.
  • Make the first render show a loading state, or the correct result.
  • Add a test that changes the draft and then checks the page's first render reports out of sync.
Seen on DEV vd03 (OtOpcUa `6c4d2990`) on 2026-11-13, during the lmxopcua#700 deployed gate (record `docs/deploy/2026-11-13-vd03-fea7de0a-to-6c4d2990.md`, `9375c9e2`). **What happened.** After changing the draft (temporary tags added), the deployments page sometimes showed **In sync** on its first load. A refresh then showed the draft as differing from the deployed config. **Why it matters.** An operator can believe their change is already deployed, or that there is nothing to deploy. **To do.** - Find how the page computes draft vs. deployed. Likely culprits are a cached revision hash, a render before the comparison finishes, or a stale scoped service. - Make the first render show a loading state, or the correct result. - Add a test that changes the draft and then checks the page's first render reports out of sync.
dohertj2 added the type:bugpriority:p3 labels 2026-11-13 01:43:13 -05:00
Author
Owner

Fixed on master in 35576485.

Root cause. DeploymentDriftProbe (src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/DeploymentDriftProbe.cs) is a singleton shared by every circuit. It cached its verdict until the ConfigEdit audit watermark (MAX(EditedAtUtc), COUNT(*)) moved (old lines 83–98). That assumed every configuration write adds a ConfigEdit row. Only deployments and driver restart/reconnect do (AdminOperationsActor.cs:540/664/694). The AdminUI editors (RawTreeService, UnsTreeService, ScriptEdit, the cluster/node/ACL pages) write the configuration tables directly and add no audit row. So after the temporary tags were added, the page kept returning the cached "In sync" until something unrelated moved the watermark. The probe remarks already named this as an "accepted staleness", on the false premise that the gap was only theoretical.

Fix.

  • The probe no longer has the watermark cache. Every verdict comes from a composition that started after the caller asked. Compositions are still serialised. A caller queued behind a composition reuses its verdict only if that composition started after the caller arrived and was against the same sealed hash, so a shared answer is never stale.
  • Deployments.razor runs the drift check in OnAfterRenderAsync. That runs on the interactive render only, so a page visit costs one composition. The first render shows a neutral "Checking for changes…" chip, never a verdict. A failed check shows "Drift check failed" and logs a warning instead of breaking the page. A deploy re-runs the check.
  • Cost: one config composition per /deployments visit. That is the cost the page had before 684e59d8, and it is the price of a correct chip.

Tests. Probe tests cover a draft change with no audit row (the real ConfigComposer over InMemory), a fresh composition on each call, and queued callers not reusing an older composition. bUnit page tests check that the next page load after a direct draft change shows "Configuration drift", and that the first render shows "Checking" until the comparison answers. Mutation check: all of these fail against the previous code. DeploymentDriftProbeSqlServerTests was removed because it pinned the SQL translation of the watermark query, which no longer exists. AdminUI suite: 2171/2171 passed (SQL Server fixture live). Release build: 0 warnings.

Not re-checked in a browser; to be confirmed on the next DEV deploy.

Fixed on `master` in `35576485`. **Root cause.** `DeploymentDriftProbe` (`src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/DeploymentDriftProbe.cs`) is a singleton shared by every circuit. It cached its verdict until the `ConfigEdit` audit watermark `(MAX(EditedAtUtc), COUNT(*))` moved (old lines 83–98). That assumed every configuration write adds a `ConfigEdit` row. Only deployments and driver restart/reconnect do (`AdminOperationsActor.cs:540/664/694`). The AdminUI editors (`RawTreeService`, `UnsTreeService`, `ScriptEdit`, the cluster/node/ACL pages) write the configuration tables directly and add no audit row. So after the temporary tags were added, the page kept returning the cached "In sync" until something unrelated moved the watermark. The probe remarks already named this as an "accepted staleness", on the false premise that the gap was only theoretical. **Fix.** - The probe no longer has the watermark cache. Every verdict comes from a composition that started after the caller asked. Compositions are still serialised. A caller queued behind a composition reuses its verdict only if that composition started after the caller arrived and was against the same sealed hash, so a shared answer is never stale. - `Deployments.razor` runs the drift check in `OnAfterRenderAsync`. That runs on the interactive render only, so a page visit costs one composition. The first render shows a neutral "Checking for changes…" chip, never a verdict. A failed check shows "Drift check failed" and logs a warning instead of breaking the page. A deploy re-runs the check. - Cost: one config composition per /deployments visit. That is the cost the page had before `684e59d8`, and it is the price of a correct chip. **Tests.** Probe tests cover a draft change with no audit row (the real `ConfigComposer` over InMemory), a fresh composition on each call, and queued callers not reusing an older composition. bUnit page tests check that the next page load after a direct draft change shows "Configuration drift", and that the first render shows "Checking" until the comparison answers. Mutation check: all of these fail against the previous code. `DeploymentDriftProbeSqlServerTests` was removed because it pinned the SQL translation of the watermark query, which no longer exists. AdminUI suite: 2171/2171 passed (SQL Server fixture live). Release build: 0 warnings. Not re-checked in a browser; to be confirmed on the next DEV deploy.
Author
Owner

Deployed to DEV vd03 + QA vq03/vq04 in c9640f5c on 2026-11-13.

Live check on vd03 (/deployments, login disabled on DEV):

  • Prerendered HTML and the live page show "Checking for changes…" first, then "In sync" (rev fa9a2e28…).
  • After a draft-only change (parts-required WriteIdempotent off→on, saved, no deploy), a reload went "Checking for changes…" → "Configuration drift" with no interaction. PASS: no cached "In sync".

Observation, not a #903 regression: after reverting the toggle (row content back to its sealed value, checked against the sealed artifact), the chip stays on "Configuration drift". ConfigComposer serialises each row's RowVersion into the artifact and RevisionHash is SHA-256 of that blob, so any saved edit changes the draft hash even when it is reverted. The chip is honest about the bytes, but an edit-and-revert reads as drift until the next deploy. vd03 is left that way: the running config is still sealed 595cb909/fa9a2e28…, and no deploy was made to clear it.

Record: docs/deploy/2026-11-13-vd03-qa-to-c9640f5c.md (c0ba450d).

Deployed to DEV vd03 + QA vq03/vq04 in `c9640f5c` on 2026-11-13. Live check on vd03 (`/deployments`, login disabled on DEV): - Prerendered HTML and the live page show **"Checking for changes…"** first, then "In sync" (rev `fa9a2e28…`). - After a draft-only change (`parts-required` WriteIdempotent off→on, saved, no deploy), a reload went "Checking for changes…" → **"Configuration drift"** with no interaction. **PASS**: no cached "In sync". Observation, not a #903 regression: after reverting the toggle (row content back to its sealed value, checked against the sealed artifact), the chip **stays on "Configuration drift"**. `ConfigComposer` serialises each row's `RowVersion` into the artifact and `RevisionHash` is SHA-256 of that blob, so any saved edit changes the draft hash even when it is reverted. The chip is honest about the bytes, but an edit-and-revert reads as drift until the next deploy. vd03 is left that way: the running config is still sealed `595cb909`/`fa9a2e28…`, and no deploy was made to clear it. Record: `docs/deploy/2026-11-13-vd03-qa-to-c9640f5c.md` (`c0ba450d`).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#903