diff --git a/crates/buzz-db/src/runtime/tests/personal_read_migration_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/personal_read_migration_postgres_tests.rs index 0d1319f2f3..c804ed0f71 100644 --- a/crates/buzz-db/src/runtime/tests/personal_read_migration_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/personal_read_migration_postgres_tests.rs @@ -89,3 +89,104 @@ async fn migration_schema_personal_read_fresh_build_and_budgets() { assert_eq!(regclass_oid(&pool, "idx_thread_metadata_root").await, None); drop_scratch_db(&admin, pool, &name).await; } + +/// Under 0056 only a read intent creates an account, so 0057 starts every +/// existing account at the migration. A message that arrives after upgrade +/// counts without a further intent. Accounts in a community being deleted +/// are skipped, so the write fence cannot fail the migration. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn migration_schema_personal_read_starts_existing_accounts() { + use crate::{ + channel::{ChannelType, ChannelVisibility}, + Db, + }; + use nostr::{EventBuilder, Keys, Kind, Tag}; + + let admin = PgPool::connect(&admin_url().await).await.unwrap(); + let (pool, name) = create_scratch_db_through(&admin, "pr_upgrade", Some(56)).await; + let actor = Keys::generate(); + let actor_bytes = actor.public_key().to_bytes(); + let active: Uuid = + sqlx::query_scalar("INSERT INTO communities (host) VALUES ($1) RETURNING id") + .bind(format!("upgrade-{}.local", Uuid::new_v4())) + .fetch_one(&pool) + .await + .unwrap(); + sqlx::query("INSERT INTO personal_read_accounts (community_id,actor) VALUES ($1,$2)") + .bind(active) + .bind(actor_bytes.as_slice()) + .execute(&pool) + .await + .unwrap(); + // A fenced community's account, set up past its fence only in this + // disposable superuser DB. + let mut tx = pool.begin().await.unwrap(); + sqlx::query("SET LOCAL session_replication_role = replica") + .execute(&mut *tx) + .await + .unwrap(); + let fenced: Uuid = sqlx::query_scalar( + "INSERT INTO communities (host,deletion_state) VALUES ($1,'fenced') RETURNING id", + ) + .bind(format!("fenced-{}.local", Uuid::new_v4())) + .fetch_one(&mut *tx) + .await + .unwrap(); + sqlx::query("INSERT INTO personal_read_accounts (community_id,actor) VALUES ($1,$2)") + .bind(fenced) + .bind(actor_bytes.as_slice()) + .execute(&mut *tx) + .await + .unwrap(); + tx.commit().await.unwrap(); + + migration::run_migrations(&pool).await.unwrap(); + let started = |community: Uuid| { + sqlx::query_scalar::<_, bool>( + "SELECT started_at IS NOT NULL FROM personal_read_accounts + WHERE community_id=$1 AND actor=$2", + ) + .bind(community) + .bind(actor_bytes.as_slice()) + .fetch_one(&pool) + }; + assert!(started(active).await.unwrap(), "existing account starts"); + assert!(!started(fenced).await.unwrap(), "fenced account is skipped"); + + let db = Db::from_pool(pool.clone()); + let community = CommunityId::from_uuid(active); + let channel = db + .create_channel( + community, + "upgraded", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &actor_bytes, + None, + ) + .await + .unwrap() + .id; + let event = EventBuilder::new(Kind::Custom(9), "after upgrade") + .tags(vec![Tag::public_key(actor.public_key())]) + .sign_with_keys(&Keys::generate()) + .unwrap(); + db.insert_event(community, &event, Some(channel)) + .await + .unwrap(); + let row = db + .personal_read_sidebar(community, &actor.public_key(), 20, None) + .await + .unwrap() + .channels + .remove(0); + assert_eq!( + (row.unread, row.mentions), + (true, 1), + "an arrival after upgrade counts before the next intent" + ); + drop(db); + drop_scratch_db(&admin, pool, &name).await; +} diff --git a/docs/buzz-v1-read-state.md b/docs/buzz-v1-read-state.md index 0913448115..bc0f4caeb9 100644 --- a/docs/buzz-v1-read-state.md +++ b/docs/buzz-v1-read-state.md @@ -232,7 +232,9 @@ device-local. Apply migrations 0056 and 0057 (or the equivalent desired schema). 0056 creates two private tables. 0057 adds `personal_read_accounts.started_at` -(NULL for accounts from 0056, which start at their next read intent), +(set to the migration's time for accounts from 0056, which only a read intent +created, so arrivals from deploy onward count; accounts in a community being +deleted stay NULL), `personal_read_frontiers.following`, `thread_metadata.last_reply_received_at`, `channels.last_timeline_received_at` and `event_mentions.received_at`/`root_id` (NULL for rows from before it, which no @@ -241,11 +243,13 @@ position can reach), and one index on `event_mentions`. It also drops (0049). Ingest writes the author's own position and follow rows in the same transaction as the message. -Like 0049, 0057 bounds lock waits and statement time, so it fails -instead of blocking `event_mentions` writes while building its index on a -populated table. Brownfield deployments prebuild the index first. Adding -nullable columns is a catalog-only change; `CREATE INDEX CONCURRENTLY` cannot -run inside a transaction block: +Like 0049, 0057 bounds lock waits (1s per acquisition) and statement time +(5s), so it fails instead of blocking `event_mentions` writes for a whole +index build. On a large `event_mentions` table, 0057 will hit its statement +timeout and roll back without recording the version, so brownfield +deployments prebuild the index first. Adding nullable columns is a +catalog-only change; `CREATE INDEX CONCURRENTLY` cannot run inside a +transaction block: ```sql ALTER TABLE public.event_mentions @@ -278,8 +282,9 @@ DROP INDEX CONCURRENTLY IF EXISTS public.idx_thread_metadata_root; 0057 still takes brief exclusive locks to add columns to the two read-state tables, `thread_metadata`, `channels` and `event_mentions`. If a long reader holds one of them past the -lock budget, startup fails and can be retried; ingestion does not queue behind -it. +lock budget, startup fails and can be retried. Ingestion may queue until the +migration completes or aborts; lock waits are limited to 1s per acquisition +and statements to 5s. During a rolling deploy, relay processes still on the old code write `event_mentions` rows without `received_at`/`root_id` and do not mark posting diff --git a/migrations/0057_personal_read_scopes.sql b/migrations/0057_personal_read_scopes.sql index 37be6bfe3d..b7141bb244 100644 --- a/migrations/0057_personal_read_scopes.sql +++ b/migrations/0057_personal_read_scopes.sql @@ -11,9 +11,21 @@ SET LOCAL statement_timeout = '5s'; -- started_at is the actor's first read intent and the floor of every -- position: until then nothing counts, and from then the actor starts caught -- up. Ingest creates accounts for thread membership without starting them. --- Accounts from 0056 stay NULL and start at their next read intent. ALTER TABLE personal_read_accounts ADD COLUMN started_at TIMESTAMPTZ; +-- Under 0056 only a read intent creates an account, so every existing +-- account has already started. Start it now (this transaction's time) so +-- arrivals from deploy onward count without waiting for its next intent. The +-- arrival columns below fill in from the same point, so every position is +-- still floored after them. Communities being deleted are skipped: their +-- write fence would reject the update, and their accounts are going away. +UPDATE personal_read_accounts a + SET started_at = now() + FROM communities c + WHERE c.id = a.community_id + AND c.deletion_state = 'active' + AND a.started_at IS NULL; + -- A thread row exists for each thread the actor follows or has unfollowed. -- Ingest moves the author's own frontiers through each message they post. -- following: thread rows only, false after unfollow, kept so later replies @@ -31,7 +43,7 @@ ALTER TABLE thread_metadata ADD COLUMN last_reply_received_at TIMESTAMPTZ; -- reader with one comparison instead of walking past replies. Unindexed so -- updates stay HOT. NULL for channels with no timeline message since this -- migration, which no position can be behind: every position is floored at --- a started_at set after it. +-- a started_at set by this migration or later. ALTER TABLE channels ADD COLUMN last_timeline_received_at TIMESTAMPTZ; -- Arrival time and read scope of each mention, so the sidebar counts a @@ -39,7 +51,7 @@ ALTER TABLE channels ADD COLUMN last_timeline_received_at TIMESTAMPTZ; -- position. root_id is the thread a reply belongs to, NULL when the mention -- shows on the channel timeline (a top-level message or broadcast reply). -- Existing rows stay NULL: every position is floored at its account's --- started_at, set by a read intent after this migration, so no earlier +-- started_at, set by this migration or a later read intent, so no earlier -- mention can be unread. -- IF NOT EXISTS admits the brownfield prebuild, which adds these columns -- before building the index concurrently. As in 0049, a prebuilt index skips