Skip to content

Commit ea2cdc9

Browse files
author
SqlRush
committed
fix(cluster): clean-leave own-commit latch consumes marker evidence, not inference
Nightly t/331 C1 (clean_leave x idle) / C4 (leave_remove x idle) regressed at the r2 P2-1 head (run 28948167577; parent green): the leaver's barrier-tick own-commit latch inferred its commit from epoch>baseline + others-dead bitmap unchanged + scalar dead_generation unchanged. The scalar is monotone, so a third-party transient SUSPECTED->DEAD->ALIVE flap on the leaver's local CSSD view during the leave window advances it forever; the latch then refuses a leave the coordinator ACTUALLY committed and the immediate-escalate arm (or the barrier deadline) aborts it: phase ends ABORTED_ESCALATE -> IDLE, never 'committed', breaking the C1/C4 drains+commits assertions. Reverting to bitmap-only would re-open the r2 P2-1 wedge (rebound mis-latch of a REFUSED leave suppresses escalation forever), so neither inference is usable. Replace inference with direct evidence: latch <=> the durable COMMITTED marker for THIS leave attempt is confirmed. The evidence already reaches the leaver with zero extra IO: the coordinator sends the nonce-bound LEAVE_COMMITTED only after its qvotec ACKs the COMMITTED marker majority-durable (there is no runtime voting-disk read path on the leaver, and none is needed - the LMON tick budget this spec protects is untouched). cl_committed_handler now routes through a pure identity gate (self-addressed + currently leaving + per-attempt nonce + committed epoch past the bound baseline; fail-closed on any mismatch, so a stale confirmation - and through it a stale COMMITTED marker from a previous leave of the same node - can never false-latch) and records the attested committed epoch E before publishing the evidence flag; the barrier tick consumes E instead of re-reading its possibly-stale local epoch view. The latch and the P1-V0.7 exit gate collapse into one step (the evidence IS the durable-truth-source confirmation). Escalation semantics are unchanged: no evidence by the barrier deadline -> the existing cl_escalate path, which is exactly what bounds a refused leave (it never gets a COMMITTED marker). Third-party flaps no longer matter: the coherence observations are now contract inputs the predicate must ignore (pinned by the U3b unit matrix, red-first in the parent commit) and feed only a flap-noise LOG at the latch. Survivor-side coherence sites (drive_drain / staged-ACK / pre-check) are untouched. Unit: U3b matrix flipped green; new U3c pins the evidence identity gate (match / stale-nonce / misrouted / not-leaving / epoch-not-advanced). Local gates: t/331 x4 all green (6/6), t/310 24/24, t/363 18/18, t/274 18/18 (prebump shmem-bit stays green), cluster_unit 158 binaries, cluster_regress 13/13, PG core 219/219, clang-format-18 + comment-headers clean. Spec: spec-2.29a-reconfig-marker-async-lmon-liveness.md
1 parent ee2c09f commit ea2cdc9

4 files changed

Lines changed: 218 additions & 116 deletions

File tree

src/backend/cluster/cluster_clean_leave.c

Lines changed: 89 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ cluster_clean_leave_shmem_init(void)
122122
pg_atomic_init_u32(&cl_state->commit_point_observed, 0);
123123
pg_atomic_init_u32(&cl_state->committed_durable_confirmed, 0);
124124
pg_atomic_init_u32(&cl_state->committed_marker_durable, 0);
125+
/* spec-2.29a r3 (evidence latch). */
126+
cl_state->committed_confirmed_epoch = 0;
125127
/* Hardening v1.0.2 (P1 preflight / P2 nonce). */
126128
pg_atomic_init_u64(&cl_state->leave_attempt_nonce, 0);
127129
pg_atomic_init_u32(&cl_state->preflight_pending, 0);
@@ -555,11 +557,15 @@ cl_commit_ready_handler(const ClusterICEnvelope *env, const void *payload)
555557
}
556558

557559
/*
558-
* cl_committed_handler -- leaving node side (Hardening v1.0.1, P1-V0.7 exit gate):
559-
* the survivor coordinator has made the COMMITTED marker majority-durable and
560-
* signals that the durable truth-source now exists, so this leaving node may
561-
* proceed to COMMITTED and exit. Only acted on if it is about OUR leave; idem-
562-
* potent (the coordinator re-sends each tick until we are gone).
560+
* cl_committed_handler -- leaving node side (Hardening v1.0.1, P1-V0.7 exit gate;
561+
* spec-2.29a r3 evidence latch): the survivor coordinator has made the COMMITTED
562+
* marker majority-durable and attests that the durable truth-source now exists.
563+
* This confirmation is the leaver's own-commit MARKER EVIDENCE: the barrier tick
564+
* latches on it (and only on it) and proceeds to COMMITTED/exit. Identity is
565+
* checked by the pure evidence gate (self-addressed + currently leaving +
566+
* per-attempt nonce + committed epoch past the bound baseline) — any mismatch is
567+
* dropped fail-closed. Idempotent (the coordinator re-sends each tick until we
568+
* are gone).
563569
*/
564570
static void
565571
cl_committed_handler(const ClusterICEnvelope *env, const void *payload)
@@ -571,17 +577,19 @@ cl_committed_handler(const ClusterICEnvelope *env, const void *payload)
571577
return;
572578
if (!cluster_clean_leave_announce_payload_valid(p))
573579
return;
574-
if (p->leaving_node_id != cluster_node_id)
575-
return; /* only the leaving node consumes its own COMMITTED confirmation */
576-
/* Hardening v1.0.2 (P2): bind to THIS attempt — a stale LEAVE_COMMITTED from a
577-
* prior same-epoch attempt must not prematurely confirm the current one. Only
578-
* meaningful once we are the leaver actively awaiting confirmation. */
579-
if (p->leave_nonce != pg_atomic_read_u64(&cl_state->leave_attempt_nonce))
580+
LWLockAcquire(&cl_state->lock, LW_EXCLUSIVE);
581+
if (!cluster_clean_leave_committed_evidence_matches(
582+
p->leaving_node_id, p->leave_nonce, p->leave_epoch, cluster_node_id,
583+
cl_state->leaving_node_id, pg_atomic_read_u64(&cl_state->leave_attempt_nonce),
584+
cl_state->leave_epoch)) {
585+
LWLockRelease(&cl_state->lock);
580586
return;
581-
if (cl_state->leaving_node_id != cluster_node_id)
582-
return; /* we are not currently leaving */
583-
587+
}
588+
/* the committed epoch E is recorded BEFORE the evidence flag is published,
589+
* inside the same critical section the barrier tick reads it back under. */
590+
cl_state->committed_confirmed_epoch = p->leave_epoch;
584591
pg_atomic_write_u32(&cl_state->committed_durable_confirmed, 1);
592+
LWLockRelease(&cl_state->lock);
585593
}
586594

587595
/*
@@ -1467,6 +1475,7 @@ cl_request_body(void)
14671475
pg_atomic_write_u32(&cl_state->commit_point_observed, 0);
14681476
pg_atomic_write_u32(&cl_state->committed_durable_confirmed, 0);
14691477
pg_atomic_write_u32(&cl_state->committed_marker_durable, 0);
1478+
cl_state->committed_confirmed_epoch = 0; /* spec-2.29a r3: per-attempt evidence */
14701479
LWLockRelease(&cl_state->lock);
14711480
cl_set_phase(CLUSTER_LEAVE_REQUESTED);
14721481

@@ -1755,83 +1764,72 @@ cl_leaving_barrier_tick(void)
17551764
{
17561765
uint64 baseline_epoch = cl_state->leave_epoch;
17571766
int32 coordinator;
1767+
uint8 now_others_dead[CLUSTER_CLEAN_LEAVE_ACK_BITMAP_BYTES];
1768+
bool committed_evidence;
1769+
bool others_dead_unchanged;
1770+
bool dead_gen_unchanged;
17581771

17591772
/*
1760-
* 1. observe the commit. The survivor coordinator runs the actual two-phase
1761-
* commit and publishes the CLEAN_LEAVE event into ITS OWN reconfig state —
1762-
* the leaving node's last_applied never carries it. What DOES propagate to
1763-
* the leaving node is the membership epoch (every IC envelope piggybacks it).
1764-
* So the leaving node detects its own commit by the epoch advancing past the
1765-
* bound baseline. Discriminate from a real death intruding (CL-I3) by the
1766-
* CSSD dead_generation: a cooperative leave does NOT mark anyone CSSD-dead, so
1767-
* an unchanged dead_generation at the bump means OUR clean-leave committed; a
1768-
* changed one means a real death (escalate).
1773+
* 1. observe the commit — EVIDENCE over inference (spec-2.29a r3). The
1774+
* survivor coordinator runs the actual two-phase commit, publishes the
1775+
* CLEAN_LEAVE event into ITS OWN reconfig state, drives the COMMITTED marker
1776+
* to voting-disk majority-durability, and only then sends the nonce-bound
1777+
* LEAVE_COMMITTED (re-sent each tick while we are alive). That confirmation
1778+
* — validated by the pure identity gate in cl_committed_handler — is the ONLY
1779+
* basis on which this node latches its own commit: latch <=> a valid
1780+
* COMMITTED marker for THIS leave attempt exists.
17691781
*
1770-
* Hardening v1.0.4 (P2): this "epoch bump + dead_gen unchanged == OUR commit"
1771-
* inference is sound ONLY because there is no OTHER dead_gen-unchanged reconfig
1772-
* in flight. A spec-5.15 online JOIN also bumps the epoch with dead_gen
1773-
* unchanged and would be mis-observed here as the leave's commit (then the node
1774-
* stops escalating but never gets the real LEAVE_COMMITTED -> BARRIER_WAIT
1775-
* hang). That collision is removed STRUCTURALLY by the one-membership-reconfig-
1776-
* at-a-time serialization: the join driver does not bump the epoch while a clean
1777-
* leave is active (cluster_clean_leave_in_progress gates drive_joins +
1778-
* commit_member), and the leave does not start while a join is pending
1779-
* (cluster_reconfig_join_in_progress gates the request). Under that invariant a
1780-
* bump with an unchanged version during a leave can only be the leave's own
1781-
* commit.
1782+
* Two inference generations preceded this and each failed one way (see the
1783+
* predicate comment in cluster_clean_leave_policy.c): "epoch advanced +
1784+
* others-dead bitmap unchanged" could mis-latch a REFUSED leave after a
1785+
* third-party false-DEAD rebound (r2 P2-1 wedge); adding the monotone scalar
1786+
* dead_generation conjunct then false-escalated a healthy committed leave
1787+
* whenever a transient third-party flap advanced the leaver's local
1788+
* dead_generation during the leave window (nightly t/331 C1/C4). Marker
1789+
* evidence is immune to both: flaps cannot erase a durable marker, and a
1790+
* refused leave never produces one — no latch, so the barrier deadline below
1791+
* still bounds the wait (fail-closed escalation, unchanged semantics).
17821792
*
1783-
* spec-2.29a ②b + r2 P2-1: "unchanged version" here means the others-dead
1784-
* bitmap AND the scalar dead_generation both unchanged. The bitmap excludes
1785-
* the leaving node's own expected DEAD (②b: else its heartbeat stop would
1786-
* falsely escalate), but the bitmap is not monotone — a third-party
1787-
* false-DEAD→ALIVE rebound restores it while the scalar dead_generation only
1788-
* advances (r2 P2-1: else the leaver could mis-latch a refused leave and
1789-
* hang). The leaver's own DEAD never bumps its OWN dead_generation, so the
1790-
* scalar conjunct is safe on this side (it would NOT be safe on the survivor
1791-
* side, which keeps the bitmap-only coherence check).
1793+
* The coherence observations are still taken, but ONLY for the flap-noise
1794+
* LOG at the latch (the predicate contract pins that they never affect the
1795+
* verdict). A real third-party death intruding mid-leave is refused on the
1796+
* survivor side (cl_coherent pre-check + guarded CAS, CL-I3), so no evidence
1797+
* arrives here and the deadline escalates this leaver — same outcome as the
1798+
* old immediate escalate arm, now bounded by the barrier deadline instead.
1799+
*
1800+
* Latching collapses the old two-step gate: the evidence IS the P1-V0.7
1801+
* durable-truth-source confirmation, so the leave can no longer be
1802+
* un-committed (deadline must not escalate past here, Hardening v1.0.1
1803+
* P1-1) AND the node may exit (COMMITTED) in the same tick.
17921804
*/
1793-
if (cluster_epoch_get_current() > baseline_epoch) {
1794-
uint8 now_others_dead[CLUSTER_CLEAN_LEAVE_ACK_BITMAP_BYTES];
1795-
bool others_dead_unchanged;
1796-
bool dead_gen_unchanged;
1797-
1798-
cl_others_dead_snapshot(cl_state->leaving_node_id, now_others_dead);
1799-
others_dead_unchanged = (memcmp(now_others_dead, cl_state->leave_baseline_others_dead,
1800-
CLUSTER_CLEAN_LEAVE_ACK_BITMAP_BYTES)
1801-
== 0);
1802-
dead_gen_unchanged
1803-
= (cluster_cssd_get_dead_generation() == cl_state->leave_baseline_dead_gen);
1804-
if (cluster_clean_leave_own_commit_latched(true, others_dead_unchanged,
1805-
dead_gen_unchanged)) {
1806-
/*
1807-
* Commit point observed (our clean-leave epoch was published; no
1808-
* third-party death intruded). The leave can no longer be
1809-
* un-committed, so from here the barrier deadline must NOT escalate
1810-
* (Hardening v1.0.1 P1-1).
1811-
*/
1812-
pg_atomic_write_u32(&cl_state->commit_point_observed, 1);
1813-
LWLockAcquire(&cl_state->lock, LW_EXCLUSIVE);
1814-
cl_state->leave_epoch = cluster_epoch_get_current(); /* the committed epoch E */
1815-
LWLockRelease(&cl_state->lock);
1805+
committed_evidence = (pg_atomic_read_u32(&cl_state->committed_durable_confirmed) != 0);
1806+
cl_others_dead_snapshot(cl_state->leaving_node_id, now_others_dead);
1807+
others_dead_unchanged = (memcmp(now_others_dead, cl_state->leave_baseline_others_dead,
1808+
CLUSTER_CLEAN_LEAVE_ACK_BITMAP_BYTES)
1809+
== 0);
1810+
dead_gen_unchanged = (cluster_cssd_get_dead_generation() == cl_state->leave_baseline_dead_gen);
1811+
if (cluster_clean_leave_own_commit_latched(committed_evidence, others_dead_unchanged,
1812+
dead_gen_unchanged)) {
1813+
uint64 committed_epoch;
1814+
1815+
pg_atomic_write_u32(&cl_state->commit_point_observed, 1);
1816+
LWLockAcquire(&cl_state->lock, LW_EXCLUSIVE);
1817+
committed_epoch = cl_state->committed_confirmed_epoch; /* the committed epoch E */
1818+
cl_state->leave_epoch = committed_epoch;
1819+
LWLockRelease(&cl_state->lock);
18161820

1817-
/*
1818-
* P1-V0.7 exit gate: reach COMMITTED ("may exit") ONLY after the
1819-
* coordinator confirms the COMMITTED marker is majority-durable
1820-
* (LEAVE_COMMITTED). Until then stay in BARRIER_WAIT and re-tick — the
1821-
* durable truth-source must exist before this node departs, else a
1822-
* survivor restart could not rebuild the clean-departed fact.
1823-
*/
1824-
if (pg_atomic_read_u32(&cl_state->committed_durable_confirmed)) {
1825-
cl_set_phase(CLUSTER_LEAVE_COMMITTED);
1826-
CLUSTER_INJECTION_POINT("cluster-clean-leave-barrier-complete");
1827-
ereport(LOG,
1828-
(errmsg("cluster clean-leave: committed at epoch %llu + COMMITTED marker "
1829-
"majority-durable; this node has drained and may exit",
1830-
(unsigned long long)cl_state->leave_epoch)));
1831-
}
1832-
} else {
1833-
cl_escalate(); /* a real death changed the version mid-leave (CL-I3) */
1834-
}
1821+
if (!others_dead_unchanged || !dead_gen_unchanged)
1822+
ereport(LOG, (errmsg("cluster clean-leave: third-party liveness flap observed at the "
1823+
"commit latch (others-dead %s, dead_generation %s); durable "
1824+
"COMMITTED marker evidence overrides it",
1825+
others_dead_unchanged ? "unchanged" : "changed",
1826+
dead_gen_unchanged ? "unchanged" : "advanced")));
1827+
1828+
cl_set_phase(CLUSTER_LEAVE_COMMITTED);
1829+
CLUSTER_INJECTION_POINT("cluster-clean-leave-barrier-complete");
1830+
ereport(LOG, (errmsg("cluster clean-leave: committed at epoch %llu + COMMITTED marker "
1831+
"majority-durable; this node has drained and may exit",
1832+
(unsigned long long)committed_epoch)));
18351833
return;
18361834
}
18371835

@@ -1842,8 +1840,12 @@ cl_leaving_barrier_tick(void)
18421840
}
18431841

18441842
/* 3. fail-closed deadline — ONLY before the commit point (P1-1: a committed
1845-
* leave is never un-committed; post-commit we wait for the durable marker,
1846-
* bounded by disk health, and never escalate). */
1843+
* leave is never un-committed). With the r3 evidence latch this arm is what
1844+
* bounds EVERY no-evidence outcome: a refused leave, a foreign death that
1845+
* moved the version (the coordinator then refuses, CL-I3), or a lost/never-
1846+
* sent confirmation all end here instead of hanging in BARRIER_WAIT. The
1847+
* commit_point_observed guard is belt-and-braces: the latch above transitions
1848+
* out of BARRIER_WAIT in the same tick it sets the flag. */
18471849
if (!pg_atomic_read_u32(&cl_state->commit_point_observed)
18481850
&& (uint64)GetCurrentTimestamp() > cl_state->barrier_deadline_us) {
18491851
cl_escalate();

src/backend/cluster/cluster_clean_leave_policy.c

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -169,32 +169,79 @@ cluster_clean_leave_version_coherent(uint64 bound_epoch, uint64 current_epoch,
169169

170170
/*
171171
* cluster_clean_leave_own_commit_latched -- the LEAVING node's barrier tick
172-
* infers "my clean-leave committed" from the membership epoch advancing past
173-
* its bound baseline (the coordinator publishes the CLEAN_LEAVE event into ITS
174-
* own reconfig state, but the epoch piggybacks to the leaver). Latching that
175-
* inference suppresses the barrier-deadline escalation, so a FALSE latch hangs
176-
* the leaver in BARRIER_WAIT forever.
172+
* latches "my clean-leave committed" on direct EVIDENCE, never on inference
173+
* (spec-2.29a r3). The evidence is committed_marker_evidence: the survivor
174+
* coordinator made the COMMITTED marker for THIS leave attempt majority-
175+
* durable on the voting disk and attested it with a nonce-bound
176+
* LEAVE_COMMITTED (validated by cluster_clean_leave_committed_evidence_
177+
* matches before the flag this argument mirrors is ever set). Latching
178+
* suppresses the barrier-deadline escalation and lets the leaver exit, so
179+
* the verdict must be exactly: latch <=> evidence.
177180
*
178-
* spec-2.29a r2 P2-1: the leaver's own-commit inference needs BOTH a bitmap
179-
* check AND the scalar dead_generation. The others-dead bitmap alone is not
180-
* monotone: a third-party node that false-fail-stopped (bumping the epoch)
181-
* then recovered leaves the CSSD hysteresis DEAD→ALIVE, so the others-dead
182-
* bitmap returns to its bound value while the scalar dead_generation (which
183-
* only advances) does not. If the leaver's first epoch>baseline observation
184-
* lands after that rebound, a bitmap-only check would mis-latch a leave the
185-
* survivor coordinator actually refused. The scalar conjunct closes it — and
186-
* it does NOT reintroduce the ②b false positive, because the leaving node's
187-
* OWN alive→DEAD transition never bumps ITS OWN dead_generation (a node does
188-
* not observe itself dead), so on the leaver side dead_gen stays at baseline
189-
* through its own drain. (The survivor-side coherence sites keep the
190-
* others-dead bitmap: a survivor DOES observe the leaver's DEAD and would be
191-
* falsely escalated by the scalar there.)
181+
* History of the two inference failure modes this replaces:
182+
* - r2 P2-1 (mis-latch wedge): the pre-r2 "epoch advanced + others-dead
183+
* bitmap unchanged" inference could mis-latch a REFUSED leave after a
184+
* third-party false-DEAD -> ALIVE rebound restored the (non-monotone)
185+
* bitmap, suppressing the deadline escalation forever.
186+
* - r3 (t/331 C1/C4 false-escalation): the r2 scalar dead_generation
187+
* conjunct is monotone the other way — a third-party transient flap on
188+
* the leaver's local CSSD view advances it forever, so a healthy
189+
* committed leave was refused until the deadline escalated it.
190+
* Marker evidence is immune to both: a refused leave never gets a COMMITTED
191+
* marker (no latch -> bounded deadline escalation), and a flap cannot make
192+
* durable evidence disappear (latch -> no false escalation).
193+
*
194+
* others_dead_unchanged / dead_gen_unchanged are the leaver's live coherence
195+
* observations. They are deliberately kept in the signature as contract
196+
* inputs that MUST NOT affect the verdict — the U3b unit matrix pins the
197+
* flap-immunity on them — and the runtime uses them only for the flap-noise
198+
* LOG at the latch point (observability, never control flow).
192199
*/
193200
bool
194-
cluster_clean_leave_own_commit_latched(bool epoch_advanced, bool others_dead_unchanged,
201+
cluster_clean_leave_own_commit_latched(bool committed_marker_evidence, bool others_dead_unchanged,
195202
bool dead_gen_unchanged)
196203
{
197-
return epoch_advanced && others_dead_unchanged && dead_gen_unchanged;
204+
(void)others_dead_unchanged; /* observability-only input (see above) */
205+
(void)dead_gen_unchanged; /* observability-only input (see above) */
206+
return committed_marker_evidence;
207+
}
208+
209+
/*
210+
* cluster_clean_leave_committed_evidence_matches -- may the leaving node
211+
* accept a LEAVE_COMMITTED confirmation as marker evidence for THIS leave
212+
* attempt? (spec-2.29a r3; the payload's magic/version/CRC were already
213+
* checked by cluster_clean_leave_announce_payload_valid.)
214+
*
215+
* Identity is bound three ways, all fail-closed:
216+
* - payload_leaving_node == self_node: the confirmation is addressed to
217+
* this node's own leave (LEAVE_COMMITTED is point-to-point, but a
218+
* misrouted frame must still not latch).
219+
* - current_leaving_node == self_node: this node IS currently the leaver
220+
* (not idle, not a survivor of someone else's leave).
221+
* - payload_nonce == current_attempt_nonce: the per-attempt nonce
222+
* (Hardening v1.0.2) pins the confirmation to THIS attempt, so a stale
223+
* LEAVE_COMMITTED — and through it a stale COMMITTED marker — from a
224+
* PREVIOUS leave of the same node can never false-latch a new attempt.
225+
* - payload_epoch > bound_leave_epoch: the committed epoch E the
226+
* coordinator attests must lie past the baseline this leave bound
227+
* (sanity; the commit is a guarded CAS off that baseline).
228+
*/
229+
bool
230+
cluster_clean_leave_committed_evidence_matches(int32 payload_leaving_node, uint64 payload_nonce,
231+
uint64 payload_epoch, int32 self_node,
232+
int32 current_leaving_node,
233+
uint64 current_attempt_nonce,
234+
uint64 bound_leave_epoch)
235+
{
236+
if (payload_leaving_node != self_node)
237+
return false;
238+
if (current_leaving_node != self_node)
239+
return false;
240+
if (payload_nonce != current_attempt_nonce)
241+
return false;
242+
if (payload_epoch <= bound_leave_epoch)
243+
return false;
244+
return true;
198245
}
199246

200247

0 commit comments

Comments
 (0)