Skip to content

fix(postgres): recover heartbeat election after connection loss - #3451

Open
kbrwn wants to merge 3 commits into
authzed:mainfrom
kbrwn:main
Open

kbrwn wants to merge 3 commits into
authzed:mainfrom
kbrwn:main

Conversation

@kbrwn

@kbrwn kbrwn commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Description

After a PostgreSQL restart or failover, a heartbeat follower can repeatedly retry election on a closed connection and never become leader without restarting SpiceDB.

Acquire a connection for each election attempt and release it when unsuccessful, allowing the pool to replace closed connections. Existing heartbeat writes and retry timing are preserved.

Testing

Added a regression test that terminates a follower’s election connection and verifies that it reconnects, acquires leadership, and resumes heartbeats. The test fails before the fix and passes afterward with the race detector:

POSTGRES_TEST_VERSION=16-alpine go test -race -tags='datastore postgres' \
  ./internal/datastore/postgres \
  -run '^TestRevisionHeartbeatFollowerReconnects$' -count=1 -v

Also verified recovery using two local SpiceDB instances and PostgreSQL 16 in Docker, testing both follower connection termination and a database restart.

@kbrwn
kbrwn requested a review from a team as a code owner November 3, 2026 21:27
@github-actions github-actions Bot added area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Nov 3, 2026
@codecov

codecov Bot commented Nov 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

tstirrat15
tstirrat15 previously approved these changes Nov 4, 2026

@tstirrat15 tstirrat15 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.

LGTM, see comment

engine := testdatastore.RunPostgresForTesting(t, postgresTestVersion(), false)
uri := engine.NewDatabase(t)
ctx, cancel := context.WithCancel(t.Context())
defer cancel()

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.

pref t.Cleanup

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants