Repository navigation
buzz-db: drop the events_created_at_floor trigger (BUZZ-316 phase 2) - #8326
Draft
TheSentinel454 wants to merge 10 commits into
Draft
TheSentinel454 wants to merge 10 commits into
TheSentinel454 wants to merge 10 commits into
Conversation
Every channel-bearing INSERT INTO events now checks created_at against database clock_timestamp() minus CREATED_AT_FLOOR_SECS on the admitted transaction before inserting, and returns the typed, non-retryable DbError::CreatedAtFloorRejected. The check samples the clock after BEGIN, so the replica-fence wall min(oldest_xact_start, S) - floor - margin still covers it. Migration 0021's deferred trigger and the writer-pool GUC stay in place as the mixed-version backstop until phase 2. A source policy requires the check on every channel-bearing events INSERT and forbids production UPDATEs of events.created_at/channel_id. The admin seed script now refuses non-local PGHOST. Refs BUZZ-316. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
TheSentinel454
force-pushed
the
elrond/drop-floor-guard
branch
from
October 14, 2026 18:12
82535d6 to
4e09720
Compare
Review rework for the insert-time created_at floor (BUZZ-316 phase 1). - Ordinary and thread event writers now run the floor check after their ON CONFLICT DO NOTHING INSERT, only when a row was inserted and before follow-ups. Replaying an already-stored id stays Duplicate even when its created_at has aged below the floor, matching the trigger. - Source policy: UPDATE rule catches ONLY, aliases and row-constructor column lists; quoted identifiers are normalized; the floor policy scans every crate's src via the shared workspace walk. - NIP-43 membership insert writes a literal NULL channel_id, pinned positionally; the bind-based exemption is gone. - Canvas CAS fixtures take a per-test base instead of a process-wide OnceLock. - seed-admin-dashboard.sh also refuses PGHOSTADDR, PGSERVICE and multi-host PGHOST. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
TheSentinel454
force-pushed
the
elrond/drop-floor-guard
branch
3 times, most recently
from
October 14, 2026 19:36
d530d30 to
89831dd
Compare
…ed guard psql -d treats URI and key=value values as full connection strings that override -h, so PGDATABASE could route the unarmed fixture writer to a shared database. Accept only a plain database name before either psql path, and extend the refusal test with URI and key/value cases (host=, hostaddr=, service=) plus a plain-name control that must reach psql. Also note in replica_fence that prepared-transaction coverage comes from the retained deferred trigger, not the insert-time app guard. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
added 4 commits
October 16, 2026 10:02
… guard psql -c no longer implies -X, so ~/.psqlrc or PSQLRC (exportable from .env) ran after the guard and could \connect to another host. Pass -X on both the local and Docker psql paths and assert the control invocation carries it. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Phase 2 of BUZZ-316. The insert-time application floor check (phase 1) is now the only created_at floor guard, so drop the deferred constraint trigger, its function, the writer-pool buzz.created_at_floor GUC, and the startup catalog/behavior probes that required them. Migration 0062 drops the trigger on events and every partition plus the function, all IF EXISTS. Desired-state schema, pgschema reconciliation and migration asserts drop the same objects. spawn_fence_probe becomes synchronous; the ordered heartbeat probe still fails closed. DESTRUCTIVE: deploy only after phase 1 runs on every writer, with no rollback expected, out-of-band writers stopped, and the replica breaker held closed through the drop until replay catches up. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Address review of the phase-2 floor retirement: - the workspace-wide floor source policy now comes from phase 1's shared crates/*/src walk (which pins buzz-relay coverage); this commit only drops the GUC from its constant-parity message - drop the legacy-0021 half of the writer/probe test, which could not fail, and rename the desired-state test to what it checks - fix stale GUC prose and document the ACCESS EXCLUSIVE lock in 0062 Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
With the floor trigger retired, the source policy is the only guard against rewriting created_at/channel_id on existing event rows. Close two matcher gaps found in review: - `UPDATE events SET(created_at, ...)` with no space before the parenthesis slipped past the " set " split - `INSERT INTO events ... ON CONFLICT ... DO UPDATE SET channel_id = ...` names no table, so it passed even though it can re-home an existing row below the floor; bind each DO UPDATE to the INSERT it belongs to and treat it as an events update when that INSERT targets events Share the SET-list check between both paths, and pin each gap with a negative fixture plus controls: a non-floor-column events upsert, DO NOTHING, and a guarded events insert next to a watermark upsert that sets created_at (the shape of the replaceable-event writer). Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
TheSentinel454
force-pushed
the
elrond/drop-floor-guard
branch
from
October 16, 2026 14:05
89831dd to
8d2d16c
Compare
With the trigger gone the source policy is the only guard. Expose SQL the per-function scan could not see: include_str! is expanded from the file it loads and concat! literals are joined, so both reach the existing insert/update checks. Fail closed on what stays opaque: a non-literal include path, a concat! that spells SQL around a non-literal argument, event-write SQL in a const, static or macro body outside any function, and PREPARE TRANSACTION in any production writer. Cut each function slice to its own item so trailing consts are judged at item level. The replica fence no longer folds pg_prepared_xacts.prepared into the wall: only the retired trigger checked prepared rows at PREPARE, and the insert-time guard's clock is not exposed there. A prepared transaction now fails the sample closed (PreparedTransactions). Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
TheSentinel454
force-pushed
the
elrond/drop-floor-guard
branch
from
October 16, 2026 14:08
8d2d16c to
8752ce1
Compare
added 2 commits
October 16, 2026 10:26
…annot read Review of 8752ce1 found the opaque-concat rule red on production: the two retention deletes concat a predicate macro into their WHERE, and any "update " around a non-literal argument was a violation. Replace that heuristic with one detector. concat! now becomes a single literal with an {opaque} placeholder per non-literal argument, and every SQL literal is checked on its own: a placeholder (from concat! or format!) in a write target or SET column, a floor-column SET list whose UPDATE is in another literal, an INSERT INTO with no table after it or an events INSERT with a placeholder tail, events written by MERGE INTO or COPY, and a dangling PREPARE are violations. A placeholder in a WHERE or a value is allowed, so the retention deletes and the tenant-severing UPDATE {table} SET community_id = NULL pass. Also recognize concat!/include_str! with [] and {} delimiters, and ban sqlx query_file*! macros, which load SQL the scan cannot see. Document that the prepared-transaction count is cluster-wide on purpose, like the activity scan. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
The per-literal UPDATE check only saw the SET list inside the literal
that holds SET, so format!("UPDATE events SET {}", ..), a split
[..].concat(), and QueryBuilder pushes after SET slipped through. A SET
list that is empty or holds an item with no '=' is now unattributable
when its target is events, a placeholder, or DO UPDATE. Prose that
merely contains update/set is still ignored.
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
TheSentinel454
force-pushed
the
elrond/floor-guard-app
branch
2 times, most recently
from
October 16, 2026 21:12
b8878d5 to
2f337aa
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 2 of BUZZ-316. This is a destructive contract change, so it merges last. It is stacked on #8325 (phase 1), which adds the insert-time application floor check.
After phase 1, the app check at every channel-bearing
INSERT INTO eventsis the onlycreated_atfloor guard. This PR removes the database backstop and everything that existed only to verify it:events_created_at_flooronevents. That also removes its partition clones. It then loops overpg_partition_tree('events')to drop any child copy that was attached separately, and drops theevents_created_at_floor_guard()function. Every drop usesIF EXISTSand none usesCASCADE. The migration setslock_timeoutto 5s. Migration 0021 stays byte-for-byte unchanged.schema.sqlinclude, the trigger inschema/triggers/events.sql, and the eight child-copy drops inscripts/reconcile-schema-after-pgschema.sql. Both boot paths stay catalog-equivalent, andadmin_schema_parity_between_desired_state_and_migrationspasses.verify_floor_guard_catalog,verify_floor_guard_behavior, and the boot wrapper that gated the probe on them. Also deletes the writer-poolbuzz.created_at_floorGUC arming and itsCreatedAtFloorconnection-setup metric step. The setup-series ceiling drops from 71 to 54.spawn_fence_probebecomes synchronous and returnsbool, so the relay's startup error branch goes away.CREATED_AT_FLOOR_SECS(960s) stays because the app check and the fence wall both use it.Deploy gate (required)
DROP TRIGGERtakesACCESS EXCLUSIVEoneventsand every partition, so expecteventsreads and writes to stall for up to the 5slock_timeout. A lock timeout aborts the migration transaction and is safe to retry. Under auto-migrate, the pod fails boot, restarts, and retries.Mixed-version behavior:
migrate.autoMigrate: truein the chart): sqlx finds applied migration 62 missing from its embedded set and returnsVersionMissing(62). Boot is fatal and the pod crash-loops. A binary rollback is therefore unsafe once 0062 has run.verify_floor_guard_catalogfails, the fence starts closed, and the relay degrades theReplicaFenceProbestartup step. It keeps serving, with cursor reads on the writer.Tests
crates/buzz-db/src/runtime/tests/floor_retirement.rsadds four PostgreSQL tests. CI's nextest filter picks them up throughpostgres_tests:::All four fail when the change is reverted (0/4 pass) and pass with it (4/4). Four tests that only exercised the trigger or the probes are deleted. The outbox durable-failure regression is kept: it now injects an isolated
BEFORE INSERTfailure instead of arming the floor. The phase-1 floor fixture no longer disables a trigger that no longer exists. It still asserts that no enabled floor trigger can mask the app check.With the trigger gone, the floor source policy (
every_production_channel_event_insert_checks_the_floor_and_never_rewrites_it) is the only guard against a writer that skips the app check or rewritescreated_at/channel_idon an existing row. Phase 1 already makes it scan every crate'ssrc. This PR closes two matcher gaps the trigger used to cover:UPDATE events SET(created_at, …)written with no space before the parenthesis;INSERT INTO events … ON CONFLICT … DO UPDATE SET channel_id = …, which names no table but can re-home an existing row below the floor. EachDO UPDATEis bound to the INSERT it belongs to, so a watermark upsert that setscreated_atnext to a guarded events insert (the replaceable-event writer's shape) is not flagged.Both gaps have negative fixtures, plus controls for a non-floor-column upsert,
DO NOTHING, and the watermark shape.The policy also now fails closed on SQL the per-function literal scan could not attribute to a writer (
event_floor_policy_fails_closed_on_sql_it_cannot_attribute):include_str!is expanded from the file it loads, andconcat!becomes one literal, with an{opaque}placeholder for each non-literal argument. Every delimiter ((),[],{}) and path prefix (::core::concat!) is recognized.SETcolumn that is a placeholder (concat!orformat!); aSETlist of floor columns whoseUPDATEis in another literal; anUPDATE eventsorDO UPDATEwhoseSETlist continues in another literal or argument (empty, or an item with no=, as informat!("UPDATE events SET {}", …)orQueryBuilder::new("UPDATE events SET ").push(…)); anINSERT INTOwith no table after it in the literal, or an events INSERT with a placeholder tail;eventswritten byMERGE INTO/COPY; and event-write SQL in aconst,static, or macro body outside any function. A placeholder in aWHEREor a value is allowed, so the retention deletes (concat!around a predicate macro) and the tenant-severingUPDATE {table} SET community_id = NULLpass.include_str!whose path is not one readable literal and any sqlxquery_file*!macro are violations. Nothing usesquery_file.PREPARE TRANSACTIONin any production literal, including a danglingPREPARE, is a violation (see below).constis judged at item level instead of being credited to the function before it.Prepared transactions. The fence used to bound a prepared (2PC) transaction by
pg_prepared_xacts.prepared. That only held because the deferred trigger checked its rows atPREPARE; the insert-time check runs earlier, at a timepg_prepared_xactsdoes not expose. So the probe no longer foldspreparedinto the wall: any prepared transaction fails the sample closed (ProbeError::PreparedTransactions). No supported writer prepares transactions (now source-policy enforced) andmax_prepared_transactionsdefaults to 0, so in supported deployments this never fires.cluster_global_sample_writer_fails_closed_on_prepared_transactionasserts 2PC is unavailable at the default setting, and asserts the fail-closed error when the server allows it. CI runs only the default branch; the 2PC-enabled branch and a check-removed falsification run are pinned from the Blox gate. The prepared count is cluster-wide, like the activity scan.What got simpler
The change is net −600 lines. Gone are one function, one parent constraint trigger plus its partition clones, two startup probes and their scratch helper, one boot wrapper, the per-connection GUC setup, one metric step, and an async startup branch that could no longer be reached.
Notes
migration.rs.buzz-relaygroup_state_postgres_testshitPoolTimedOuton a clean base too.