Skip to content

Restore coherent durable Subscription snapshots (OPC 10000-4)#4045

Merged
marcschier merged 3 commits into
OPCFoundation:masterfrom
marcschier:marcschier/review-fix/subscription-snapshot-generation
Jul 21, 2026
Merged

Restore coherent durable Subscription snapshots (OPC 10000-4)#4045
marcschier merged 3 commits into
OPCFoundation:masterfrom
marcschier:marcschier/review-fix/subscription-snapshot-generation

Conversation

@marcschier

@marcschier marcschier commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Failure

Durable Subscription restore depended on a process-local cache, while full snapshots were written record-by-record under shared mutable keys. A restart or concurrent writer could therefore expose stale, mixed, or partially committed Subscription state.

Fix

  • Write each snapshot to an immutable generation and atomically publish a protected manifest.
  • Restore only the committed generation, validate record identities and completeness, and retain legacy-key fallback.
  • Replace the cache only after the full snapshot is authenticated and decoded, including explicit empty snapshots.

Standard

OPC 10000-4 §6.6.2.4.4 and §6.6.2.4.5.5 require durable Subscription and retransmission state to remain coherent across redundant Server failover and restart.

Tests

  • dotnet test tests\Opc.Ua.Redundancy.Server.Tests\Opc.Ua.Redundancy.Server.Tests.csproj -f net10.0 --no-restore --nologo --verbosity minimal — 542 passed

  • CustomTestTarget=net48 dotnet test tests\Opc.Ua.Redundancy.Server.Tests\Opc.Ua.Redundancy.Server.Tests.csproj --no-restore --nologo --verbosity minimal -m:1 — 542 passed

  • Review follow-up: SharedKeyValueSubscriptionStoreTests — 39 passed on net10.0 and 39 passed on net48.

Split

Split from #4021.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 10:49
Copilot AI review requested due to automatic review settings July 19, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Improves durability/coherence of persisted Subscription snapshots by moving to immutable, generation-based records with a committed manifest, and strengthening restore-time validation to prevent mixed/partial/tampered state from being loaded.

Changes:

  • Persist snapshots into per-generation immutable keys and publish a protected manifest used for restore.
  • Restore only from the committed generation (or legacy keys when manifest is absent) with added identity/completeness validation.
  • Expand test coverage for concurrent snapshot commits, process restart behavior, empty snapshots, tamper/corruption handling, and legacy fallback.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/Opc.Ua.Redundancy.Server.Tests/Subscriptions/SharedKeyValueSubscriptionStoreTests.cs Adds/updates tests validating generation-commit semantics, restart/legacy fallback, and tamper/corruption behavior.
src/Opc.Ua.Redundancy.Server/Subscriptions/SharedKeyValueSubscriptionStore.cs Implements generation+manifest snapshot persistence and stricter restore-time validation; updates cache replacement behavior.

Comment thread src/Opc.Ua.Redundancy.Server/Subscriptions/SharedKeyValueSubscriptionStore.cs Outdated
@marcschier
marcschier marked this pull request as draft July 19, 2026 11:35
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 11:55
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as draft July 19, 2026 13:38
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 14:13
@marcschier
marcschier marked this pull request as draft July 19, 2026 17:25
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 17:31
@marcschier
marcschier requested a review from romanett July 19, 2026 18:37
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 153 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.50%. Comparing base (70c74e7) to head (d8367ff).
⚠️ Report is 21 commits behind head on master.

Files with missing lines Patch % Lines
...r/Subscriptions/SharedKeyValueSubscriptionStore.cs 0.00% 153 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4045      +/-   ##
==========================================
- Coverage   73.55%   73.50%   -0.05%     
==========================================
  Files        1345     1345              
  Lines      180036   180213     +177     
  Branches    31677    31702      +25     
==========================================
+ Hits       132427   132470      +43     
- Misses      36911    37031     +120     
- Partials    10698    10712      +14     
Files with missing lines Coverage Δ
...r/Subscriptions/SharedKeyValueSubscriptionStore.cs 0.00% <0.00%> (ø)

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ueSubscriptionStore

Add deterministic NUnit coverage for the snapshot-generation/manifest
rework: duplicate subscription-id rejection, cancellation before the
commit semaphore, commit-lock release and cache/manifest integrity on
a mid-write store failure, generation retention across sequential
commits, tampered/malformed manifests, malformed generation keys
(bare prefix, non-numeric suffix, numeric overflow), duplicate
records surfaced by a non-idempotent scan, incomplete generations,
persisted records with invalid monitored-item identities, and
trailing data after a decoded record.

Raises measured patch coverage for the changed lines in
src/Opc.Ua.Redundancy.Server/Subscriptions/SharedKeyValueSubscriptionStore.cs
from 127/154 (82.47%) to 154/154 (100%).
@marcschier marcschier added the ready Ready to merge once CI Passes label Jul 21, 2026
@marcschier
marcschier merged commit 6b4208a into OPCFoundation:master Jul 21, 2026
208 of 211 checks passed
@marcschier
marcschier deleted the marcschier/review-fix/subscription-snapshot-generation branch July 21, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready Ready to merge once CI Passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants