Skip to content

Commit f282b9c

Browse files
Merge pull request #318 from amazon-mq/manifest-replica-lifecycle
manifest replica: Gate sync on a live reader context
2 parents 5027a25 + a89739b commit f282b9c

11 files changed

Lines changed: 1428 additions & 38 deletions

File tree

.github/workflows/p-model-check.yaml

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ jobs:
3737
orphan_leak: ${{ steps.filter.outputs.orphan_leak }}
3838
tier_routing: ${{ steps.filter.outputs.tier_routing }}
3939
writer_fencing: ${{ steps.filter.outputs.writer_fencing }}
40+
manifest_replica_lifecycle: ${{ steps.filter.outputs.manifest_replica_lifecycle }}
4041
steps:
4142
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
4243
with:
@@ -72,6 +73,8 @@ jobs:
7273
echo "$CHANGED" | grep -q '^p/tier-routing/' && echo "tier_routing=true" >> "$GITHUB_OUTPUT" || true
7374
echo "writer_fencing=false" >> "$GITHUB_OUTPUT"
7475
echo "$CHANGED" | grep -q '^p/writer-fencing/' && echo "writer_fencing=true" >> "$GITHUB_OUTPUT" || true
76+
echo "manifest_replica_lifecycle=false" >> "$GITHUB_OUTPUT"
77+
echo "$CHANGED" | grep -q '^p/manifest-replica-lifecycle/' && echo "manifest_replica_lifecycle=true" >> "$GITHUB_OUTPUT" || true
7578
if echo "$CHANGED" | grep -q '^\.github/workflows/p-model-check\.yaml'; then
7679
echo "gc_reset=true" >> "$GITHUB_OUTPUT"
7780
echo "gc_reset_multinode=true" >> "$GITHUB_OUTPUT"
@@ -84,6 +87,7 @@ jobs:
8487
echo "orphan_leak=true" >> "$GITHUB_OUTPUT"
8588
echo "tier_routing=true" >> "$GITHUB_OUTPUT"
8689
echo "writer_fencing=true" >> "$GITHUB_OUTPUT"
90+
echo "manifest_replica_lifecycle=true" >> "$GITHUB_OUTPUT"
8791
fi
8892
8993
gc-reset:
@@ -370,6 +374,74 @@ jobs:
370374
echo "Gate satisfied: PCT reaches the reset-after-snapshot live deletion."
371375
echo "::endgroup::"
372376
377+
manifest-replica-lifecycle:
378+
needs: changes
379+
if: needs.changes.outputs.manifest_replica_lifecycle == 'true' || github.event_name == 'schedule'
380+
runs-on: ubuntu-latest
381+
steps:
382+
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
383+
- uses: actions/setup-dotnet@26b0ec14cb23fa6904739307f278c14f94c95bf1 # v5.4.0
384+
with:
385+
dotnet-version: '8.0.x'
386+
- name: Install the P checker
387+
run: |
388+
dotnet tool install --global P --version 3.1.0 --configfile "$GITHUB_WORKSPACE/p/NuGet.config"
389+
echo "$HOME/.dotnet/tools" >> "$GITHUB_PATH"
390+
- name: Compile
391+
run: |
392+
cd p/manifest-replica-lifecycle
393+
p compile
394+
- name: Tests that must hold
395+
run: |
396+
cd p/manifest-replica-lifecycle
397+
for tc in tcCleanupGuarded tcStaleSyncGuarded tcReregisterGuarded \
398+
tcConvergenceGuarded tcForgetReleasesWriterRow tcSyncAfterExitFixed \
399+
tcStartupRaceContextFirst tcReconcileRaceResync tcExplore; do
400+
echo "::group::$tc (expect hold)"
401+
p check -tc "$tc" -i 5000
402+
echo "::endgroup::"
403+
done
404+
# PCT surfaces rare interleavings the random strategy misses.
405+
echo "::group::tcExplore (PCT)"
406+
p check -tc tcExplore --sch-pct 10 -i 5000
407+
echo "::endgroup::"
408+
- name: Validation gate (each guard is load-bearing)
409+
run: |
410+
cd p/manifest-replica-lifecycle
411+
# Each gate removes one guard and must reproduce its specific failure,
412+
# otherwise the guarded results above are not trustworthy. A gate that
413+
# passes means the guard is no longer load-bearing, or the model rotted.
414+
check_gate() {
415+
tc="$1"; expected="$2"
416+
rm -rf PCheckerOutput/
417+
echo "::group::$tc (expect failure: $expected)"
418+
if p check -tc "$tc" -i 2000; then
419+
echo "ERROR: $tc passed with the guard removed; the model no longer reproduces the failure."
420+
exit 1
421+
fi
422+
if ! grep -rqF "$expected" PCheckerOutput/BugFinding/; then
423+
echo "ERROR: $tc failed, but not with the expected counterexample: $expected"
424+
exit 1
425+
fi
426+
echo "Gate satisfied: $tc reproduces the failure as expected."
427+
echo "::endgroup::"
428+
}
429+
# The three shipped guards: member-DOWN cleanup, stale-sync ordering,
430+
# and the re-registration monitor repoint.
431+
check_gate tcCleanupUnguarded 'NOLEAK violated: replica holds per-node state for stream 0 with no live reader'
432+
check_gate tcStaleSyncUnguarded 'STALEFLOOR violated: cache for stream 0 regressed to (epoch=1, sn=1) below already-applied (epoch=2, sn=2)'
433+
check_gate tcReregisterUnguarded 'RETAIN violated: replica dropped per-node state for stream 0 that still has a live reader (a superseded member DOWN evicted the live context)'
434+
# The gap this model surfaces: a sync after a member DOWN re-strands the
435+
# row (the shipped-in-this-branch A2 guard closes it in tcSyncAfterExitFixed).
436+
check_gate tcSyncAfterExitStrands 'NOLEAK violated: replica holds per-node state for stream 0 with no live reader'
437+
# Liveness: syncs dropped forever (tcConvergenceStuck), and the two
438+
# startup triggers that starve the cache without recovery - the
439+
# WriterFirst acceptor race (A1 fixes it) and the writer-driven reconcile
440+
# race (only A1' can, since the node's attach ordering cannot reach it).
441+
check_gate tcConvergenceStuck "ReplicaConverges detected liveness bug in hot state 'Lagging'"
442+
check_gate tcStartupRaceWriterFirst "ReplicaConverges detected liveness bug in hot state 'Lagging'"
443+
check_gate tcReconcileRaceNoResync "ReplicaConverges detected liveness bug in hot state 'Lagging'"
444+
373445
delete-stream-anchor:
374446
needs: changes
375447
if: needs.changes.outputs.delete_stream_anchor == 'true' || github.event_name == 'schedule'

p/README.md

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ The models target five global invariants of the tiered-storage design:
2121
| --- | --- | --- |
2222
| INV#1 | no lost acked data | `gc-reset/` |
2323
| INV#2 | no dangling reference (GC never deletes a live object) | `gc-reset/`, `gc-leading-group/`, `gc-decision/` |
24-
| INV#3 | no unbounded orphan leak (liveness) | `orphan-leak/` |
24+
| INV#3 | no unbounded orphan leak (liveness) | `orphan-leak/`, `manifest-replica-lifecycle/` |
2525
| INV#4 | tier resolution total / gap-free | `read-resolution/`, `tier-routing/` |
2626
| INV#5 | monotonic frontier except a labeled reset | `gc-reset/`, `trimmed-segment/` |
27+
| (lifecycle) | per-node replica state is released when a reader exits | `manifest-replica-lifecycle/` |
2728
| (foundation) | epoch monotonicity / split-brain safety | `writer-fencing/` |
2829

2930
The last row is the mechanism the others assume: `writer-fencing/` proves the
@@ -100,6 +101,29 @@ re-sweep eventually reclaims a transiently-failed delete rather than leaking it
100101
forever. A *liveness* property (`hot`-state monitor). See
101102
[`orphan-leak/README.md`](orphan-leak/README.md).
102103

104+
### `manifest-replica-lifecycle/`
105+
106+
The per-node manifest replica's lifecycle: registration and cleanup when a reader
107+
(osiris member) exits, verifying the design behind commit `cc50092`. osiris has no
108+
terminate hook, so the replica monitors the registering member and releases its
109+
context, gap sequence, and cached row on `'DOWN'`. Proves three shipped guards
110+
load-bearing — the member-`DOWN` cleanup (no metadata leak), `is_stale_sync` (no
111+
stale floor served), and the re-registration monitor repoint (no eviction of a
112+
live reader) — plus a `hot`-state convergence property. Surfaces a gap the
113+
single-axis view misses: with every shipped guard on, a sync that arrives after
114+
the `DOWN` re-strands a cache row with no monitor to reclaim it, and models a
115+
proposed `syncRequiresContext` guard (A2) that closes it. Adds an attach-ordering
116+
axis that proves the guard is **unsafe in isolation**: with the shipped writer-first
117+
attach order a startup sync that beats the local context registration is dropped
118+
and never re-sent (the cache stays empty — a convergence violation). A1
119+
(register-context-before-writer) closes that for the acceptor-reply sync, but a
120+
second axis models the **writer-driven reconcile path** — a member-visible sync
121+
independent of `register_acceptor` that A1 cannot reach — and a proposed
122+
**A1′ resync-on-register** toggle. The gates show A1′ recovers *both* startup
123+
triggers (acceptor reply and reconcile) and **subsumes A1**, so the minimal sound
124+
fix is **A2 + A1′**, not A2 + A1 + A1′.
125+
See [`manifest-replica-lifecycle/README.md`](manifest-replica-lifecycle/README.md).
126+
103127
## Running
104128

105129
Requires the `p` CLI. From a model directory:
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/* Global invariant monitors for the manifest-replica lifecycle seam.
2+
3+
ReplicaStateMatchesReaders (safety, INV: no leak / no premature loss). At a
4+
quiesce checkpoint, the set of streams the replica still holds state for must
5+
equal the set of streams with a live reader. Two directions, each tied to a
6+
guard:
7+
- held => live (no leak): a stranded entry for an exited reader trips this.
8+
Proves the member-DOWN cleanup (G1) load-bearing, and surfaces the
9+
sync-after-exit gap (a sync re-creates a row after cleanup released it).
10+
- live => held (no loss): evicting a live reader's state trips this. Proves
11+
the re-registration monitor repoint (G3) load-bearing.
12+
13+
NoStaleFloorServed (safety, INV: no stale floor). Read-side analogue of the
14+
../gc-reset-multinode finding. The cache's applied (epoch, sn) must be
15+
monotonically non-decreasing and must only ever hold a committed floor, so a
16+
replica never serves a floor that a newer write already superseded. Proves
17+
is_stale_sync (G2) load-bearing.
18+
19+
ReplicaConverges (liveness, hot state). Once the writer has committed a floor,
20+
a replica that keeps receiving syncs must eventually catch up to the latest
21+
committed (epoch, sn). A run that ends with the cache permanently behind is a
22+
liveness violation. */
23+
24+
/* held(stream) == (live readers for stream > 0), checked at quiesce. */
25+
spec ReplicaStateMatchesReaders
26+
observes eReaderUp, eReaderDown, eQuiesceBegin, eHeld, eQuiesceEnd {
27+
var liveReaders: map[StreamId, int];
28+
var held: set[StreamId];
29+
30+
start state Watching {
31+
on eReaderUp do (s: StreamId) {
32+
if (s in liveReaders) {
33+
liveReaders[s] = liveReaders[s] + 1;
34+
} else {
35+
liveReaders[s] = 1;
36+
}
37+
}
38+
on eReaderDown do (s: StreamId) {
39+
if (s in liveReaders) {
40+
liveReaders[s] = liveReaders[s] - 1;
41+
if (liveReaders[s] == 0) {
42+
liveReaders -= (s);
43+
}
44+
}
45+
}
46+
on eQuiesceBegin do {
47+
held = default(set[StreamId]);
48+
}
49+
on eHeld do (s: StreamId) {
50+
held += (s);
51+
}
52+
on eQuiesceEnd do {
53+
var s: StreamId;
54+
/* No leak: every held stream must still have a live reader. */
55+
foreach (s in held) {
56+
assert s in liveReaders,
57+
format("NOLEAK violated: replica holds per-node state for stream {0} with no live reader (an exited reader was not cleaned up, or a sync re-stranded the row after cleanup)",
58+
s);
59+
}
60+
/* No loss: every live reader's stream must still be held. */
61+
foreach (s in keys(liveReaders)) {
62+
assert s in held,
63+
format("RETAIN violated: replica dropped per-node state for stream {0} that still has a live reader (a superseded member DOWN evicted the live context)",
64+
s);
65+
}
66+
}
67+
}
68+
}
69+
70+
/* The cache may only ever hold a committed floor, and its applied (epoch, sn)
71+
must never regress - the read-side stale-floor guard. */
72+
spec NoStaleFloorServed observes eFloorCommitted, eCacheUpdated {
73+
var committed: set[(floor: int, epoch: int, sn: int)];
74+
var maxApplied: map[StreamId, (epoch: int, sn: int)];
75+
76+
start state Watching {
77+
on eFloorCommitted do (p: (floor: int, epoch: int, sn: int)) {
78+
committed += ((floor = p.floor, epoch = p.epoch, sn = p.sn));
79+
}
80+
on eCacheUpdated do (p: (stream: StreamId, floor: int, epoch: int, sn: int)) {
81+
var prev: (epoch: int, sn: int);
82+
assert (floor = p.floor, epoch = p.epoch, sn = p.sn) in committed,
83+
format("STALEFLOOR violated: replica cached an uncommitted floor for stream {0} (floor={1}, epoch={2}, sn={3})",
84+
p.stream, p.floor, p.epoch, p.sn);
85+
if (p.stream in maxApplied) {
86+
prev = maxApplied[p.stream];
87+
assert !StaleLex(p.epoch, p.sn, prev.epoch, prev.sn),
88+
format("STALEFLOOR violated: cache for stream {0} regressed to (epoch={1}, sn={2}) below already-applied (epoch={3}, sn={4}) - a stale sync rolled the floor backward",
89+
p.stream, p.epoch, p.sn, prev.epoch, prev.sn);
90+
}
91+
maxApplied[p.stream] = (epoch = p.epoch, sn = p.sn);
92+
}
93+
}
94+
}
95+
96+
/* Liveness: a single tracked stream's cache must eventually reach the latest
97+
committed (epoch, sn). Lagging is hot: a terminal state with the cache behind
98+
the committed floor is a violation. */
99+
spec ReplicaConverges observes eFloorCommitted, eCacheUpdated {
100+
var committedEpoch: int;
101+
var committedSeq: int;
102+
var cacheEpoch: int;
103+
var cacheSeq: int;
104+
105+
start state Caught {
106+
on eFloorCommitted do (p: (floor: int, epoch: int, sn: int)) {
107+
committedEpoch = p.epoch;
108+
committedSeq = p.sn;
109+
if (StaleLex(cacheEpoch, cacheSeq, committedEpoch, committedSeq)) {
110+
goto Lagging;
111+
}
112+
}
113+
on eCacheUpdated do (p: (stream: StreamId, floor: int, epoch: int, sn: int)) {
114+
cacheEpoch = p.epoch;
115+
cacheSeq = p.sn;
116+
}
117+
}
118+
119+
hot state Lagging {
120+
on eFloorCommitted do (p: (floor: int, epoch: int, sn: int)) {
121+
committedEpoch = p.epoch;
122+
committedSeq = p.sn;
123+
}
124+
on eCacheUpdated do (p: (stream: StreamId, floor: int, epoch: int, sn: int)) {
125+
cacheEpoch = p.epoch;
126+
cacheSeq = p.sn;
127+
if (!StaleLex(cacheEpoch, cacheSeq, committedEpoch, committedSeq)) {
128+
goto Caught;
129+
}
130+
}
131+
}
132+
}

0 commit comments

Comments
 (0)