Skip to content

Fix active active - #225

Closed
matthewcooper wants to merge 472 commits into
masterfrom
fix-active-active
Closed

Fix active active#225
matthewcooper wants to merge 472 commits into
masterfrom
fix-active-active

Conversation

@matthewcooper

Copy link
Copy Markdown
Collaborator

Summary

Root cause: TOCTOU race between CreateCluster and InitiateJoin gRPC handlers running as concurrent goroutines on node 2.

Your setup process sent both pulsectl cluster create and pulsectl cluster join to node 2 within the same second. Both handlers check ClusterCheck() (len(config.Nodes) > 0) at the start — but both saw 0 nodes because neither had finished
writing yet. Both passed the guard and proceeded. The join correctly set node 2 to Maintenance, but CreateCluster ran after and unconditionally forced Status = Active at server.go:3326, overriding it.

The log evidence was clear:

  • Node 2 line 52: generated its own cluster token — impossible if joining correctly
  • Node 2 line 60: "Member already exists" — the join had already added this node; CreateCluster ignored the warning and set Active anyway
  • Node 2 line 62: "First node activated successfully" — this code path should never run on a joining node

Fix: Added clusterInitMu sync.Mutex to Server. Both CreateCluster (line 3201) and InitiateJoin (line 4549) acquire it before their ClusterCheck() call. Whichever runs first completes normally; the second acquires the mutex and now sees
nodes > 0, returns "cluster is already configured" / "leave first", and exits without touching state.

Syleron and others added 30 commits May 1, 2020 09:15
Moved official plugins into core repo
resolved FIP not coming up
renamed internal to src to allow for referencing
The pulseha service is now called pulseha and the pulse command line …
Syleron and others added 29 commits March 11, 2026 15:27
END-1615: Added nil check to the exported methods of IPMonitor so tha…
…-in-pulseha

GRPC config, GRPC over unix socket and maintenance mode
…ng logic

- Introduced a `Maintenance` flag in the configuration for nodes to persist state across reboots.
- Improved safeguards to prevent peers from overriding local node maintenance state.
- Refactored maintenance mode handling for local and remote nodes, including quorum checks and failover logic.
- Updated gRPC and CLI to better support maintenance mode operations.
- Added a periodic gRPC-based deep check for cluster membership validation to detect split-brain scenarios.
- Enhanced the health check mechanism to differentiate between TCP connectivity checks and comprehensive membership validation.
- Updated health check logic to isolate nodes failing the deep check until they pass subsequent validations.
- Added cluster token validation to the gRPC health check handler.
…-on-active-node-does-not-update-if

Introduce deep cluster membership validation in health checks
Two follow-up fixes for the maintenance mode / Unix socket changes
introduced in PR #217:

- setMaintenanceLocal: the MakePassive demotion result was discarded.
  A transport error was logged at Warn level and the flow continued;
  a soft failure (Success=false response) was ignored entirely. The
  node would be marked maintenance while still holding its active IPs,
  leaving the cluster with no eligible active node. Abort maintenance
  entry and surface the error to the operator instead.

- Server.Start: the CLI Unix socket was removed unconditionally before
  Listen, which silently steals the socket out from under another
  pulseha daemon running on the same host. Dial-probe first; if a
  process is already listening, refuse to start.
…ocket-safety

Fail-fast on maintenance demote errors and protect CLI socket from steal
…stener

Two concurrent ConfigSync callbacks could each spawn a Reconfigure() in
their own goroutine. Each goroutine: snapshot grpcServer pointer briefly
under lock, GracefulStop the old server, create a new one, and then call
net.Listen("tcp", IP:9083). With no serialization between them and no
SO_REUSEADDR on the listener, the bind races and one or both attempts
fail with "address already in use", leaving the cluster listener torn
down and the node effectively unreachable from peers. We see this in
journals as repeated "Async reconfigure failed after ConfigSync" errors
followed by the joining node disappearing from the cluster mid-test.

- Add Server.reconfigureMu and acquire it for the duration of Reconfigure().
  Concurrent ConfigSync callbacks now run serially; the second sees the
  listener already at the desired address and the bind succeeds normally.
- Replace net.Listen with a small helper that sets SO_REUSEADDR (and
  SO_REUSEPORT on Linux) on the listening socket so a re-bind succeeds
  even if the previous socket is still in TIME_WAIT in the kernel.
- Add a Linux unit test that closes a listener and immediately re-binds
  the same address, covering the failure mode.
- Replace ineffective rebind smoke test with a deterministic getsockopt
  assertion that SO_REUSEADDR is actually set on the bound socket;
  SO_REUSEPORT is verified best-effort to tolerate legacy kernels and
  seccomp-restricted sandboxes.
- Relax the test build tag from linux-only to !windows so it runs on
  macOS dev machines too.
- Add tests/unit/reconfigure_concurrent_test.go covering the primary
  fix: 8 goroutines fire Reconfigure() through a barrier channel after
  CreateCluster brings up the listener on an ephemeral port; asserts
  all return nil (without reconfigureMu, one would EADDRINUSE).
- Capture s.grpcServer into a local pointer before spawning the Serve
  goroutine in startClusterListener so a concurrent Reconfigure swap
  doesn't race the goroutine's read (caught by -race in the new test).
- Rewrite drifting comments (reconfigureMu field, listenTCPReuseAddr,
  SO_REUSEPORT inline, Reconfigure doc) as present-tense invariants and
  drop PR-narrative phrasing; document reconfigureMu -> s.RWMutex lock
  ordering and the caller constraint.
- Log SO_REUSEPORT setsockopt failures at debug level instead of
  silently discarding errno, so legacy-kernel / seccomp issues remain
  traceable.
- Switch listenCtx cleanup to defer cancelListen() and wrap the listen
  error with %w for errors.Is compatibility.
fix(server): serialize Reconfigure and SO_REUSEADDR the cluster listener
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.

6 participants