Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Mark /relations endpoint as usable on workers. #14028

Merged
merged 8 commits into from
Oct 12, 2022
Merged

Conversation

clokep
Copy link
Member

@clokep clokep commented Oct 3, 2022

This has always been safe to my knowledge, but now there are complement tests (matrix-org/complement#467).

Fixes #12290

@clokep clokep marked this pull request as ready for review October 3, 2022 16:33
@clokep clokep requested a review from a team as a code owner October 3, 2022 16:33
@clokep clokep changed the title Mark relations as usable on workers. Mark /relations endpoint as usable on workers. Oct 3, 2022
realtyem added a commit to realtyem/synapse-unraid that referenced this pull request Oct 3, 2022
@realtyem
Copy link
Contributor

realtyem commented Oct 3, 2022

For what it's worth, I get an error on isolated client_reader tests.
My test repo

@clokep
Copy link
Member Author

clokep commented Oct 3, 2022

Hmmm, maybe we're not mounting that properly, but why isn't it failing in this repo?! 😢

@clokep clokep removed the request for review from a team October 3, 2022 20:13
@realtyem
Copy link
Contributor

realtyem commented Oct 3, 2022 via email

@clokep
Copy link
Member Author

clokep commented Oct 3, 2022

Ah ok, so this goes to my comment above saying that I don't. Now if adding the config is all that's needed. Thanks for answering!

@realtyem
Copy link
Contributor

realtyem commented Oct 4, 2022

That looks good for testing with the docker image. Outside of that is out of my scope of expertise. For example: I don't know enough about the internals of how generic_workers become actual workers to say that defining an endpoint and giving it an arbitrary name is sufficient. Documentation is...misleading, in this aspect.

scripts-dev/complement.sh Outdated Show resolved Hide resolved
@clokep clokep requested a review from a team October 4, 2022 20:03
Co-authored-by: Eric Eastwood <erice@element.io>
MadLittleMods added a commit to matrix-org/complement that referenced this pull request Oct 6, 2022
Spawning from matrix-org/synapse#14028 (comment)

The reason they failed before was because we're fighting against
stale caches across workers racing while waiting for invalidation,
see matrix-org/synapse#13185 (comment)

> Here is what happens:
>
>  1. `serverB` has `event1` stored as an `outlier` from previous requests (specifically from MSC3030 jump to date pulling in a missing `prev_event` after backfilling)
>  1. Client on `serverB` calls `/messages?dir=b`
>  1. `serverB:client_reader1` accepts the request and drives things
>  1. `serverB:client_reader1` has some backward extremities in range and requests `/backfill` from `serverA`
>  1. `serverB:client_reader1` processes the events from backfill including `event1` and puts them in the `_event_persist_queue`
>  1. `serverB:master` picks up the events from the `_event_persist_queue` and persists them to the database, de-outliers `event1` and invalidates its own cache and sends them over replication
>  1. `serverB:client_reader1` starts assembling the `/messages` response and gets `event1` out of the stale cache still as an `outlier`
>  1. `serverB:client_reader1` responds to the `/messages` request without `event1` because `outliers` are filtered out
>  1. `serverB:client_reader1` finally gets the replication data and invalidates its own cache for `event1` (too late, we already got the events from the stale cache and responded)
Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

Can you update the docs/workers.md documentation please?

@clokep
Copy link
Member Author

clokep commented Oct 12, 2022

Can you update the docs/workers.md documentation please?

This PR already updates that w/ the new endpoint. Is there an additional change you'd like to see?

@erikjohnston
Copy link
Member

Can you update the docs/workers.md documentation please?

This PR already updates that w/ the new endpoint. Is there an additional change you'd like to see?

I swear that wasn't there before!!!!

@clokep clokep merged commit c604d2c into develop Oct 12, 2022
@clokep clokep deleted the clokep/rel-workers branch October 12, 2022 10:46
realtyem added a commit to realtyem/synapse-old that referenced this pull request Oct 12, 2022
clokep added a commit that referenced this pull request Oct 12, 2022
Co-authored-by: Eric Eastwood <erice@element.io>
MadLittleMods added a commit to matrix-org/complement that referenced this pull request Oct 13, 2022
Spawning from matrix-org/synapse#14028 (comment)

CI failure: https://github.com/matrix-org/synapse/actions/runs/3182998161/jobs/5189731097#step:6:15343 (from [discussion](matrix-org/synapse#14028 (comment)))
```
WORKERS=1 POSTGRES=1 COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS=1 COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh -run TestJumpToDateEndpoint/parallel/federation/can_paginate_after_getting_remote_event_from_timestamp_to_event_endpoint
```

### Why did this test fail with workers before?

The reason they failed before was because we're fighting against stale caches across workers racing while waiting for invalidation, see matrix-org/synapse#13185 (comment)

> Here is what happens:
>
>  1. `serverB` has `event1` stored as an `outlier` from previous requests (specifically from MSC3030 jump to date pulling in a missing `prev_event` after backfilling)
>  1. Client on `serverB` calls `/messages?dir=b`
>  1. `serverB:client_reader1` accepts the request and drives things
>  1. `serverB:client_reader1` has some backward extremities in range and requests `/backfill` from `serverA`
>  1. `serverB:client_reader1` processes the events from backfill including `event1` and puts them in the `_event_persist_queue`
>  1. `serverB:master` picks up the events from the `_event_persist_queue` and persists them to the database, de-outliers `event1` and invalidates its own cache and sends them over replication
>  1. `serverB:client_reader1` starts assembling the `/messages` response and gets `event1` out of the stale cache still as an `outlier`
>  1. `serverB:client_reader1` responds to the `/messages` request without `event1` because `outliers` are filtered out
>  1. `serverB:client_reader1` finally gets the replication data and invalidates its own cache for `event1` (too late, we already got the events from the stale cache and responded)
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 29, 2022
Upstream changes:

Synapse 1.70.1 (2022-10-28)
===========================

(bugfixes)


Synapse 1.70.0 (2022-10-26)
===========================

Features
--------

- Support for
  [MSC3856](matrix-org/matrix-spec-proposals#3856):
  threads list
  API. ([\#13394](matrix-org/synapse#13394),
  [\#14171](matrix-org/synapse#14171),
  [\#14175](matrix-org/synapse#14175))

- Support for thread-specific notifications & receipts
  ([MSC3771](matrix-org/matrix-spec-proposals#3771)
  and
  [MSC3773](matrix-org/matrix-spec-proposals#3773)). ([\#13776](matrix-org/synapse#13776),
  [\#13824](matrix-org/synapse#13824),
  [\#13877](matrix-org/synapse#13877),
  [\#13878](matrix-org/synapse#13878),
  [\#14050](matrix-org/synapse#14050),
  [\#14140](matrix-org/synapse#14140),
  [\#14159](matrix-org/synapse#14159),
  [\#14163](matrix-org/synapse#14163),
  [\#14174](matrix-org/synapse#14174),
  [\#14222](matrix-org/synapse#14222))

- Stop fetching missing `prev_events` after we already know their
  signature is
  invalid. ([\#13816](matrix-org/synapse#13816))

- Send application service access tokens as a header (and query
  parameter). Implements
  [MSC2832](matrix-org/matrix-spec-proposals#2832). ([\#13996](matrix-org/synapse#13996))

- Ignore server ACL changes when generating pushes. Implements
  [MSC3786](matrix-org/matrix-spec-proposals#3786). ([\#13997](matrix-org/synapse#13997))

- Experimental support for redirecting to an implementation of a
  [MSC3886](matrix-org/matrix-spec-proposals#3886)
  HTTP rendezvous
  service. ([\#14018](matrix-org/synapse#14018))

- The `/relations` endpoint can now be used on
  workers. ([\#14028](matrix-org/synapse#14028))

- Advertise support for Matrix 1.3 and 1.4 on
  `/_matrix/client/versions`. ([\#14032](matrix-org/synapse#14032),
  [\#14184](matrix-org/synapse#14184))

- Improve validation of request bodies for the [Device
  Management](https://spec.matrix.org/v1.4/client-server-api/#device-management)
  and [MSC2697 Device
  Dehyrdation](matrix-org/matrix-spec-proposals#2697)
  client-server API
  endpoints. ([\#14054](matrix-org/synapse#14054))

- Experimental support for
  [MSC3874](matrix-org/matrix-spec-proposals#3874):
  Filtering threads from the `/messages`
  endpoint. ([\#14148](matrix-org/synapse#14148))

- Improve the validation of the following PUT endpoints:
  [`/directory/room/{roomAlias}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3directoryroomroomalias),
  [`/directory/list/room/{roomId}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3directorylistroomroomid)
  and
  [`/directory/list/appservice/{networkId}/{roomId}`](https://spec.matrix.org/v1.4/application-service-api/#put_matrixclientv3directorylistappservicenetworkidroomid). ([\#14179](matrix-org/synapse#14179))


Deprecations and Removals
-------------------------

- Remove the experimental implementation of
  [MSC3772](matrix-org/matrix-spec-proposals#3772). ([\#14094](matrix-org/synapse#14094))

- Remove the unstable identifier for
  [MSC3715](matrix-org/matrix-spec-proposals#3715). ([\#14106](matrix-org/synapse#14106),
  [\#14146](matrix-org/synapse#14146))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run /relations on workers
4 participants