Skip to content

fix(db): label routes replica_is_writer when READ_DATABASE_URL is not a standby - #4014

Open
rshmdev wants to merge 1 commit into
block:mainfrom
rshmdev:fix/db-reader-is-writer-telemetry
Open

fix(db): label routes replica_is_writer when READ_DATABASE_URL is not a standby#4014
rshmdev wants to merge 1 commit into
block:mainfrom
rshmdev:fix/db-reader-is-writer-telemetry

Conversation

@rshmdev

@rshmdev rshmdev commented Jul 31, 2026

Copy link
Copy Markdown

Summary

Pointing READ_DATABASE_URL at the writer is accepted silently, and route telemetry then reports decision="replica" for pages the writer served. The freshness fence is trivially satisfied in that configuration — the "replica" has zero lag because it is the writer — so nothing is unsafe and no wrong rows are served. The damage is observability: decision="replica" is the offload percentage on the rollout dashboards, so an operator reads a healthy offload while every byte still comes off the writer.

What this changes:

  • replica_fence::cluster_identity() reads pg_is_in_recovery() and, when the role is permitted, pg_control_system().system_identifier for one session.
  • Db::spawn_read_pool_boot_ping settles a cached verdict on the reader connection it already holds (no extra acquire on the read path), and WARNs when the reader endpoint is not a standby, reporting both identifiers and whether they match.
  • While that verdict stands, pages the reader serves are labeled buzz_db_route_decision{decision="replica_is_writer"} instead of decision="replica", so decision="replica" stays a truthful offload signal. The reason label (fresh/covered) is unchanged — the predicate verdict is orthogonal to where the page came from.

Routing behavior is untouched: identity is evidence, never a gate — the same posture as the existing Aurora identity probe.

Three decisions worth a reviewer's eye

1. The gate is pg_is_in_recovery(), not the identifier match the issue suggested. system_identifier comes from pg_control_system(), which is superuser-only by default; requiring a match would make this silently no-op on every deployment where the relay's role is not a superuser. A reader that is not replaying WAL is not a replica whatever its identifier says, so the recovery flag alone is the gate, and the identifier is read when permitted and logged to separate "the same node as the writer" from "some other primary" — both of which offload the writer just as little. That does mean an unrelated non-standby primary is also labeled replica_is_writer; I kept the label name from the issue rather than inventing one, but happy to rename if you'd rather the label describe the gate.

2. Warn-only, not fail-closed. The issue offered refusing the reader pool as an alternative. I didn't take it: it trades a reporting defect for lost capacity, and it would latch the wrong answer on Aurora, where cluster-ro legitimately resolves to the writer while a cluster has no readers and starts resolving to a real reader the moment one is added.

3. A new decision value, not a new reason. ⚠️ Dashboards that assume decision ∈ {replica, writer} need the new value added. Putting it in reason instead would have left decision="replica" counting writer-served pages, which is the defect. buzz_db_route_decision appears nowhere else in this repo, so nothing here needed updating — the rollout dashboards live in block-coder-tf-stacks.

Also boot-only, and only on the boot ping's successful path: a reader unreachable at boot leaves the verdict unset and routes are labeled replica as before. Re-probing per request would put a privileged catalog call on the read path to serve a dashboard.

Related issue

Fixes #3644.

Duplicate check: no open PR touches reader identity or replica route telemetry. Closest neighbours are #3643 (NIP-50 search pool bypassing replica fencing) and #3622 (CI never runs the replica-routing Postgres fixtures), both split out of the same redteam thread as this issue; neither overlaps. Builds on #3268 (merged).

Testing

cargo test -p buzz-db --lib                      # 95 passed, 154 ignored
cargo test -p buzz-db --lib -- --ignored         # 151 passed serially, 3 pre-existing failures (see below)
cargo clippy -p buzz-db --lib --tests -- -D warnings
cargo fmt -p buzz-db -- --check

New tests:

  • cluster_identity_compares_only_when_both_identifiers_are_readable — no infrastructure. Pins that an unreadable identifier reports unknown, never different, so a non-superuser role is not misdiagnosed as a distinct cluster.
  • cluster_identity_reads_a_primary_with_or_without_control_privileges — Postgres. Ok either way, identifier present or None, and the privilege error must not poison the session for the statements the boot ping runs next.
  • reader_pointing_at_the_writer_is_labeled_replica_is_writer — Postgres. Both URLs point at the same scratch database, which is the misconfiguration rather than a stand-in for it. Asserts the label before probing too, so a constructor that prejudged every reader would fail the test rather than pass it.

The two Postgres-gated tests carry the crate's existing #[ignore = "requires Postgres"], so per #3622 they will not run in CI — they need just test (or cargo test -p buzz-db --lib -- --ignored) locally.

Existing routing coverage re-run green: count_events_routed_is_bounded_only, query_events_routed_defaults_dark_and_routes_covered_when_enabled, head_fetch_routes_by_configured_budget, channel_window_routes_head_to_writer_and_cursor_pages_to_replica, routed_fallback_spends_one_acquire_budget_when_aurora_cache_is_cold, thread_replies_cursor_pages_route_to_replica_with_writer_terminal_verification, routed_reads_are_confined_to_the_requested_community, routed_request_holds_one_snapshot_across_page_and_aux, plus the six replica_fence tests.

Three failures in the full --ignored run are pre-existing and unrelated (create_community_with_owner_enforces_per_owner_limit, transfer_ownership_returns_limit_reached_for_maxed_transferee, duplicate_event_keeps_first_community_provenance) — they depend on residual per-owner state in the shared dev database and reproduce identically with this branch stashed. A further 12 pass serially but fail at --test-threads 4, the same shared-database interference as #3929 / #2458.

No UI change, so no screenshots.

… a standby

Pointing READ_DATABASE_URL at the writer was accepted silently, and route
telemetry then reported decision="replica" for pages the writer served. The
freshness fence is trivially satisfied in that configuration -- the "replica"
has zero lag because it IS the writer -- so nothing is unsafe, but
buzz_db_route_decision becomes semantically misleading: operators watching the
rollout dashboards read decision="replica" as the offload percentage and would
believe replica offload is working while every byte still comes off the writer.

The boot ping now reads cluster identity on the reader connection it already
holds and settles a cached verdict. Pages the reader serves are labeled
decision="replica_is_writer" while that verdict stands, so decision="replica"
stays a truthful offload signal. The reason label (fresh/covered) is unchanged.

Warn-only by design. Refusing the reader pool would trade a reporting defect
for lost capacity, and it would latch the wrong answer on Aurora, where
cluster-ro legitimately resolves to the writer while a cluster has no readers
and starts resolving to a real reader as soon as one is added.

The gate is pg_is_in_recovery() on the reader rather than an identifier match:
system_identifier comes from pg_control_system(), which is superuser-only by
default, so requiring a match would silently no-op wherever the relay's role
is not a superuser. The identifier is read when permitted and reported in the
warning to separate "the same node as the writer" from "some other primary",
both of which offload the writer just as little.

Refs block#3644

Signed-off-by: Rian <fortpcnite21@gmail.com>
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.

Writer-pointing READ_DATABASE_URL accepted silently; route telemetry reports replica/fresh from the writer

1 participant