Skip to content

PLAT-38547 Account-data compare-and-swap - #64

Open
SpiritCroc wants to merge 3 commits into
beeperfrom
tobias/plat-38547-account-data-expect
Open

PLAT-38547 Account-data compare-and-swap#64
SpiritCroc wants to merge 3 commits into
beeperfrom
tobias/plat-38547-account-data-expect

Conversation

@SpiritCroc

Copy link
Copy Markdown
Contributor

No description provided.

@linear-code

linear-code Bot commented Aug 21, 2026

Copy link
Copy Markdown

PLAT-38547

@SpiritCroc
SpiritCroc marked this pull request as ready for review August 21, 2026 15:39
@indent

indent Bot commented Aug 21, 2026

Copy link
Copy Markdown
PR Summary

Adds a Beeper compare-and-swap (CAS) mechanism for account data. Clients may pass a com.beeper.expect_revision_id query param on account-data PUTs; the write only lands if the stored content's com.beeper.revision_id matches, otherwise the server returns 409 COM.BEEPER.REVISION_ID_MISMATCH with the current content so the client can retry. The CAS check is enforced atomically in the storage layer and handles the no-existing-row race and concurrent writers.

  • New error code COM.BEEPER.REVISION_ID_MISMATCH in synapse/api/errors.py.
  • expected_revision_id threaded through REST servlets, AccountDataHandler, the replication endpoints, and the storage writes.
  • New _upsert_account_data_txn enforces CAS: SELECT ... FOR UPDATE (Postgres), compares the stored revision id, and either raises 409 (with com.beeper.current_content) or upserts; the no-row case uses INSERT ... ON CONFLICT DO NOTHING + re-read to serialize concurrent first writes.
  • Room account-data writes moved from simple_upsert to runInteraction so the CAS read and write share one transaction.
  • The 409 error, including its extra fields, propagates verbatim across workers via the replication layer.

Issues

1 potential issue found:

  • Both account-data PUT servlets parse com.beeper.expect_revision_id but silently ignore it when MSC3391 is enabled and the body is {}, routing to the unconditional delete path; a client using CAS to clear account data loses the precondition. Latent: only triggers when experimental_features.msc3391_enabled is true (off by default).

Select any checkbox above to have Indent auto-fix the issue

CI Checks

Both failures are unrelated to the account-data changes. build-python fails building the Docker image because the branch name tobias/plat-38547-account-data-expect contains slashes, producing an invalid image tag; test-complement fails on TestThreadsEndpoint (a threads/relations ordering assertion) which does not touch account data and is a pre-existing flake. Lint, types, trial, and sytest all pass.

Failing build-python (github-actions)
  • The build step tags the image with the branch name for non-beeper refs: --tag .../synapse:tobias/plat-38547-account-data-expect-<sha>. Docker rejects this: invalid tag ...: invalid reference format, because Docker tags cannot contain /. This affects any branch whose name contains a slash and is unrelated to the PR's code; it would need a CI workflow change to sanitize the branch name before using it as a tag.
Failing test-complement (github-actions)
  • The only failing test is TestThreadsEndpoint (room_threads_test.go:89): HaveInOrder: index 0 got $L6w...1_1Y| want $L6w...1_1Y|$AMN...NTw, an ordering assertion on thread root events. This path does not exercise account data and is unrelated to the CAS change (the account-data complement tests TestAddAccountData passed). It looks like a pre-existing/flaky ordering test; re-running is the likely resolution.
Bulk Actions
  • Autofix all issues

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant