Skip to content

perf(relay): index default channels and read them from the replica at AUTH - #9222

Open
TheSentinel454 wants to merge 5 commits into
mainfrom
elrond/auto-join-auth-index
Open

TheSentinel454 wants to merge 5 commits into
mainfrom
elrond/auto-join-auth-index

Conversation

@TheSentinel454

@TheSentinel454 TheSentinel454 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

In roster-free communities, every AUTH runs list_never_joined_auto_join_channels to join the caller to the community's default (auto_join) channels. Two changes make that read cheap, plus one recheck that keeps a stale read from joining an unflagged channel.

  1. Partial index idx_channels_auto_join on channels (community_id) WHERE auto_join AND channel_type = 'stream' AND visibility = 'open' AND archived_at IS NULL AND deleted_at IS NULL. Without it, the query plans through idx_channels_community_visibility, reads every open channel in the community, and filters nearly all of them away, because default channels are a handful out of thousands. With it, the planner probes only the flagged rows.
  2. Read from the bounded replica. Db::list_never_joined_auto_join_channels_routed routes through route_read(.., RoutePredicate::Bounded, ReaderOperation::Authorization), the same as pending_default_channel_join_routed. A replica error falls back to the writer, and a cancelled statement propagates. The only production caller, join_never_joined_channels in side_effects.rs, uses it. With no replica budget configured, it reads the writer as before.

Why a stale read is safe

put_first_member decides on the writer, under the channel's membership lock, so a stale candidate list cannot cause a wrong join:

  • A channel the replica still lists as unjoined is skipped when a membership row already exists.
  • A channel unflagged within the replica budget is skipped. put_first_member now rechecks auto_join under the lock. Both of its callers are default-channel joins.
  • A channel archived or made private within the budget is refused by put_member and counted in buzz_auto_join_failures_total, as today.
  • A channel flagged within the budget is joined on the next admission.

Database review checklist (docs/database.md)

  • Phase: expand only. Nothing comes before it. Nothing needs to follow: the deployed relay's query uses the index as soon as it exists.

  • No trigger, function, procedure, rule, or foreign key is added.

  • No migration mutates existing rows, and there is no backfill.

  • Not a contract migration.

  • Rule 4 locations: migrations/0087_channels_auto_join_index.sql and schema/tables/public/channels.sql. admin_schema_parity_between_desired_state_and_migrations now compares idx_channels_auto_join alongside idx_channels_listed_name, between the desired-state and migrated catalogs. No new files, so schema/schema.sql is unchanged. A partial btree index needs no reconcile-schema-after-pgschema.sql step.

  • Deployed relay: the index is additive and the SQL text is unchanged. The running relay ignores it or uses it. Its plan was checked with EXPLAIN on a migrated database (below).

  • Locks and build: SET LOCAL lock_timeout = '1s' and statement_timeout = '5s'. The migration builds the index inline only when to_regclass finds none, then verifies pg_get_indexdef and indisvalid/indisready/indislive, and stops if the definition differs. Prebuild for deployments that apply migrations out of band (in the migration header):

    CREATE INDEX CONCURRENTLY idx_channels_auto_join ON public.channels (community_id)
      WHERE auto_join AND channel_type = 'stream' AND visibility = 'open'
        AND archived_at IS NULL AND deleted_at IS NULL;

Migration number

0087. Main is at 0086. #9108 (#9108) also needs new numbers. Whichever PR merges second renumbers.

Tests

  • never_joined_auto_join_channels_routed_reads_the_bounded_replica (buzz-db, postgres_tests) seeds a writer and a replica with different default channels. It asserts the writer's channel with no budget, the replica's with a 5 s budget, and the writer's again when the replica fence is stale. A mutant that always reads the writer fails it.
  • embedded_migrator_contains_consolidated_initial_schema pins the migration count, the timeouts, and the absence of CONCURRENTLY.
  • The parity test covers the new index.
  • first_member_put_joins_once_and_never_undoes_a_leave now also asserts that put_first_member writes nothing for a channel that is not, or is no longer, auto_join.
  • auto_join_index_migration_accepts_a_prebuild_without_waiting_for_writers runs 0087 over a concurrent prebuild while a channel writer holds its lock: it neither waits nor replaces the index. migration_schema_listed_name_index_validates_a_prebuild now stops at 0083, since later migrations that build their own index inline need the lock it holds.
  • On a fully migrated scratch database, re-running 0087 against an existing index succeeds, and running it against a wrong-definition index stops. EXPLAIN of the query, as both a custom and a generic (prepared) plan, uses idx_channels_auto_join.

What got simpler

Nothing was removed, but the concept count stays flat. The routed method replaces the writer-only Db method instead of sitting beside it. Both arms share one connection-level helper, so the SQL lives in one place.

🤖 Generated with Claude Code

@TheSentinel454
TheSentinel454 marked this pull request as ready for review November 6, 2026 22:14
@TheSentinel454
TheSentinel454 requested a review from a team as a code owner November 6, 2026 22:14
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

  • Exact PR diff: 96af488d968a1ae9d961c2132bea27822735f8da...99c07da83aee4d72cf97199b2363923e93f29ca4
  • Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.


Review Summary

Overall Risk: NONE

No concrete findings in the authorized PR range. Replica routing preserves community scoping, and writer-side locks and validation protect default-channel membership changes.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

  • Review used read-only source and history inspection. Builds, tests, and database execution were not performed.

Generated by Codex Security Review |
Requested by: @TheSentinel454 |
Workflow run

@TheSentinel454
TheSentinel454 force-pushed the elrond/auto-join-auth-index branch from a50be7a to 2cfb952 Compare November 6, 2026 22:19
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 6, 2026
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 6, 2026
@github-actions github-actions Bot added codex-security-review-current The posted Codex security review matches its recorded range. and removed codex-security-review-current The posted Codex security review matches its recorded range. labels Nov 6, 2026
TheSentinel454 and others added 4 commits November 6, 2026 18:45
… AUTH

On a community that does not require relay membership, every admitted human
AUTH lists the default channels the person has never joined. That read walked
every open channel in the community on the writer pool to find the few with
auto_join set.

- Migration 0085 adds a partial index on channels (community_id) holding only
  effective default channels (flagged, open, unarchived, not deleted stream
  channels), built inline under 1s lock / 5s statement budgets as 0049 does,
  with an exact-definition check so a prebuilt CONCURRENTLY index verifies.
- Db::list_never_joined_auto_join_channels_routed reads the bounded replica,
  falling back to the writer as pending_default_channel_join_routed does.
  put_first_member decides again under the membership lock, so a stale
  answer can only skip a channel joined since or defer one flagged since.

No behavior change.

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>
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
…build

The 0083 prebuild test held a channels writer lock while running every
migration, so 0087's inline index build hit its lock timeout. Run it only
through 0083, and add the matching check that 0087 over a concurrent
prebuild neither waits for writers nor replaces the index.

Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
@TheSentinel454
TheSentinel454 force-pushed the elrond/auto-join-auth-index branch from 45d812c to 8b74366 Compare November 6, 2026 23:46
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 6, 2026
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 6, 2026
The AUTH auto-join list can now come from the bounded replica, so a
channel unflagged within the replica budget could still be listed.
put_first_member already decides membership history under the channel
lock; it now also skips a channel that is no longer auto_join, so a
stale candidate writes nothing. Both callers are default-channel joins.

Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 7, 2026
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 7, 2026

This branch was successfully deployed

1 active deployment
codex-review — 99c07da8 Deployed Nov 7, 2026 by TheSentinel454 via Run Codex Security Review #13694
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant