mirror of
https://github.com/block/buzz.git
synced 2026-11-02 23:41:08 +00:00
## What Removes every production call to the SQL fence predicate `community_write_allowed()` from application SQL, so no application mutation depends on the boolean predicate anymore. This is one prerequisite for BUZZ-315 part B; #8322 (tolerating absent fence objects) is a separate one. The triggers and SQL functions stay in this PR because mixed-version pods still depend on them. The predicate isn't inlined. It's a VOLATILE plpgsql function that takes the deletion lock and *then* reads lifecycle state on a fresh snapshot, and an inline expression would read state from the statement's start snapshot. A separate earlier check would race deletion. So each sweep now: 1. reads its candidate community IDs with a plain read, bounded the same way the original statement was, 2. opens one `AdmittedTx` per community (`begin_community_sweep_transaction`, a thin wrapper over `begin_community_write_transaction` that turns an admission refusal into "skip this tenant"; the four periodic sweeps go through `sweep_admitted_communities`, which also owns their error handling), 3. runs the original mutation scoped to `community_id = $1` and commits. | Site | Change | | --- | --- | | `runtime/push_enqueue.rs` | Deleted the redundant predicate. This transaction already calls `guard_transaction`. | | `store/relay_invite.rs` `reap_expired_relay_invites` | Keeps the global expiry order and the 1,000-row ceiling. Candidates are pre-filtered to active tenants so fenced invites can't fill the batch. Admission is still what authorizes the write. | | `store/workflow.rs` `prune_scheduled_workflow_fires_before` | Prunes per admitted community and sums the row counts. | | `store/channel.rs` `reap_expired_ephemeral_channels` | Archives per admitted community and returns the committed rows, so the relay still runs their system and discovery follow-ups. The candidate read has no lifecycle pre-filter (it has no `LIMIT`, so admission alone decides). | | `store/event.rs` `backfill_d_tags` | Backfills per admitted community at startup. | | `store/push.rs` `claim_due_match_batch_with_loader` | Picks the earliest due active community, admits it, and claims under the same `FOR UPDATE OF q SKIP LOCKED` and batch limit. It skips at most 16 tenants that fence between the read and admission. | | `store/push.rs` `reap_exhausted_matches` | Reaps per admitted community. | **Tenant failures.** The four periodic sweeps (invite reap, workflow fire prune, channel reap, exhausted match reap) log a warning and continue when one tenant fails. That tenant's rows are left unmodified and stay eligible, so the next tick retries them. A tenant error doesn't abort processing of the other tenants already in the selected candidate page. Invite cleanup keeps its oldest-first 1,000-row selection, so if one tenant owns that entire page and its delete keeps failing, tenants outside the page wait. No persistent tenant-local failure for that delete is known in the current schema, and base aborted the whole page on any error. The returned count or rows cover the tenants that committed. Claim, enqueue and the startup backfill still return the error to the caller. **Database outages.** A connection-level error (pool acquire timeout, closed pool, socket I/O; `DbError::is_connection_unavailable`) isn't tenant-local, so it stops the periodic sweep at that tenant instead of moving on. Without this, an outage that began after the candidate read cost one writer acquire timeout and one warning per remaining tenant, and invite cleanup runs inside the usage-metrics leader tick. Rows already committed are still returned, so reaped channels keep their follow-ups. Remaining tenants retry next tick. **Known limit (pre-existing, tracked separately):** - *Claim at a locked head.* `claim_due_match_batch_with_loader` returns `None` once an admitted tenant claims zero rows. If every due row in the oldest active tenant is row-locked, younger tenants aren't tried on that call. Base behaved the same way. This PR doesn't change it. ## Tests All tests use a one-session fixture (`test_support::admission_fixture`) with `session_replication_role = replica`, so the trigger backstop is off and only application admission protects the fenced tenants. The fixture shadows `community_write_allowed` with a function that raises, in a per-call `admission_test_<uuid>` schema. - One test per site: a quiescing tenant's rows are untouched while an active tenant in the same sweep still progresses. - Race tests for invite reap, channel reap and claim. These are the sites where the candidate read already filters out fenced tenants. Each test holds the tenant's deletion lock, waits until the sweep is blocked in admission, fences the tenant in that transaction, and asserts its rows are untouched. The claim test also asserts that a second active tenant is claimed in the same call. - `channel_reap_returns_committed_rows_after_earlier_tenant_error` and `periodic_reaps_continue_after_tenant_error_and_retry_retained_rows` inject a per-tenant failure. They check that later tenants commit, that the failed tenant's rows are retained, and that the next call processes them. - `fleet_sweep_stops_at_first_unavailable_connection` runs the sweep helper against a closed pool with three candidates and asserts only the first is attempted. `fleet_sweep_returns_committed_results_when_database_drops_mid_page` (PostgreSQL) commits the first tenant, closes the pool, and asserts the first tenant's result is returned and the sweep stops at the second of three candidates. `connection_unavailable_covers_only_pool_and_io_failures` pins the classification; the per-tenant failure tests above confirm a database-raised error still continues. Falsification, each with the tests failing as expected: | Mutation | Result | | --- | --- | | Original SQL restored at invite sweep, push claim, d-tag backfill | 0/3 pass (`legacy SQL fence predicate called`) | | `guard_community_write` returns `Ok(())` after taking the lock | race tests + channel reap test 0/4 pass | | Per-tenant errors abort the sweep again | 0/2 pass | | Sweep helper `continue`s instead of stopping on a connection-level error | `fleet_sweep_stops_at_first_unavailable_connection` and `fleet_sweep_returns_committed_results_when_database_drops_mid_page` fail | | Sweep helper drops already-committed results when it stops (`return Vec::new()`) | `fleet_sweep_returns_committed_results_when_database_drops_mid_page` fails | | Sweep helper removed from the source-policy chokepoint check | `serving_table_writes_expose_syntactic_chokepoint_or_guarded_tx_routes` fails | The PostgreSQL tests are under `postgres_tests` modules with `#[ignore = "requires Postgres"]`, which puts them in the `.config/nextest.toml` PostgreSQL filter. CI evidence is under Verification. ## Verification CI at head `5d37fbda` ([run 38925996275](https://github.com/block/buzz/actions/runs/38925996275)) is green. The PostgreSQL job ([job 117062895683](https://github.com/block/buzz/actions/runs/38925996275/job/117062895683)) passed 1017/1017. That includes named PASS lines for all 30 `admission_postgres_tests`, `fleet_sweep_returns_committed_results_when_database_drops_mid_page`, and both writer-pool tests listed below. Unit Tests ([job 117058136482](https://github.com/block/buzz/actions/runs/38925996275/job/117058136482)) has PASS lines for `connection_unavailable_covers_only_pool_and_io_failures` and `fleet_sweep_stops_at_first_unavailable_connection`. Local gates ran on a Linux cloud workstation against private PostgreSQL 16 and Redis. The tree was `735170df`, which is this head rebased on `main` `c0ab789`, so the gates built the merge result. | Gate | Result | | --- | --- | | `cargo fmt --all -- --check` | pass | | `cargo clippy --workspace --all-targets -- -D warnings` | pass | | `cargo nextest run -p buzz-db --lib --tests` | 185 passed | | `scripts/test-postgres-test-discovery.sh` | pass | | PostgreSQL lane for buzz-db (excluding the two writer-pool tests below) | 530 passed | | PostgreSQL lanes for buzz-admin, buzz-audit, buzz-deletion, buzz-push-gateway, buzz-search, buzz-workflow | all pass | | PostgreSQL lane for buzz-relay | 413/426. The 13 `group_state_postgres_tests` fail with `PoolTimedOut` on unmodified `main` `c0ab789` too (see below) | | The three mutations in the falsification table above (`continue` ×2, `return Vec::new()`) | each fails its target test | These failures happen only on the workstation, and each one passes in CI's PostgreSQL job at this head: - `writer_pool_metrics_stop_after_session_timeout_setup_failure` stalls. It's an unchanged writer-pool configuration test, and it stalled the same way at earlier heads. - The 13 relay `group_state_postgres_tests` fail at the same point, `ensure community: Sqlx(PoolTimedOut)`, on unmodified `main` `c0ab789`. This PR doesn't touch `buzz-relay`. ## What got simpler - No application mutation calls the boolean predicate `community_write_allowed()` anymore. Dropping the fence objects still has other dependencies: serving-write lease acquisition calls `assert_community_write_allowed()` (`store/deletion.rs:3068`), catalog validation requires both functions (`store/deletion.rs:1195-1198`), and the triggers and compatibility SQL still use them. Those belong to #8322 and part B, not this PR. - The claim's `target` CTE is gone, and every sweep uses the same admission-and-skip helper. - The four periodic sweeps share one loop (`sweep_admitted_communities`) for admission, skip, continue-on-tenant-error and stop-on-outage, instead of four copies of it. - The source policy treats that helper as a tenant-local chokepoint, so `backfill_d_tags` no longer needs a reviewed exception. - The per-tenant loops add code, because each community now gets its own admitted transaction. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
317 lines
13 KiB
Rust
317 lines
13 KiB
Rust
const DEFAULT_DATABASE_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1 -- local test-only credentials
|
|
|
|
/// Resolve the database URL shared by PostgreSQL-backed unit tests.
|
|
pub(crate) fn database_url() -> String {
|
|
std::env::var("BUZZ_TEST_DATABASE_URL")
|
|
.or_else(|_| std::env::var("TEST_DATABASE_URL"))
|
|
.or_else(|_| std::env::var("DATABASE_URL"))
|
|
.unwrap_or_else(|_| DEFAULT_DATABASE_URL.to_owned())
|
|
}
|
|
|
|
/// One-session fixture that bypasses ordinary write-fence triggers while
|
|
/// retaining application admission. Communities are active, quiescing, fenced.
|
|
pub(crate) async fn admission_fixture() -> (crate::Db, [buzz_core::CommunityId; 3]) {
|
|
use buzz_core::CommunityId;
|
|
use uuid::Uuid;
|
|
|
|
let db = crate::Db::new(&crate::DbConfig {
|
|
database_url: database_url(),
|
|
max_connections: 1,
|
|
min_connections: 1,
|
|
..crate::DbConfig::default()
|
|
})
|
|
.await
|
|
.expect("connect test database");
|
|
let pool = &db.pool;
|
|
let mut communities = [CommunityId::from_uuid(Uuid::nil()); 3];
|
|
for community in &mut communities {
|
|
let id = Uuid::new_v4();
|
|
sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)")
|
|
.bind(id)
|
|
.bind(format!("admission-{id}.example"))
|
|
.execute(pool)
|
|
.await
|
|
.expect("create community");
|
|
*community = CommunityId::from_uuid(id);
|
|
}
|
|
set_deletion_state(pool, *communities[1].as_uuid(), "quiescing").await;
|
|
set_deletion_state(pool, *communities[2].as_uuid(), "fenced").await;
|
|
// The one physical session serves every API call in this fixture.
|
|
sqlx::query("SET session_replication_role = replica")
|
|
.execute(pool)
|
|
.await
|
|
.expect("disable ordinary triggers");
|
|
let role: String = sqlx::query_scalar("SHOW session_replication_role")
|
|
.fetch_one(pool)
|
|
.await
|
|
.expect("read replication role");
|
|
assert_eq!(role, "replica");
|
|
// A raw write to the fenced tenant proves the DB backstop is bypassed.
|
|
sqlx::query("INSERT INTO users (community_id, pubkey) VALUES ($1, $2)")
|
|
.bind(communities[2].as_uuid())
|
|
.bind(vec![0xfe_u8; 32])
|
|
.execute(pool)
|
|
.await
|
|
.expect("raw fenced insert succeeds with triggers disabled");
|
|
(db, communities)
|
|
}
|
|
|
|
/// In one per-test database, make any return to the retired SQL predicate
|
|
/// observable without changing the production function in the public schema.
|
|
pub(crate) async fn reject_legacy_sql_fence_predicate(pool: &sqlx::PgPool) -> String {
|
|
let schema = format!("admission_test_{}", uuid::Uuid::new_v4().simple());
|
|
sqlx::query(sqlx::AssertSqlSafe(format!("CREATE SCHEMA {schema}")))
|
|
.execute(pool)
|
|
.await
|
|
.expect("create test-only function schema");
|
|
sqlx::query(sqlx::AssertSqlSafe(format!(
|
|
"CREATE FUNCTION {schema}.community_write_allowed(uuid) RETURNS boolean \
|
|
LANGUAGE plpgsql VOLATILE AS $$ BEGIN \
|
|
RAISE EXCEPTION 'legacy SQL fence predicate called'; END $$",
|
|
)))
|
|
.execute(pool)
|
|
.await
|
|
.expect("shadow legacy SQL predicate");
|
|
sqlx::query(sqlx::AssertSqlSafe(format!(
|
|
"SET search_path = {schema}, public"
|
|
)))
|
|
.execute(pool)
|
|
.await
|
|
.expect("resolve test-local predicate first");
|
|
let shadowed: bool = sqlx::query_scalar(
|
|
"SELECT to_regprocedure('community_write_allowed(uuid)') = to_regprocedure($1)",
|
|
)
|
|
.bind(format!("{schema}.community_write_allowed(uuid)"))
|
|
.fetch_one(pool)
|
|
.await
|
|
.expect("resolve shadowed predicate");
|
|
assert!(shadowed);
|
|
schema
|
|
}
|
|
|
|
/// Hold the executor's exclusive lock until a sweep reaches shared admission,
|
|
/// then quiesce in that same transaction before releasing it. The ordinary
|
|
/// second pool keeps lifecycle triggers enabled; the sweep fixture bypasses them.
|
|
pub(crate) async fn fence_while_sweep_waits<F, T>(community: buzz_core::CommunityId, sweep: F) -> T
|
|
where
|
|
F: std::future::Future<Output = T> + Send + 'static,
|
|
T: Send + 'static,
|
|
{
|
|
let pool = sqlx::postgres::PgPoolOptions::new()
|
|
.max_connections(1)
|
|
.connect(&database_url())
|
|
.await
|
|
.expect("connect deletion executor pool");
|
|
let mut tx = pool.begin().await.expect("begin deletion executor");
|
|
sqlx::query("SELECT pg_advisory_xact_lock(community_deletion_lock_key($1))")
|
|
.bind(community.as_uuid())
|
|
.execute(&mut *tx)
|
|
.await
|
|
.expect("hold exclusive deletion lock");
|
|
let mut task = tokio::spawn(sweep);
|
|
let waiting = tokio::time::timeout(std::time::Duration::from_secs(5), async {
|
|
loop {
|
|
let blocked: bool = sqlx::query_scalar(
|
|
"SELECT EXISTS(SELECT 1 FROM pg_locks \
|
|
WHERE locktype = 'advisory' AND NOT granted \
|
|
AND database = (SELECT oid FROM pg_database WHERE datname = current_database()) \
|
|
AND pg_backend_pid() = ANY(pg_blocking_pids(pid)))",
|
|
)
|
|
.fetch_one(&mut *tx)
|
|
.await
|
|
.expect("inspect blocked advisory admission");
|
|
if blocked {
|
|
break;
|
|
}
|
|
tokio::task::yield_now().await;
|
|
}
|
|
})
|
|
.await;
|
|
if waiting.is_err() {
|
|
task.abort();
|
|
panic!("sweep did not block in admission before deadline");
|
|
}
|
|
set_deletion_state_in_transaction(&mut tx, *community.as_uuid(), "quiescing").await;
|
|
tx.commit().await.expect("commit deletion fence");
|
|
let result = tokio::time::timeout(std::time::Duration::from_secs(5), &mut task).await;
|
|
if result.is_err() {
|
|
task.abort();
|
|
panic!("sweep did not finish after deletion fence committed");
|
|
}
|
|
pool.close().await;
|
|
result.unwrap().expect("sweep task")
|
|
}
|
|
|
|
/// Move a community fixture into a deletion lifecycle state the way the
|
|
/// executor does: the tombstone trigger only accepts lifecycle changes from a
|
|
/// transaction carrying the executor GUCs for that community, including its
|
|
/// current fence generation.
|
|
pub(crate) async fn set_deletion_state(pool: &sqlx::PgPool, id: uuid::Uuid, state: &str) {
|
|
let mut tx = pool.begin().await.expect("begin lifecycle fixture");
|
|
set_deletion_state_in_transaction(&mut tx, id, state).await;
|
|
tx.commit().await.expect("commit lifecycle fixture");
|
|
}
|
|
|
|
async fn set_deletion_state_in_transaction(
|
|
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
|
id: uuid::Uuid,
|
|
state: &str,
|
|
) {
|
|
sqlx::query(
|
|
"SELECT set_config('buzz.deletion_executor_community', $1, true), \
|
|
set_config('buzz.deletion_fence_generation', \
|
|
(SELECT deletion_fence_generation::text FROM communities WHERE id = $2), true)",
|
|
)
|
|
.bind(id.to_string())
|
|
.bind(id)
|
|
.execute(&mut **tx)
|
|
.await
|
|
.expect("authorize lifecycle fixture");
|
|
sqlx::query(
|
|
"UPDATE communities SET deletion_state = $2, \
|
|
deleted_at = CASE WHEN $2 = 'tombstone' THEN now() END \
|
|
WHERE id = $1",
|
|
)
|
|
.bind(id)
|
|
.bind(state)
|
|
.execute(&mut **tx)
|
|
.await
|
|
.expect("set lifecycle state");
|
|
}
|
|
|
|
/// Move a disposable test community into `quiescing`, the state in which
|
|
/// admission must reject new serving writes.
|
|
pub(crate) async fn quiesce_community_for_tests(
|
|
pool: &sqlx::PgPool,
|
|
community: buzz_core::CommunityId,
|
|
) {
|
|
set_deletion_state(pool, *community.as_uuid(), "quiescing").await;
|
|
}
|
|
|
|
/// Whether `error` is the community-admission rejection taken at transaction
|
|
/// entry, as opposed to a later commit-time trigger rejection.
|
|
pub(crate) fn is_admission_rejection(error: &crate::DbError) -> bool {
|
|
matches!(error, crate::DbError::AccessDenied(message) if message.contains("write-fenced (quiescing)"))
|
|
}
|
|
|
|
/// The desired-state schema as one SQL script.
|
|
///
|
|
/// `schema/schema.sql` is a manifest of `\i` includes resolved relative to the
|
|
/// including file (as `pgschema` does). This expands every include in place,
|
|
/// so text assertions and `sqlx::raw_sql` bootstraps see the same statements,
|
|
/// in the same order, that `pgschema apply` loads.
|
|
pub(crate) fn desired_state_schema_sql() -> String {
|
|
fn expand(path: &std::path::Path, out: &mut String) {
|
|
let text = std::fs::read_to_string(path)
|
|
.unwrap_or_else(|error| panic!("read {}: {error}", path.display()));
|
|
let dir = path.parent().expect("schema file has a parent directory");
|
|
for line in text.split_inclusive('\n') {
|
|
match line.trim_end().strip_prefix("\\i ") {
|
|
Some(include) => expand(&dir.join(include.trim()), out),
|
|
None => out.push_str(line),
|
|
}
|
|
}
|
|
if !out.ends_with('\n') {
|
|
out.push('\n');
|
|
}
|
|
}
|
|
let mut out = String::new();
|
|
expand(&desired_state_schema_dir().join("schema.sql"), &mut out);
|
|
out
|
|
}
|
|
|
|
/// `schema/` at the workspace root.
|
|
pub(crate) fn desired_state_schema_dir() -> std::path::PathBuf {
|
|
std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../schema")
|
|
}
|
|
|
|
/// Rewrite every `ON DELETE CASCADE`, `SET NULL` and `SET DEFAULT` foreign key
|
|
/// in this test's database as `NO ACTION`: the schema the cascade retirement
|
|
/// targets. Every trigger stays live, including the foreign-key checks and the
|
|
/// community fence, so a delete path exercised afterwards must remove its
|
|
/// children itself, in an order the remaining constraints accept.
|
|
///
|
|
/// Each PostgreSQL test process owns its database (`crates/buzz-db/TESTING.md`),
|
|
/// so the rewrite does not reach other tests. It refuses any database whose
|
|
/// name does not match the per-test wrapper's (`buzz_nt_` plus 24 hex digits),
|
|
/// which excludes the run's `_desired` template. The name is the only check:
|
|
/// a manually created database with that shape would pass.
|
|
pub(crate) async fn retire_foreign_key_delete_actions(pool: &sqlx::PgPool) {
|
|
let database: String = sqlx::query_scalar("SELECT current_database()")
|
|
.fetch_one(pool)
|
|
.await
|
|
.expect("read current database");
|
|
assert!(
|
|
database
|
|
.strip_prefix("buzz_nt_")
|
|
.is_some_and(|hash| hash.len() == 24
|
|
&& hash
|
|
.bytes()
|
|
.all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b))),
|
|
"refusing to rewrite foreign keys in shared database {database}; \
|
|
run this test through scripts/postgres-test-run.sh"
|
|
);
|
|
sqlx::raw_sql(
|
|
r#"DO $$
|
|
DECLARE c record;
|
|
BEGIN
|
|
FOR c IN SELECT conrelid::regclass AS tbl, conname, pg_get_constraintdef(oid) AS def
|
|
FROM pg_constraint
|
|
WHERE contype = 'f' AND confdeltype IN ('c', 'n', 'd') AND conparentid = 0
|
|
LOOP
|
|
EXECUTE format('ALTER TABLE %s DROP CONSTRAINT %I, ADD CONSTRAINT %I %s',
|
|
c.tbl, c.conname, c.conname,
|
|
regexp_replace(c.def, ' ON DELETE (CASCADE|SET NULL|SET DEFAULT)( \([^)]*\))?', ''));
|
|
END LOOP;
|
|
END $$"#,
|
|
)
|
|
.execute(pool)
|
|
.await
|
|
.expect("retire foreign-key delete actions");
|
|
let remaining: i64 = sqlx::query_scalar(
|
|
"SELECT count(*) FROM pg_constraint WHERE contype = 'f' AND confdeltype IN ('c', 'n', 'd')",
|
|
)
|
|
.fetch_one(pool)
|
|
.await
|
|
.expect("count foreign-key delete actions");
|
|
assert_eq!(remaining, 0, "every foreign-key delete action is retired");
|
|
}
|
|
|
|
/// A single-connection pool whose session carries `application_name`, so
|
|
/// [`wait_for_lock_wait`] can tell when its statement is blocked.
|
|
pub(crate) async fn named_pool(application_name: &str) -> sqlx::PgPool {
|
|
use std::str::FromStr as _;
|
|
let options = sqlx::postgres::PgConnectOptions::from_str(&database_url())
|
|
.expect("parse test database URL")
|
|
.application_name(application_name);
|
|
sqlx::postgres::PgPoolOptions::new()
|
|
.max_connections(1)
|
|
.connect_with(options)
|
|
.await
|
|
.expect("connect named test pool")
|
|
}
|
|
|
|
/// Wait until the session named `application_name` in this test's database is
|
|
/// blocked on a lock.
|
|
pub(crate) async fn wait_for_lock_wait(pool: &sqlx::PgPool, application_name: &str) {
|
|
tokio::time::timeout(std::time::Duration::from_secs(5), async {
|
|
loop {
|
|
let waiting: bool = sqlx::query_scalar(
|
|
"SELECT EXISTS(SELECT 1 FROM pg_stat_activity \
|
|
WHERE datname = current_database() AND application_name = $1 \
|
|
AND wait_event_type = 'Lock')",
|
|
)
|
|
.bind(application_name)
|
|
.fetch_one(pool)
|
|
.await
|
|
.expect("inspect lock wait");
|
|
if waiting {
|
|
break;
|
|
}
|
|
tokio::task::yield_now().await;
|
|
}
|
|
})
|
|
.await
|
|
.expect("session reached a lock wait");
|
|
}
|