Skip to content

feat(postgres): add stock-PostgreSQL storage backend (KPEP-0001)#5

Open
cc-ocelotai wants to merge 1 commit into
kplane-dev:mainfrom
cc-ocelotai:feat/postgres-backend
Open

feat(postgres): add stock-PostgreSQL storage backend (KPEP-0001)#5
cc-ocelotai wants to merge 1 commit into
kplane-dev:mainfrom
cc-ocelotai:feat/postgres-backend

Conversation

@cc-ocelotai

Copy link
Copy Markdown

Summary

Adds a PostgreSQL implementation of storage.Interface behind --storage-backend=postgres, alongside the existing etcd3 and Spanner backends. It targets stock PostgreSQL, no extensions, no superuser, no replication slots, no server configuration changes, so it runs unmodified on a managed instance (RDS, Cloud SQL, Neon) or a laptop docker run postgres.

Registration is the one-line change KPEP-0001 promised: a single b.Register(postgres.NewOptions()) in backends/register.go. Nothing outside backends/postgres/ changes, and the apiserver needs no code change to select it.

It passes the full applicable upstream conformance battery (23 cases, 0 failures, 1 self-skip, the compaction case, same as Spanner/Cockroach) and is verified against a live PostgreSQL 16.

Motivation

KPEP-0001 establishes the pluggable-backend registry and argues we'll want "at least three backends (Spanner, kine -> postgres for ergonomics-first deployments, postgres-native for perf-sensitive ones)." This is that third one, built as a direct storage.Interface implementation rather than a kine shim.

Why not just use kine-over-Postgres? The kplane-kine benchmarks already in the KPEP make the case: 1.5–3× lower throughput, 2–14× wider p99, and a 1-second watch floor from the polling driver. Postgres is the most widely operated database on earth; a first-class backend that talks to it directly, with etcd-class watch latency and no operational prerequisites, is worth the implementation cost. Spanner proved the direct-storage.Interface approach; this extends it to the database most teams already run.

Design, and why it deliberately diverges from the Spanner/Cockroach shape

This is the part worth reviewers' attention. The Spanner and Cockroach backends keep one row per key and lean on the storage engine for two things PostgreSQL simply does not expose:

Obligation etcd3 Spanner Cockroach Stock PostgreSQL
Monotonic resource version raft index TrueTime commit ts HLC ❌ no client-visible commit clock
Point-in-time reads mvcc revision ReadTimestamp bound AS OF SYSTEM TIME ❌ no time-travel at all
Ordered change stream native watch in-proc broadcaster CREATE CHANGEFEED ❌ no native CDC
Write-conflict retry txn RW txn crdbpgx.ExecuteTx SERIALIZABLE + 40001 retry

Because Postgres has neither a commit clock nor time-travel, the single-row model can't work here. The idiomatic answer, the one the Kubernetes ecosystem already validated via kine, is an append-only MVCC log: every mutation INSERTs a new row, and the row's BIGSERIAL id is the resource version. That one decision discharges three obligations at once:

  • Resource versions are the id sequence, monotonic by construction.
  • Point-in-time reads are reconstructed from retained history with Postgres's own DISTINCT ON (name) … WHERE id <= rv ORDER BY name, id DESC. No engine time-travel required, we keep the history and query it.
  • Watch is a resumable tail of the log (WHERE id > cursor), which is durably replayable across restarts because the cursor lives in the log's own ids.

The remaining sub-decisions, each with its rationale:

Watch transport: poll-the-log + LISTEN/NOTIFY, not logical replication.
Logical replication would be the closest analog to Cockroach's changefeed, but it requires wal_level=logical, replication privileges, and slot lifecycle management, and an unconsumed slot pins WAL and can fill the disk. That's the wrong default for a backend meant to run anywhere. Instead a single per-process reader polls the log; LISTEN/NOTIFY (via an AFTER INSERT trigger) is only a sub-100ms wakeup, and a 1s timer is the backstop. The poll is authoritative, so a missed notification loses nothing. the next poll from the cursor recovers it. This also makes the feed multi-writer correct (it sees every apiserver's rows), which the in-process broadcaster approach is not.

Gap-safe watermark, the one genuinely subtle piece. A BIGSERIAL
sequence hands out ids at INSERT time, but transactions commit in an unrelated order: writer A can reserve id=5 while B reserves id=6 and commits first. A naive poller reading WHERE id > cursor ORDER BY id would
deliver 6, advance its cursor past 5, and silently drop 5 when A commits. This is the same hazard the Spanner broadcaster's ticket/heap machinery and Cockroach's resolved-timestamp both exist to close. Here it's closed the
stock-Postgres way: the feed only delivers up to a safe watermark derived from transaction visibility, the highest id all of whose predecessors have committed, computed from txid_snapshot_xmin(txid_current_snapshot()). It is,
functionally, a resolved timestamp built from xids instead of a clock. See currentSafeRV in rv.go and the delivery loop in feed.go.

Writes run SERIALIZABLE with an automatic 40001/40P01 retry (retry.go)
— stock-Postgres's answer to crdbpgx.ExecuteTx. The append-only model needs it for correctness, not just throughput: two racing Creates of one key must not both observe "no live row," and serializable isolation + retry is what makes the loser see the winner's row and return KeyExists.

TTL is a scanner that appends tombstones (ttl.go). Postgres has no
built-in row-level TTL, so the scanner is the only expiry path, but it fits the log model cleanly: it INSERTs a tombstone (never a physical DELETE), the feed picks it up on the next poll, and watchers see an ordinary DELETED. A NOT EXISTS (newer row) guard plus the serializable re-read is the CAS that keeps a concurrently-renewed lease from being wrongly deleted.

name TEXT COLLATE "C". Keys are byte strings and every prefix scan
(name >= start AND name < prefixEnd(start)) assumes byte ordering. Postgres's default locale collation reorders punctuation like /, which silently breaks every prefix list. COLLATE "C" forces bytewise comparison; the (name, id DESC) index inherits it. Calling this out explicitly because it's invisible in review and only surfaces against a real server (it did, see Testing).

Non-goals / deliberately deferred

Scoped out to keep this reviewable, each a clean follow-up rather than a gap:

  • Compaction. The log grows unbounded; CompactRevision() returns 0 for now (matching Spanner and Cockroach). The schema and RV math already leave the seam. A background compactor that prunes superseded rows below a horizon is the obvious next PR.
  • Logical-replication watch. Rejected as the default above; could be an opt-in transport later for deployments that already run wal_level=logical and want to shed the poll.
  • A server-version probe. The backend assumes a compatible Postgres; it doesn't defensively check the version at startup.
  • Multi-writer (HA apiserver) stress. The design is multi-writer correct, but this PR validates it via the conformance suite on a single writer; a concurrent-writer soak is worth adding before we lean on HA.

Testing

  • Upstream conformance: the RunTest* battery from k8s.io/apiserver/pkg/storage/testing, the same suite etcd3 itself is held to, delegated case-by-case in conformance_test.go. 23 pass, 0 fail, 1 self-skip. The skip is WatchFromZero, which self-skips when the compaction callback is nil; Spanner and Cockroach skip it identically.
  • A real bug the live run caught: recursive GetList returned empty despite correct rows, because the DB's default collation isn't byte-ordered and broke every prefix range. Fixed with COLLATE "C" (above). Called out because it's exactly the class of defect that passes code review and fails in production.
  • Flake check: the watch-path cases run clean across repeated -count=3 runs; watch latency is NOTIFY-driven, not bound to the poll backstop.
  • Environment: developed and verified against PostgreSQL 16. The backend uses only long-standing features (aggregate FILTER, the txid snapshot helpers, DISTINCT ON, COLLATE "C"), so PostgreSQL 12+ is the intended floor, not yet CI-verified on older majors.

Compatibility & operations

  • No special Postgres configuration. No wal_level change, no replication role, no extensions. A default docker run postgres (or managed instance) works as-is.
  • Flags: --postgres-dsn (required), plus --postgres-database, --postgres-max-conns, --postgres-max-conn-lifetime, --postgres-max-conn-idle-time, --postgres-health-check-period.
  • One reader connection outside the pool for LISTEN (a connection blocked in WaitForNotification can't be pooled); everything else uses the shared pgxpool.

How it's laid out

Mirrors the Cockroach/Spanner file structure 1:1 so it reads the same way.
~2,350 source lines + ~300 test lines.

File Role
doc.go Package doc + the append-log rationale
config.go pgxpool construction, DSN, defaults
schema.go Append-only log table, indexes, NOTIFY trigger, init sentinel
retry.go SERIALIZABLE + 40001/40P01 retry helper
rv.go id<-> RV, the gap-safe watermark, decode helpers
store.go Create / Get / Delete
list.go GetList, DISTINCT ON snapshot reads, paging
update.go GuaranteedUpdate, optimistic concurrency
feed.go Log poller, LISTEN/NOTIFY wakeup, watermark, fanout
watcher.go watch.Interface over a feed subscriber
ttl.go Tombstone-appending TTL scanner
factory.go / factory_backend.go Shared runtime (pool + feed + scanner)
register.go Options implementing registry.Backend
stubs.go Stats, ReadinessCheck, RequestWatchProgress, …
conformance_test.go, testutil_test.go Suite wiring + fresh-DB harness

Suggested reading order for review: doc.go -> schema.go -> rv.go (the watermark) -> feed.go (the watch kernel) -> store.go/update.go, then the wiring. I'm happy to split this into the same small, self-contained commits the Cockroach PR used if that's easier to walk.

Test plan

  • Reviewer boots a local Postgres (docker run -d -e POSTGRES_HOST_AUTH_METHOD=trust -p 5432:5432 postgres:16)
  • POSTGRES_TEST_DSN=postgres://postgres@localhost:5432/postgres?sslmode=disable go test ./backends/postgres/...
  • Spot-check watch stability: go test ./backends/postgres/... -run TestConformance_Watch -count=3
  • Optional apiserver e2e per the KPEP-0001 recipe with --storage-backend=postgres --postgres-dsn=…

Open questions for reviewers

I'd genuinely value a second opinion on the two load-bearing calls:

  1. Watch transport. I chose poll + LISTEN/NOTIFY over logical replication for zero-config portability and durable resume. If we expect deployments to already run wal_level=logical, an opt-in logical-replication transport could be a worthwhile follow-up, but I don't think it should be the default.
  2. Watermark. The txid_snapshot_xmin approach is correct and index-cheap on the recent tail, but it's the subtlest code in the PR. Extra eyes on currentSafeRV and the feed.poll delivery bound are welcome.

The append-only-log-over-SQL pattern is also distinct enough from the single-row engine-MVCC backends that a short design note (or a KPEP-0001 addendum) may be worth writing, so the next SQL backend, kine-native, MySQL, whatever, follows the same shape deliberately rather than by archaeology. Happy to author that if maintainers agree it's warranted.

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