jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-11-10 11:21:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(db): cover the kind:9000 archive race and tighten the archive-race waits

Browse Source 
Address the review of af4730a54 on #8594. The lock order holds; these
are the comment, coverage and doc gaps it found.

- `put_member_waits_for_a_concurrent_archive` now drives
  `put_members_with_event` against a second uncommitted archive. The
  kind:9000 add waits for it, then returns `Refused` with one
  `AccessDenied("channel is archived")` and stores no event. It gets its
  own archive because a write queued behind the join's membership lock
  would wait whether or not its own call site took the row lock.
  Reverting that call site to the membership lock alone fails the test
  (verified by mutation): the add never waits.
- The re-add refusal in the same test is pinned to
  `"channel is archived"`, not any `AccessDenied`.
- `auto_join_skips_a_default_channel_archived_while_it_joins` waits for a
  `Lock` wait on the `SELECT archived_at FROM channels ... FOR NO KEY
  UPDATE` statement, not for any session the archive blocks.
- The `CHANNEL_MEMBERSHIP_LOCK_NAMESPACE` rationale says admissions also
  lock the channel row (`lock_channel_for_member_put`) so an archive is
  ordered against them, and that only removals take just this key.
- NOSTR.md "Default channels": the auto-join paragraph is rewrapped, and
  its dangling "It" now says "The relay".

Verified: fmt; clippy -D warnings --all-targets on buzz-db and
buzz-relay; the buzz-db `channel_members` Postgres tests (32 passed) and
the four buzz-relay auto_join Postgres tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
This commit is contained in:
Matt Toohey
2026-10-29 17:06:26 +11:00
parent 4e81ffe145
commit 2da7cac273
3 changed files with 103 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
NOSTR.md
View File

@@ -222,17 +222,17 @@ body of kind:9021, so the channel's `join_role` applies and the channel gets
its `member_joined` kind:40099, kind:39002 and kind:44100. There is no cap on
how many channels that is: whatever carries the kind:39000 tag is joined, and
the work a new member costs grows with the number of channels an owner or
admin flags. It does this once,
at that moment, and never again on login or reconnect, so a later kind:9022
leave stands. A channel that fails to join is logged and counted
(`buzz_auto_join_failures_total`), not retried: the kind:39000 tag is the
record a client reconciles against. That includes a channel archived or made
private while the join runs: the membership write decides both again under
the channel's locks, so the member is not added and nothing is announced. `buzz-admin add-member` writes the
roster row only and does not join default channels. Auto-join fires only at
those roster-insert sites, so a community that does not require relay
membership advertises the kind:39000 tag but auto-joins nobody; a client that
wants the same result there self-joins (kind:9021) on the tag.
admin flags. The relay does this once, at that moment, and never again on
login or reconnect, so a later kind:9022 leave stands. A channel that fails
to join is logged and counted (`buzz_auto_join_failures_total`), not retried:
the kind:39000 tag is the record a client reconciles against. That includes a
channel archived or made private while the join runs: the membership write
decides both again under the channel's locks, so the member is not added and
nothing is announced. `buzz-admin add-member` writes the roster row only and
does not join default channels. Auto-join fires only at those roster-insert
sites, so a community that does not require relay membership advertises the
kind:39000 tag but auto-joins nobody; a client that wants the same result
there self-joins (kind:9021) on the tag.
### Group Discovery

99
crates/buzz-db/src/store/channel_members.rs
View File

@@ -41,9 +41,11 @@ pub struct MemberRecord {
/// concurrent demotions (or a demotion racing a removal) to each observe two
/// owners, each pass, and together leave zero — the exact governance loss the
/// guards exist to prevent. An advisory key rather than `SELECT ... FOR UPDATE`
/// on the channel row: membership is its own contention domain and must not
/// serialize against unrelated channel metadata writers (`update_channel`,
/// `set_topic`, the TTL transition). Distinct key domain from
/// on the channel row, so a removal, which takes only this key, does not
/// serialize against the row's writers that leave admission alone
/// (`set_topic`, `set_canvas`, the TTL transition). Admissions also lock the
/// row, after this key (see [`lock_channel_for_member_put`]), because the row
/// is what orders an archive against them. Distinct key domain from
/// `buzz_channel_ttl:`.
const CHANNEL_MEMBERSHIP_LOCK_NAMESPACE: &str = "buzz_channel_membership:";
@@ -3885,22 +3887,28 @@ mod postgres_tests {
/// An archive and an admission are ordered by the channel row lock, and an
/// archived channel admits nobody new. A join that waits on an uncommitted
/// archive is refused once it commits, with no row, and re-adding a
/// removed member is refused too. The other order: an archive waits for
/// an admission that holds the locks.
/// removed member is refused too. A kind:9000 add that waits on an
/// uncommitted archive is refused the same way, and its event is not
/// stored. The other order: an archive waits for an admission that holds
/// the locks.
/// The archive here is the TTL reaper's single UPDATE, which takes no
/// advisory lock. Mutations: drop the row lock from
/// `lock_channel_for_member_put`, or the archived refusal from
/// `put_members_on` → RED.
/// advisory lock. The join and the kind:9000 add each wait on their own
/// archive, so neither queues behind the other's membership lock.
/// Mutations: drop the row lock from `lock_channel_for_member_put`, take
/// only the membership lock in `put_members_with_event`, or drop the
/// archived refusal from `put_members_on` → RED.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn put_member_waits_for_a_concurrent_archive() {
let pool = setup_pool().await;
let community_id = make_test_community(&pool).await;
let community = CommunityId::from_uuid(community_id);
let owner = random_pubkey();
let owner_keys = Keys::generate();
let owner = owner_keys.public_key().to_bytes().to_vec();
let joiner = random_pubkey();
let removed = random_pubkey();
for pk in [&owner, &joiner, &removed] {
let invitee = random_pubkey();
for pk in [&owner, &joiner, &removed, &invitee] {
ensure_user(&pool, community, pk)
.await
.expect("ensure user");
@@ -3956,16 +3964,81 @@ mod postgres_tests {
let readd = put_member(&pool, community, channel.id, &removed, None, Some(&owner))
.await
.expect_err("an archived channel does not reactivate a removed member");
assert!(matches!(readd, DbError::AccessDenied(_)), "{readd:?}");
assert!(
matches!(&readd, DbError::AccessDenied(reason) if reason == "channel is archived"),
"{readd:?}"
);
// A kind:9000 add stores its event in the same transaction as the
// write, so it takes the same locks: it waits on an uncommitted
// archive, then is refused and stores nothing.
crate::channel::unarchive_channel(&pool, community, channel.id)
.await
.expect("unarchive before the kind:9000 race");
let mut archive = pool.begin().await.expect("begin second archive");
sqlx::query(
"UPDATE channels SET archived_at = NOW() \
WHERE community_id = $1 AND id = $2 AND archived_at IS NULL",
)
.bind(community_id)
.bind(channel.id)
.execute(&mut *archive)
.await
.expect("archive again");
let add = nostr::EventBuilder::new(nostr::Kind::Custom(9000), "")
.tags([
nostr::Tag::parse(["h", &channel.id.to_string()]).unwrap(),
nostr::Tag::parse(["p", &hex::encode(&invitee)]).unwrap(),
])
.sign_with_keys(&owner_keys)
.expect("sign 9000");
let add_pool = crate::test_support::named_pool("put-members-event-archive-race").await;
let add_write = tokio::spawn({
let channel_id = channel.id;
let add = add.clone();
let invitee = invitee.clone();
async move {
let targets = [MemberPut {
pubkey: &invitee,
requested: None,
}];
put_members_with_event(&add_pool, None, community, channel_id, &add, &targets, None)
.await
}
});
crate::test_support::wait_for_lock_wait(&pool, "put-members-event-archive-race").await;
assert!(
!add_write.is_finished(),
"the kind:9000 add must wait for the archive"
);
archive.commit().await.expect("commit second archive");
let write = add_write
.await
.expect("kind:9000 task")
.expect("kind:9000 write");
assert!(
matches!(&write, MemberPutEventWrite::Refused(errors)
if matches!(errors.as_slice(),
[DbError::AccessDenied(reason)] if reason == "channel is archived")),
"expected an archived refusal"
);
assert!(
crate::event::get_event_by_id(&pool, community, add.id.as_bytes())
.await
.expect("read event")
.is_none(),
"a refused kind:9000 is not stored"
);
let mut roles = Vec::new();
for pk in [&owner, &joiner, &removed] {
for pk in [&owner, &joiner, &removed, &invitee] {
roles.push(
get_member_role(&pool, community, channel.id, pk)
.await
.expect("read role"),
);
}
assert_eq!(roles, [Some("owner".to_string()), None, None]);
assert_eq!(roles, [Some("owner".to_string()), None, None, None]);
// The other order: an archive waits for an admission that holds the
// locks, and the member it admitted stays a member.

9
crates/buzz-relay/src/api/invites.rs
View File

@@ -1532,13 +1532,16 @@ mod postgres_tests {
let joiner_hex = joiner_hex.clone();
async move { auto_join_default_channels(&tenant, &state, &joiner_hex).await }
});
// Wait until the join is blocked behind the archive, so it has
// already listed the channel and passed the join body's check.
// Wait until the join is blocked on the channel row lock in
// `put_member`, behind the archive, so it has already listed the
// channel and passed the join body's check.
let deadline = std::time::Instant::now() + Duration::from_secs(10);
loop {
let waiting: bool = sqlx::query_scalar(
"SELECT EXISTS (SELECT 1 FROM pg_stat_activity \
WHERE $1 = ANY(pg_blocking_pids(pid)))",
WHERE wait_event_type = 'Lock' \
AND $1 = ANY(pg_blocking_pids(pid)) \
AND query LIKE 'SELECT archived_at FROM channels %FOR NO KEY UPDATE')",
)
.bind(archive_pid)
.fetch_one(&pool)