Support MSC4140: Delayed events (Futures)

[AndrewFerr]

See: MSC4140

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct
  (run the linters)

[anoadragon453]

@AndrewFerr would it be possible to break this PR up into a few more logical commits? Currently it's roughly 1 commit with >1000 lines changed, which appears daunting to review 🙂

[AndrewFerr]

Ironically, the reason I had squashed the commits was to try make it easier to review 🙃

I've now split them up to isolate what should be the most relevant components of the PR.

[anoadragon453]

Thanks for all the time you've spent (and patiently waited) on this. This now looks good to me, and the added tests help.

I see you have a matching Complement branch here: matrix-org/complement#734. I assume you would like to hold off on merging this PR until those tests pass?

Finally, once this is merged and done, if you have some time to document how the whole mechanism works (however detailed or briefly) in https://element-hq.github.io/synapse/latest/development/internal_documentation/index.html, that would help future development efforts.

[AndrewFerr]

I see you have a matching Complement branch here: matrix-org/complement#734. I assume you would like to hold off on merging this PR until those tests pass?

Yes, as long as it's permissible to merge Complement tests before any implementations are merged.

Finally, once this is merged and done, if you have some time to document how the whole mechanism works (however detailed or briefly) in https://element-hq.github.io/synapse/latest/development/internal_documentation/index.html, that would help future development efforts.

Noted!

[anoadragon453]

Typically we'll merge them at the same time. However, yes, technically the Complement tests can be merged beforehand, as they won't be enabled on develop until this PR is merged.

[anoadragon453]

(Complement CI is expected to fail until matrix-org/complement#734 is merged.)

[anoadragon453]

matrix-org/complement#734 has been merged. I've re-run the failed Complement jobs: https://github.com/element-hq/synapse/actions/runs/10891540744?pr=17326

[anoadragon453]

LGTM, assuming the Complement tests are happy.

old

[AndrewFerr]

assuming the Complement tests are happy

They aren't 🙃 Looking into it now. The failures reproduce locally, so I should have a fix ready very soon.

[anoadragon453]

@AndrewFerr looks like there's complement failures on the delayed event tests: https://github.com/element-hq/synapse/actions/runs/10891540744/job/30325234632?pr=17326

Edit: GitHub failed to show your last comment 🤦

[AndrewFerr]

Looks like the Complement failures are false-positives when running in Postgres. I'm updating the Complement tests to account for that.

[AndrewFerr]

The Postgres tests should be fixed by matrix-org/complement#737. They pass when I run them locally, in either monolith or worker mode.

[AndrewFerr]

Alright, I added an extra commit to satisfy what the Complement test was failing on, and now we don't have to worry about merging matrix-org/complement#737 first.

The test expects GET /delayed_events to return events in the order they'll be sent, but no such order was guaranteed when using Postgres. dfde3c2 applies the ordering the test expects.

Strictly speaking, the test doesn't have to cover ordering on GET /delayed_events because the MSC doesn't (yet) define an ordering, so I wrote matrix-org/complement@279d1b3 to remove that part of the test.

The other test failure was simply due to leftover data after the first test failed, so I wrote matrix-org/complement@6f86880 to make tests do some teardown to prevent unrelated failures later on.

Note that all tests will continue to pass with matrix-org/complement#737, so it can still be merged safely, but now it is optional to do so 🙂