Wait for lazy join to complete when getting current state #12872

erikjohnston · 2022-05-25T11:05:07Z

~~Based on #12871~~

Reviewable commit-by-commit

erikjohnston · 2022-05-25T14:40:25Z

synapse/storage/state.py

+               - list of current_state_delta_stream rows. If it is empty, we are
+                 up to date.
+        """
+        # FIXME(faster room joins): what do we do here?


I'm not sure how we want to deal with state deltas?

errr, nor me. I guess we want to wait for full state though?

Suggested change

# FIXME(faster room joins): what do we do here?

# FIXME(faster_joins): what do we do here?

Yeah, though the query isn't limited to a room so we'd effectively by blocking all forward progress on handling deltas until it completes?

erikjohnston · 2022-05-25T14:41:56Z

synapse/handlers/sync.py

+                    # we don't want to block `/sync` on finishing a lazy join.
+                    # Is this the correct way of doing it?
+                    current_state_ids_map = (
+                        await self.store.get_partial_current_state_ids(room_id)


This is a check to see if the state event we're about to send down to clients is in the current state, and if so to always send it even if filter_events_for_client would filter it out. Not sure if using get_partial_current_state_ids is the right approach here?

I'm not really following, but this is something we can return to.

synapse/storage/state.py

synapse/handlers/room_member.py

synapse/handlers/room_summary.py

synapse/storage/databases/main/user_directory.py

richvdh · 2022-05-26T13:32:55Z

synapse/storage/state.py

+               - list of current_state_delta_stream rows. If it is empty, we are
+                 up to date.
+        """
+        # FIXME(faster room joins): what do we do here?


errr, nor me. I guess we want to wait for full state though?

Suggested change

# FIXME(faster room joins): what do we do here?

# FIXME(faster_joins): what do we do here?

synapse/storage/state.py

synapse/storage/databases/main/room.py

synapse/handlers/sync.py

richvdh · 2022-05-26T14:41:42Z

synapse/handlers/sync.py

+                    # we don't want to block `/sync` on finishing a lazy join.
+                    # Is this the correct way of doing it?
+                    current_state_ids_map = (
+                        await self.store.get_partial_current_state_ids(room_id)


I'm not really following, but this is something we can return to.

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

…t_state

richvdh · 2022-05-30T12:08:44Z

I'm waiting for #12913 to land before having another look at this.

…t_state

Stale

richvdh

generally looks good. a few comments.

richvdh · 2022-06-01T13:29:19Z

synapse/storage/_base.py

@@ -77,7 +77,7 @@ def _invalidate_state_caches(

        # Purge other caches based on room state.
        self._attempt_to_invalidate_cache("get_room_summary", (room_id,))
-        self._attempt_to_invalidate_cache("get_current_state_ids", (room_id,))
+        self._attempt_to_invalidate_cache("get_partial_current_state_ids", (room_id,))


I think this means a rolling restart is unwise. We should either document it, or arrange for it to be safe.

I think this is fine, as we're actually not streaming the name of the caches over replication for these, we're just sending "invalidate all current state caches kthx" instead. (_attempt_to_invalidate_cache only invalidates things on the local process)

synapse/storage/databases/main/room.py

changelog.d/12872.misc

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

richvdh · 2022-06-01T13:29:20Z

synapse/storage/_base.py

@@ -77,7 +77,7 @@ def _invalidate_state_caches(

        # Purge other caches based on room state.
        self._attempt_to_invalidate_cache("get_room_summary", (room_id,))
-        self._attempt_to_invalidate_cache("get_current_state_ids", (room_id,))
+        self._attempt_to_invalidate_cache("get_partial_current_state_ids", (room_id,))


I think this means a rolling restart is unwise. We should either document it, or arrange for it to be safe.

richvdh · 2022-06-01T14:52:24Z

synapse/storage/_base.py

@@ -77,7 +77,7 @@ def _invalidate_state_caches(

        # Purge other caches based on room state.
        self._attempt_to_invalidate_cache("get_room_summary", (room_id,))
-        self._attempt_to_invalidate_cache("get_current_state_ids", (room_id,))
+        self._attempt_to_invalidate_cache("get_partial_current_state_ids", (room_id,))


erikjohnston · 2022-06-01T15:02:45Z

Woop, thanks!

@olmari

Synapse 1.61.0 (2022-06-14) =========================== This release removes support for the non-standard feature known both as 'groups' and as 'communities', which have been superseded by *Spaces*. See [the upgrade notes](https://github.com/matrix-org/synapse/blob/develop/docs/upgrade.md#upgrading-to-v1610) for more details. Improved Documentation ---------------------- - Mention removed community/group worker endpoints in [upgrade.md](https://github.com/matrix-org/synapse/blob/develop/docs/upgrade.md#upgrading-to-v1610s). Contributed by @olmari. ([\matrix-org#13023](matrix-org#13023)) Synapse 1.61.0rc1 (2022-06-07) ============================== Features -------- - Add new `media_retention` options to the homeserver config for routinely cleaning up non-recently accessed media. ([\matrix-org#12732](matrix-org#12732), [\matrix-org#12972](matrix-org#12972), [\matrix-org#12977](matrix-org#12977)) - Experimental support for [MSC3772](matrix-org/matrix-spec-proposals#3772): Push rule for mutually related events. ([\matrix-org#12740](matrix-org#12740), [\matrix-org#12859](matrix-org#12859)) - Update to the `check_event_for_spam` module callback: Deprecate the current callback signature, replace it with a new signature that is both less ambiguous (replacing booleans with explicit allow/block) and more powerful (ability to return explicit error codes). ([\matrix-org#12808](matrix-org#12808)) - Add storage and module API methods to get monthly active users (and their corresponding appservices) within an optionally specified time range. ([\matrix-org#12838](matrix-org#12838), [\matrix-org#12917](matrix-org#12917)) - Support the new error code `ORG.MATRIX.MSC3823.USER_ACCOUNT_SUSPENDED` from [MSC3823](matrix-org/matrix-spec-proposals#3823). ([\matrix-org#12845](matrix-org#12845), [\matrix-org#12923](matrix-org#12923)) - Add a configurable background job to delete stale devices. ([\matrix-org#12855](matrix-org#12855)) - Improve URL previews for pages with empty elements. ([\matrix-org#12951](matrix-org#12951)) - Allow updating a user's password using the admin API without logging out their devices. Contributed by @jcgruenhage. ([\matrix-org#12952](matrix-org#12952)) Bugfixes -------- - Always send an `access_token` in `/thirdparty/` requests to appservices, as required by the [Application Service API specification](https://spec.matrix.org/v1.1/application-service-api/#third-party-networks). ([\matrix-org#12746](matrix-org#12746)) - Implement [MSC3816](matrix-org/matrix-spec-proposals#3816): sending the root event in a thread should count as having 'participated' in it. ([\matrix-org#12766](matrix-org#12766)) - Delete events from the `federation_inbound_events_staging` table when a room is purged through the admin API. ([\matrix-org#12784](matrix-org#12784)) - Fix a bug where we did not correctly handle invalid device list updates over federation. Contributed by Carl Bordum Hansen. ([\matrix-org#12829](matrix-org#12829)) - Fix a bug which allowed multiple async operations to access database locks concurrently. Contributed by @sumnerevans @ Beeper. ([\matrix-org#12832](matrix-org#12832)) - Fix an issue introduced in Synapse 0.34 where the `/notifications` endpoint would only return notifications if a user registered at least one pusher. Contributed by Famedly. ([\matrix-org#12840](matrix-org#12840)) - Fix a bug where servers using a Postgres database would fail to backfill from an insertion event when MSC2716 is enabled (`experimental_features.msc2716_enabled`). ([\matrix-org#12843](matrix-org#12843)) - Fix [MSC3787](matrix-org/matrix-spec-proposals#3787) rooms being omitted from room directory, room summary and space hierarchy responses. ([\matrix-org#12858](matrix-org#12858)) - Fix a bug introduced in Synapse 1.54.0 which could sometimes cause exceptions when handling federated traffic. ([\matrix-org#12877](matrix-org#12877)) - Fix a bug introduced in Synapse 1.59.0 which caused room deletion to fail with a foreign key violation error. ([\matrix-org#12889](matrix-org#12889)) - Fix a long-standing bug which caused the `/messages` endpoint to return an incorrect `end` attribute when there were no more events. Contributed by @Vetchu. ([\matrix-org#12903](matrix-org#12903)) - Fix a bug introduced in Synapse 1.58.0 where `/sync` would fail if the most recent event in a room was a redaction of an event that has since been purged. ([\matrix-org#12905](matrix-org#12905)) - Fix a potential memory leak when generating thumbnails. ([\matrix-org#12932](matrix-org#12932)) - Fix a long-standing bug where a URL preview would break if the image failed to download. ([\matrix-org#12950](matrix-org#12950)) Improved Documentation ---------------------- - Fix typographical errors in documentation. ([\matrix-org#12863](matrix-org#12863)) - Fix documentation incorrectly stating the `sendToDevice` endpoint can be directed at generic workers. Contributed by Nick @ Beeper. ([\matrix-org#12867](matrix-org#12867)) Deprecations and Removals ------------------------- - Remove support for the non-standard groups/communities feature from Synapse. ([\matrix-org#12553](matrix-org#12553), [\matrix-org#12558](matrix-org#12558), [\matrix-org#12563](matrix-org#12563), [\matrix-org#12895](matrix-org#12895), [\matrix-org#12897](matrix-org#12897), [\matrix-org#12899](matrix-org#12899), [\matrix-org#12900](matrix-org#12900), [\matrix-org#12936](matrix-org#12936), [\matrix-org#12966](matrix-org#12966)) - Remove contributed `kick_users.py` script. This is broken under Python 3, and is not added to the environment when `pip install`ing Synapse. ([\matrix-org#12908](matrix-org#12908)) - Remove `contrib/jitsimeetbridge`. This was an unused experiment that hasn't been meaningfully changed since 2014. ([\matrix-org#12909](matrix-org#12909)) - Remove unused `contrib/experiements/cursesio.py` script, which fails to run under Python 3. ([\matrix-org#12910](matrix-org#12910)) - Remove unused `contrib/experiements/test_messaging.py` script. This fails to run on Python 3. ([\matrix-org#12911](matrix-org#12911)) Internal Changes ---------------- - Test Synapse against Complement with workers. ([\matrix-org#12810](matrix-org#12810), [\matrix-org#12933](matrix-org#12933)) - Reduce the amount of state we pull from the DB. ([\matrix-org#12811](matrix-org#12811), [\matrix-org#12964](matrix-org#12964)) - Try other homeservers when re-syncing state for rooms with partial state. ([\matrix-org#12812](matrix-org#12812)) - Resume state re-syncing for rooms with partial state after a Synapse restart. ([\matrix-org#12813](matrix-org#12813)) - Remove Mutual Rooms' ([MSC2666](matrix-org/matrix-spec-proposals#2666)) endpoint dependency on the User Directory. ([\matrix-org#12836](matrix-org#12836)) - Experimental: expand `check_event_for_spam` with ability to return additional fields. This enables spam-checker implementations to experiment with mechanisms to give users more information about why they are blocked and whether any action is needed from them to be unblocked. ([\matrix-org#12846](matrix-org#12846)) - Remove `dont_notify` from the `.m.rule.room.server_acl` rule. ([\matrix-org#12849](matrix-org#12849)) - Remove the unstable `/hierarchy` endpoint from [MSC2946](matrix-org/matrix-spec-proposals#2946). ([\matrix-org#12851](matrix-org#12851)) - Pull out less state when handling gaps in room DAG. ([\matrix-org#12852](matrix-org#12852), [\matrix-org#12904](matrix-org#12904)) - Clean-up the push rules datastore. ([\matrix-org#12856](matrix-org#12856)) - Correct a type annotation in the URL preview source code. ([\matrix-org#12860](matrix-org#12860)) - Update `pyjwt` dependency to [2.4.0](https://github.com/jpadilla/pyjwt/releases/tag/2.4.0). ([\matrix-org#12865](matrix-org#12865)) - Enable the `/account/whoami` endpoint on synapse worker processes. Contributed by Nick @ Beeper. ([\matrix-org#12866](matrix-org#12866)) - Enable the `batch_send` endpoint on synapse worker processes. Contributed by Nick @ Beeper. ([\matrix-org#12868](matrix-org#12868)) - Don't generate empty AS transactions when the AS is flagged as down. Contributed by Nick @ Beeper. ([\matrix-org#12869](matrix-org#12869)) - Fix up the variable `state_store` naming. ([\matrix-org#12871](matrix-org#12871)) - Faster room joins: when querying the current state of the room, wait for state to be populated. ([\matrix-org#12872](matrix-org#12872)) - Avoid running queries which will never result in deletions. ([\matrix-org#12879](matrix-org#12879)) - Use constants for EDU types. ([\matrix-org#12884](matrix-org#12884)) - Reduce database load of `/sync` when presence is enabled. ([\matrix-org#12885](matrix-org#12885)) - Refactor `have_seen_events` to reduce memory consumed when processing federation traffic. ([\matrix-org#12886](matrix-org#12886)) - Refactor receipt linearization code. ([\matrix-org#12888](matrix-org#12888)) - Add type annotations to `synapse.logging.opentracing`. ([\matrix-org#12894](matrix-org#12894)) - Remove PyNaCl occurrences directly used in Synapse code. ([\matrix-org#12902](matrix-org#12902)) - Bump types-jsonschema from 4.4.1 to 4.4.6. ([\matrix-org#12912](matrix-org#12912)) - Rename storage classes. ([\matrix-org#12913](matrix-org#12913)) - Preparation for database schema simplifications: stop reading from `event_edges.room_id`. ([\matrix-org#12914](matrix-org#12914)) - Check if we are in a virtual environment before overriding the `PYTHONPATH` environment variable in the demo script. ([\matrix-org#12916](matrix-org#12916)) - Improve the logging when signature checks on events fail. ([\matrix-org#12925](matrix-org#12925)) # -----BEGIN PGP SIGNATURE----- # # iQFEBAABCgAuFiEEBTGR3/RnAzBGUif3pULk7RsPrAkFAmKoaa0QHGVyaWtAbWF0 # cml4Lm9yZwAKCRClQuTtGw+sCVn3B/sF8hdhrZ7hWW40ST3eG9cEKNFrj/xZXiaI # zho3ryrxaQF68BSKot15AvZSprEdwBXWrb8WeTjyw+QH7vTKrCQDZ0p7qubn10Z7 # BuKq9hyYjyCLjBZrgy8d4U3Y8gsSByuO59YKHNLn+UTJLOs5GTH8Wprwh4mpU3Jl # +o+cC+lMSVcyZij2hihFymcSxWq/I9WL0dsjRif8x0BUQwRXwmXc6+mhlgLBe2Zs # 2dUouzJ8NVZcjfWvsg4noXPrNQ/IiyCVZlSIgaDftDIxVSPk5/rXiUUNex8Tn1+I # TnOgnXhpOQD1vwVGYS8LrcfA0ubSili7xUJ8k2e5TkCjpkaVnYNu # =q+7N # -----END PGP SIGNATURE----- # gpg: Signature made Tue Jun 14 11:57:49 2022 BST # gpg: using RSA key 053191DFF4670330465227F7A542E4ED1B0FAC09 # gpg: issuer "erik@matrix.org" # gpg: WARNING: server 'dirmngr' is older than us (2.2.34 < 2.3.6) # gpg: Note: Outdated servers may lack important security fixes. # gpg: Note: Use the command "gpgconf --kill all" to restart them. # gpg: Can't check signature: No public key # Conflicts: # docs/workers.md # synapse/handlers/message.py # synapse/handlers/pagination.py # synapse/push/bulk_push_rule_evaluator.py # synapse/push/push_rule_evaluator.py # synapse/rest/client/receipts.py # synapse/storage/databases/main/appservice.py # tests/push/test_push_rule_evaluator.py

erikjohnston changed the base branch from develop to erikj/rename_state_store May 25, 2022 11:05

Base automatically changed from erikj/rename_state_store to develop May 25, 2022 11:59

erikjohnston added 2 commits May 25, 2022 12:59

Rename StateGroupStorage

78faa1a

Add fetching current state funcs to StateStorage

334844d

erikjohnston force-pushed the erikj/await_current_state branch from 007d01a to 98ec3bc Compare May 25, 2022 11:59

erikjohnston added 9 commits May 25, 2022 14:03

Use StateStorage.get_current_state_ids

fe86915

Use StateStorage.get_filtered_current_state_ids

a2465b8

Use StateStorage.get_current_state_deltas

a2945a5

Rename to get_partial_current_state_ids

e786f68

Rename to get_partial_filtered_current_state_ids

3f74b37

Rename to get_partial_current_state_deltas

c068581

Block calls to get_current_state_ids and co. when have partial state

6f386d1

Add test for PartialCurrentStateTracker

2b674f8

Newsfile

ecae768

erikjohnston force-pushed the erikj/await_current_state branch from d371c82 to ecae768 Compare May 25, 2022 13:05

Don't block sync when lazy joining a room

d1a1d6a

erikjohnston marked this pull request as ready for review May 25, 2022 14:39

erikjohnston requested a review from a team as a code owner May 25, 2022 14:39

erikjohnston commented May 25, 2022

View reviewed changes

Fix type annotation

531955f

erikjohnston added this to the Q2 2022 ─ Faster joins phase 2: correctness milestone May 26, 2022

richvdh self-assigned this May 26, 2022

richvdh previously requested changes May 26, 2022

View reviewed changes

erikjohnston and others added 5 commits May 26, 2022 17:45

Apply suggestions from code review

6cc7269

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

s/self.storage/self._storage/

3a20548

s/is_room_got_partial_state/has_room_only_got_partial_state/

6e7625e

Merge get_filtered and get_current_state_ids

711ea44

Merge remote-tracking branch 'origin/develop' into erikj/await_curren…

1f99ce0

…t_state

erikjohnston requested a review from richvdh May 27, 2022 09:32

richvdh removed their request for review May 30, 2022 12:08

erikjohnston added 2 commits May 31, 2022 15:49

Merge remote-tracking branch 'origin/develop' into erikj/await_curren…

b59418f

…t_state

Fix up naming

c6fe132

erikjohnston requested a review from a team May 31, 2022 15:20

richvdh reviewed Jun 1, 2022

View reviewed changes

erikjohnston and others added 3 commits June 1, 2022 14:59

Apply suggestions from code review

4757b00

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

Lint

b6cb65f

Rename has_room_only_got_partial_state

c513c20

erikjohnston requested a review from richvdh June 1, 2022 14:07

richvdh changed the title ~~Await for lazy join to complete when getting current state~~ Wait for lazy join to complete when getting current state Jun 1, 2022

richvdh approved these changes Jun 1, 2022

View reviewed changes

erikjohnston merged commit 888a29f into develop Jun 1, 2022

erikjohnston deleted the erikj/await_current_state branch June 1, 2022 15:02

squahtx mentioned this pull request Jun 1, 2022

Test cancellation at every await during request handling #12674

Merged

erikjohnston mentioned this pull request Jun 6, 2022

Faster joins: audit code paths that do state queries #12643

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wait for lazy join to complete when getting current state #12872

Wait for lazy join to complete when getting current state #12872

erikjohnston commented May 25, 2022 •

edited

Loading

erikjohnston May 25, 2022

richvdh May 26, 2022

erikjohnston May 27, 2022

erikjohnston May 25, 2022

richvdh May 26, 2022

richvdh May 26, 2022

richvdh May 26, 2022

richvdh commented May 30, 2022

richvdh left a comment

richvdh Jun 1, 2022

erikjohnston Jun 1, 2022

richvdh Jun 1, 2022

richvdh Jun 1, 2022

richvdh Jun 1, 2022

erikjohnston commented Jun 1, 2022

	# FIXME(faster room joins): what do we do here?
	# FIXME(faster_joins): what do we do here?

Wait for lazy join to complete when getting current state #12872

Wait for lazy join to complete when getting current state #12872

Conversation

erikjohnston commented May 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh commented May 30, 2022

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikjohnston commented Jun 1, 2022

erikjohnston commented May 25, 2022 •

edited

Loading