Skip to content

Fix race in primary election. - #371

Merged
NullHypothesis merged 3 commits into
masterfrom
fix-race
Aug 31, 2026
Merged

Fix race in primary election.#371
NullHypothesis merged 3 commits into
masterfrom
fix-race

Conversation

@NullHypothesis

@NullHypothesis NullHypothesis commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

The methods Init and PostInit each decided primary-vs-standby by reading the Consul "cluster initialized" flag and acting on it separately, with no atomicity between the two. When nodes booted close together, more than one could see the flag unset and independently self-initialize as primary, leaving a node with a fresh, credential-less local database that then failed forever trying to register as a standby against the real primary (repmgr/flypgadmin roles don't exist).

Replace both checks with Store.TryClaimPrimary, a single atomic Consul CAS (ModifyIndex: 0) keyed by machine ID, so exactly one node ever wins the claim, and a node retrying after its own crash can tell it already won rather than deferring to itself as "someone else."

The methods `Init`` and `PostInit`` each decided primary-vs-standby by
reading the Consul "cluster initialized" flag and acting on it
separately, with no atomicity between the two. When nodes booted close
together, more than one could see the flag unset and independently
self-initialize as primary, leaving a node with a fresh, credential-less
local database that then failed forever trying to register as a standby
against the real primary (repmgr/flypgadmin roles don't exist).

Replace both checks with `Store.TryClaimPrimary``, a single atomic
Consul CAS (ModifyIndex: 0) keyed by machine ID, so exactly one node
ever wins the claim, and a node retrying after its own crash can tell it
already won rather than deferring to itself as "someone else."
@NullHypothesis

Copy link
Copy Markdown
Contributor Author

I'm going to ignore the "cyclomatic complexity" complaint because the PR isn't making anything worse. In fact, it's removing a bit of code.

@NullHypothesis

Copy link
Copy Markdown
Contributor Author

Here's how I tested this:

This command currently fails for me. The leader passes all its health checks and thinks it's the leader but the other two machines (or at least one other machine) often disagree.

fly pg create --flex --name myapp-bad --initial-cluster-size 3 --vm-size shared-cpu-1x --volume-size 1

Using the patched postgres-flex image (I built it locally and manually uploaded to our registry), the command succeeds:

fly pg create --flex --name myapp-good --initial-cluster-size 3 --vm-size shared-cpu-1x --volume-size 1 --image-ref registry.fly.io/philipp-test-pg:my-fix-test

@NullHypothesis
NullHypothesis merged commit f8060b1 into master Aug 31, 2026
4 of 5 checks passed
@NullHypothesis
NullHypothesis deleted the fix-race branch August 31, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant