mirror of
https://github.com/block/buzz.git
synced 2026-11-12 20:31:12 +00:00
6878520604
## What This is phase 1 of 2 for retiring the `events_created_at_floor` deferred constraint trigger and its `events_created_at_floor_guard()` function (migration 0021). The invariant now holds in the app writers. **The trigger, function and `buzz.created_at_floor` GUC arming all stay** as the backstop while pods run mixed versions. A later PR drops them. One helper, `replica_fence::guard_event_created_at_floor(&mut AdmittedTx, created_at, channel_id)`, runs on the admitted write connection. If `created_at < clock_timestamp() - CREATED_AT_FLOOR_SECS`, it rejects the write with the typed, non-retryable `DbError::CreatedAtFloorRejected`. It uses the same 960 s constant as the GUC and the fence wall. Channel-less rows skip it and cost no query. **Why checking at insert time is enough:** the fence wall is `min(oldest_xact_start, S) - floor - 5s`. A writer transaction that is open during the probe's activity scan is bounded by its own `xact_start`. A transaction that starts after the scan samples a clock later than `S`. So any `clock_timestamp()` sample taken inside the writing transaction is as strong as the trigger's commit-time sample. This needs no commit hook, no candidate-row tracking and no ordering against the TTL refresh, and `AdmittedTx::commit` is unchanged. | Site | Check placement | |---|---| | `store/event.rs` `insert_event_on` | after the `ON CONFLICT DO NOTHING` INSERT, only when a row was inserted, before follow-ups | | `store/event.rs` `insert_event_with_thread_metadata_tx` | after the INSERT, only when a row was inserted, before follow-ups and thread metadata | | `store/replaceable.rs` parameterized replacement | after the `ON CONFLICT DO NOTHING` INSERT, only when a row was inserted, inside the supersede savepoint (a rejection rolls back to it and restores the old head) | | `store/replaceable.rs` addressable replacement | after the `ON CONFLICT DO NOTHING` INSERT, only when a row was inserted, before follow-ups (the caller's rollback restores the old head) | | `store/channel_members.rs` `LockedMemberSnapshot::replace_member_event` | before the old head is superseded | **Writer inventory:** there are 7 production `INSERT INTO events` functions. The other two are structurally `channel_id = NULL`: `push.rs` `accept_lease_event` (kind 30350) and `relay_members.rs` `publish_nip43_membership_locked` (kind 13534). Both write a literal `NULL` for `channel_id` in their INSERT SQL, and the policy exempts them only while that NULL sits at the `channel_id` position. No production UPDATE writes `events.created_at` or `events.channel_id`, and the events `ON CONFLICT` clauses are `DO NOTHING`. **Client-visible outcome is unchanged.** The trigger's SQLSTATE 23514 surfaced as a generic DB error, which the relay maps to `IngestError::Internal` (WS `OK false` / HTTP 500). The new variant follows the same path. Relay mappings are untouched. **`scripts/seed-admin-dashboard.sh`** writes a channel event with `created_at = 1790000000` through raw psql, and it accepted any `PGHOST`. It now refuses non-local hosts (`localhost`, `127.0.0.1`, `::1` and Unix socket paths are still allowed), a non-empty `PGHOSTADDR` or `PGSERVICE`, a multi-host `PGHOST`, and any `PGDATABASE` that is not a plain name (psql treats URI and `key=value` `-d` values as connection strings that override `-h`). Both psql paths pass `-X`, so a psqlrc can't `\connect` elsewhere after the guard. ## Tests There are eleven new PG tests in `store::event_floor_postgres_tests`. Each runs in its own per-test database with `ALTER TABLE events DISABLE TRIGGER events_created_at_floor`, and a `pg_partition_tree` check asserts that every partition clone is disabled, so the trigger can't hide a missing app check. - `insert_event_floor_rejects_old_and_accepts_fresh`, `thread_metadata_event_floor_rejects_old_and_accepts_fresh`, `parameterized_event_floor_rejects_old_and_accepts_fresh`, `addressable_event_floor_rejects_old_and_accepts_fresh`, `member_snapshot_event_floor_rejects_old_and_accepts_fresh`: each rejects a too-old event with the typed error, confirms no row was written, then accepts a fresh event. - `insert_event_floor_accepts_expired_duplicate`, `thread_metadata_event_floor_accepts_expired_duplicate`: replaying an already-stored event whose `created_at` is now below the floor returns `Duplicate` (inserted = false), leaves one row and runs no thread follow-ups. - `parameterized_replay_of_soft_deleted_aged_event_is_a_duplicate`, `addressable_replay_of_soft_deleted_aged_event_is_a_duplicate`: replaying a replacement whose stored row is soft-deleted and has aged below the floor returns a duplicate, not a floor rejection, as the trigger did (it never fired on `ON CONFLICT`). - `cluster_global_event_floor_held_transaction_cannot_write_below_probe_wall`: runs a real heartbeat probe while a writer transaction is open. It asserts that the wall is bounded by that writer's `xact_start`, that a below-wall event is rejected, and that a near-floor event sampled after the probe lands above the wall. - `event_floor_samples_statement_clock_after_transaction_start`: waits on the database until a boundary timestamp that was valid at BEGIN expires (normally ≤1.1 s), so `now()` would pass where `clock_timestamp()` fails. Source policy (`observability_source.rs`): every channel-bearing production events INSERT in any crate's `src` must call the helper, no production UPDATE may assign `created_at` or `channel_id` (including `ONLY`, schema-qualified, aliased and row-constructor forms; quoted identifiers are normalized), the helper must use `clock_timestamp()` and the shared constant, and the seed script must refuse a remote host. Negative fixtures cover each rule. Each check was confirmed to fail when its fix is removed. All of these mutants failed on assertions: - removing the helper from all five writers, which failed all seven PG tests - switching `clock_timestamp()` to `now()`, which failed both the clock PG test and the clock policy - removing one call - adding a `created_at` UPDATE - disabling the policy self-test - removing the seed guard - (review round 2) restoring the pre-INSERT check placement, which failed both duplicate-replay tests; the round-1 UPDATE matcher, quoted-identifier normalization, buzz-db-only scan, NIP-43 bind exemption, process-wide canvas base and each seed override were each restored and failed their tests - (review round 3) restoring the round-2 seed script, which failed the extended seed test on its URI and key/value `PGDATABASE` cases; restoring the script without `-X`, which failed the control's `-X` assertion **Fixture changes:** about 40 existing buzz-db PG tests seeded historical rows (fixed old timestamps) through the serving writers. The trigger never fired on them because the test pool is unarmed. They now seed through a `#[cfg(test)]` raw-SQL helper, `test_support::insert_historical_event` (and `insert_historical_event_with_thread_metadata` for thread fixtures), which keeps admission, follow-ups and mentions. The canvas CAS fixtures keep their logical seconds and ties but each test passes its own fresh base. The existing trigger backstop tests still exercise raw armed INSERT/UPDATE plus COMMIT returning 23514. The new PG tests are selected by the `postgres-ci` filter `(test(/postgres_tests::/) or binary(/^postgres_/)) and not test(/(^|::)external_infra[^:]*::/)`. The `cluster_global_` test also joins the existing serial group. ## Behavior notes - Prepared transactions (2PC) are covered only by the retained trigger, which runs at PREPARE. The insert-time app guard runs earlier, and no app writer issues `PREPARE TRANSACTION`. The comment in `replica_fence.rs` now says so, so the trigger drop has to keep this case covered. - One extra round trip for each newly inserted channel event. Duplicate replays add none. - Ordinary and thread event writes check the floor only for newly inserted rows, before follow-ups, mentions or thread metadata. Replaying an already-stored id stays `Duplicate` even when its timestamp has aged below the floor, as with the trigger. (Internal writers such as the admin outbox worker do replay relay-signed events with their original `created_at`.) A too-old new row returns the non-retryable `CreatedAtFloorRejected`, and the owning transaction rolls back; every current caller propagates the error and drops the transaction. The transaction-composition seam documents that contract. - Replacement writers check after their INSERT, only when it inserted a row, so a replay of any stored id (live or soft-deleted) stays a duplicate. The locked roster writer checks before the old head is superseded. - The trigger is stricter for a row that ages below the floor between the app check and COMMIT. The proof above doesn't need that strictness, and phase 2 gives it up. ## What got simpler The NIP-43 insert drops a bind for a literal `NULL`, the floor and follow-up policies share one workspace walk, and the canvas fixtures lose their process-wide `OnceLock`. No commit-time machinery was added: no candidate tracking, commit hook, cleanup protocol or new lock. Test-only historical writes now go through one helper instead of being scattered through the serving APIs. Nothing could be deleted in this phase because mixed-version pods still rely on the trigger. **Phase 2** drops the trigger and function, the GUC arming, `verify_floor_guard_catalog` / `verify_floor_guard_behavior` and the schema reconciliation entries. Its deploy gate: the whole fleet on this binary, no rollback expected, and the replica breaker held closed through the drop. **Identified debt (pre-existing, outside this diff):** three `buzz-relay` `handlers::group_state_postgres_tests::*` tests fail with `ensure community: Sqlx(PoolTimedOut)` on the Blox host. They fail the same way on a clean checkout of the base (`state.rs` test pool, 100 ms acquire timeout). `writer_pool_rejects_non_read_committed_database_default` and `writer_pool_metrics_stop_after_session_timeout_setup_failure` hang on that host at base too. ## Verification Head `b046df76d` (after the placement fix and main's newer test fixtures): on Blox, fmt, clippy and buzz-db unit (241/241) pass, and a filtered buzz-db PG lane over the floor tests and every converted fixture passes 88/88. CI is running at this head. Earlier full gate, at tree `67c4fe5e1` + this diff, before the placement fix: | Gate | Result | |---|---| | `cargo fmt --all -- --check` | pass | | `cargo clippy --workspace --all-targets -- -D warnings` | pass | | `cargo nextest run -p buzz-db --lib` | 167/167 | | `cargo nextest run -p buzz-db --test observability_source` | 22/22 | | `scripts/test-postgres-test-discovery.sh` | pass | | `scripts/postgres-test-run.sh -p buzz-db` (excluding the two host hangs) | 517/517 | | `scripts/postgres-test-run.sh -p` buzz-admin / audit / deletion / push-gateway / search / workflow | all rc 0 (4, 7, 11, 11, 20, 2) | | `scripts/postgres-test-run.sh -p buzz-relay` | 402 pass, 3 fail (the pre-existing `PoolTimedOut` failures above) | Local gates used a reverted `rustls` lock bump, because the build mirror lacks 0.23.46. The committed `Cargo.lock` is unchanged, and CI uses the real lock. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Signed-off-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>