Skip to content

ci: Update Synapse to start alongside index import - #5854

Closed
backspace wants to merge 1 commit into
mainfrom
cs-12583-early-synapse
Closed

ci: Update Synapse to start alongside index import#5854
backspace wants to merge 1 commit into
mainfrom
cs-12583-early-synapse

Conversation

@backspace

Copy link
Copy Markdown
Contributor

These aren’t interrelated, so might as well happen in parallel.

Claude explanationThe shard waits ~34s in `create realm users` for Synapse to finish booting, then imports nothing from it — the 27s index import that runs just before is entirely independent. Overlap them.

This is a different trade from speeding up a step that precedes service start. Measured on #5849, pulling the mirrored images concurrently cut that step from 29s to 13s and the readiness wait grew from 1s to 38s across every shard: the services simply started earlier and the same wall remained. Synapse is on the critical path rather than in front of it — realm-server logs into it during boot — so moving its boot earlier shortens the chain instead of relocating the wait.

Backgrounded, because assert-synapse-running blocks on Synapse's own healthcheck. It is idempotent, so test-services:host finds the container up when it reaches start:matrix and only re-registers the Traefik route; the container exists within seconds, long before that step, so they cannot race to create it.

Expected: create realm users falls toward zero and the readiness wait does not absorb it. If the wait grows instead, the same wall applies here too and the readiness path itself is the only thing left to attack.

The shard waits ~34s in `create realm users` for Synapse to finish booting,
then imports nothing from it — the 27s index import that runs just before is
entirely independent. Overlap them.

This is a different trade from speeding up a step that precedes service
start. Measured on #5849, pulling the mirrored images concurrently cut that
step from 29s to 13s and the readiness wait grew from 1s to 38s across every
shard: the services simply started earlier and the same wall remained. Synapse
is on the critical path rather than in front of it — realm-server logs into it
during boot — so moving its boot earlier shortens the chain instead of
relocating the wait.

Backgrounded, because `assert-synapse-running` blocks on Synapse's own
healthcheck. It is idempotent, so `test-services:host` finds the container up
when it reaches `start:matrix` and only re-registers the Traefik route; the
container exists within seconds, long before that step, so they cannot race to
create it.

Expected: `create realm users` falls toward zero and the readiness wait does
not absorb it. If the wait grows instead, the same wall applies here too and
the readiness path itself is the only thing left to attack.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b95f7fe18

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- name: Start Synapse ahead of the index import
working-directory: packages/matrix
run: |
pnpm assert-synapse-running >> /tmp/server.log 2>&1 &

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize the two Synapse starters

When the cached-index step is skipped—on every push/workflow-dispatch run and on PRs that change index behavior—this backgrounded command can overlap almost immediately with test-services:host. I checked mise-tasks/test-services/host:91-93 and packages/realm-server/scripts/start-matrix.sh:10; that path invokes the same assert-synapse-running, whose docker ps/start sequence in packages/matrix/scripts/assert-synapse-running.sh:15-32 has no locking. Both processes can therefore decide boxel-synapse-ci is absent and concurrently stop/start it while rewriting the shared synapse-data-ci configuration, producing intermittent container-name conflicts or a container booted with mismatched generated secrets. Gate the early start on an actual import or add a startup/readiness barrier before the service stack invokes the second starter.

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files      1 suites   1h 52m 21s ⏱️
4 368 tests 4 354 ✅ 14 💤 0 ❌
4 387 runs  4 373 ✅ 14 💤 0 ❌

Results for commit 3b95f7f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant