Skip to content

MSC4186: Simplified Sliding Sync#4186

Open
erikjohnston wants to merge 110 commits intomainfrom
erikj/sss
Open

MSC4186: Simplified Sliding Sync#4186
erikjohnston wants to merge 110 commits intomainfrom
erikj/sss

Conversation

@erikjohnston
Copy link
Copy Markdown
Member

@erikjohnston erikjohnston commented Aug 30, 2024

@erikjohnston erikjohnston marked this pull request as ready for review August 30, 2024 09:59
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
@anoadragon453 anoadragon453 added feature Suggestion for a significant extension which needs considerable consideration proposal A matrix spec change proposal kind:core MSC which is critical to the protocol's success labels Aug 30, 2024
@uhoreg uhoreg added the client-server Client-Server API label Aug 30, 2024
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Aug 30, 2024
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Implementation requirements:

  • Client (ideally multiple)
  • Server

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Current implementations to my knowledge:

  • Client implementation: matrix-rust-sdk (PR)
  • Server implementation: has been implemented in Synapse across 10s of PRs. (Perhaps @MadLittleMods, the implementer, can give better link(s) here.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

conduwuit has implemented simplified sliding sync in x86pup/conduwuit#666

Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
// The "heroes" for the room, if there is no room name. Only sent initially and when it changes.
"heroes": [
{"user_id":"@alice:example.com","displayname":"Alice","avatar_url":"mxc://..."},
],
Copy link
Copy Markdown
Contributor

@MadLittleMods MadLittleMods Sep 5, 2024

Choose a reason for hiding this comment

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

Do the heroes membership state events need to be included in required_state response when requesting required_state: ["m.room.member", "$LAZY"] (lazy-loading room members)?

Sync v2 says this:

When lazy-loading room members is enabled, the membership events for the heroes MUST be included in the state, unless they are redundant. When the list of users changes, the server notifies the client by sending a fresh list of heroes. If there are no changes since the last sync, this field may be omitted.

But I think that's because m.heroes in Sync v2 is only a list of user ID's. Whereas, in the sliding sync response here, we already have all of the info necessary in these stripped events.

One alternative is to match Sync v2 and only list the user ID's in heroes and include the membership events in required_state.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This thread hasn't been resolved.

My impression is that the answer to the question ("Do the heroes membership state events need to be included in required_state response") is "no", but I'm not certain of that.

In fact... it seems to be one of the "open questions" at the end of the proposal?

Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 14, 2024
# Synapse 1.114.0 (2024-09-02)

This release enables support for
[MSC4186](matrix-org/matrix-spec-proposals#4186) —
Simplified Sliding Sync. This allows using the upcoming releases of the Element
X mobile apps without having to run a Sliding Sync Proxy.


### Features

- Enable native sliding sync support ([MSC3575](matrix-org/matrix-spec-proposals#3575) and [MSC4186](matrix-org/matrix-spec-proposals#4186)) by default. ([\#17648](element-hq/synapse#17648))




# Synapse 1.114.0rc3 (2024-08-30)

### Bugfixes

- Fix regression in v1.114.0rc2 that caused workers to fail to start. ([\#17626](element-hq/synapse#17626))




# Synapse 1.114.0rc2 (2024-08-30)

### Features

- Improve cross-signing upload when using [MSC3861](matrix-org/matrix-spec-proposals#3861) to use a custom UIA flow stage, with web fallback support. ([\#17509](element-hq/synapse#17509))
- Make `hash_password` script accept password input from stdin. ([\#17608](element-hq/synapse#17608))

### Bugfixes

- Fix hierarchy returning 403 when room is accessible through federation. Contributed by Krishan (@kfiven). ([\#17194](element-hq/synapse#17194))
- Fix content-length on federation `/thumbnail` responses. ([\#17532](element-hq/synapse#17532))
- Fix authenticated media responses using a wrong limit when following redirects over federation. ([\#17543](element-hq/synapse#17543))

### Internal Changes

- MSC3861: load the issuer and account management URLs from OIDC discovery. ([\#17407](element-hq/synapse#17407))
- Refactor sliding sync class into multiple files. ([\#17595](element-hq/synapse#17595))
- Store sliding sync per-connection state in the database. ([\#17599](element-hq/synapse#17599))
- Make the sliding sync `PerConnectionState` class immutable. ([\#17600](element-hq/synapse#17600))
- Add support to `@tag_args` for standalone functions. ([\#17604](element-hq/synapse#17604))
- Speed up incremental syncs in sliding sync by adding some more caching. ([\#17606](element-hq/synapse#17606))
- Always return the user's own read receipts in sliding sync. ([\#17617](element-hq/synapse#17617))
- Replace `isort` and `black` with `ruff`. ([\#17620](element-hq/synapse#17620))
- Refactor sliding sync code to move room list logic out into a separate class. ([\#17622](element-hq/synapse#17622))



### Updates to locked dependencies

* Bump attrs from 23.2.0 to 24.2.0. ([\#17609](element-hq/synapse#17609))
* Bump cryptography from 42.0.8 to 43.0.0. ([\#17584](element-hq/synapse#17584))
* Bump phonenumbers from 8.13.43 to 8.13.44. ([\#17610](element-hq/synapse#17610))
* Bump pygithub from 2.3.0 to 2.4.0. ([\#17612](element-hq/synapse#17612))
* Bump pyyaml from 6.0.1 to 6.0.2. ([\#17611](element-hq/synapse#17611))
* Bump sentry-sdk from 2.12.0 to 2.13.0. ([\#17585](element-hq/synapse#17585))
* Bump serde from 1.0.206 to 1.0.208. ([\#17581](element-hq/synapse#17581))
* Bump serde from 1.0.208 to 1.0.209. ([\#17613](element-hq/synapse#17613))
* Bump serde_json from 1.0.124 to 1.0.125. ([\#17582](element-hq/synapse#17582))
* Bump serde_json from 1.0.125 to 1.0.127. ([\#17614](element-hq/synapse#17614))
* Bump types-jsonschema from 4.23.0.20240712 to 4.23.0.20240813. ([\#17583](element-hq/synapse#17583))
* Bump types-setuptools from 71.1.0.20240726 to 71.1.0.20240818. ([\#17586](element-hq/synapse#17586))

# Synapse 1.114.0rc1 (2024-08-20)

### Features

- Add a flag to `/versions`, `org.matrix.simplified_msc3575`, to indicate whether experimental sliding sync support has been enabled. ([\#17571](element-hq/synapse#17571))
- Handle changes in `timeline_limit` in experimental sliding sync. ([\#17579](element-hq/synapse#17579))
- Correctly track read receipts that should be sent down in experimental sliding sync. ([\#17575](element-hq/synapse#17575), [\#17589](element-hq/synapse#17589), [\#17592](element-hq/synapse#17592))

### Bugfixes

- Start handlers for new media endpoints when media resource configured. ([\#17483](element-hq/synapse#17483))
- Fix timeline ordering (using `stream_ordering` instead of topological ordering) in experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint. ([\#17510](element-hq/synapse#17510))
- Fix experimental sliding sync implementation to remember any updates in rooms that were not sent down immediately. ([\#17535](element-hq/synapse#17535))
- Better exclude partially stated rooms if we must await full state in experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint. ([\#17538](element-hq/synapse#17538))
- Handle lower-case http headers in `_Mulitpart_Parser_Protocol`. ([\#17545](element-hq/synapse#17545))
- Fix fetching federation signing keys from servers that omit `old_verify_keys`. Contributed by @tulir @ Beeper. ([\#17568](element-hq/synapse#17568))
- Fix bug where we would respond with an error when a remote server asked for media that had a length of 0, using the new multipart federation media endpoint. ([\#17570](element-hq/synapse#17570))

### Improved Documentation

- Clarify default behaviour of the
  [`auto_accept_invites.worker_to_run_on`](https://element-hq.github.io/synapse/develop/usage/configuration/config_documentation.html#auto-accept-invites)
  option. ([\#17515](element-hq/synapse#17515))
- Improve docstrings for profile methods. ([\#17559](element-hq/synapse#17559))

### Internal Changes

- Add more tracing to experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint. ([\#17514](element-hq/synapse#17514))
- Fixup comment in sliding sync implementation. ([\#17531](element-hq/synapse#17531))
- Replace override of deprecated method `HTTPAdapter.get_connection` with `get_connection_with_tls_context`. ([\#17536](element-hq/synapse#17536))
- Fix performance of device lists in `/key/changes` and sliding sync. ([\#17537](element-hq/synapse#17537), [\#17548](element-hq/synapse#17548))
- Bump setuptools from 67.6.0 to 72.1.0. ([\#17542](element-hq/synapse#17542))
- Add a utility function for generating random event IDs. ([\#17557](element-hq/synapse#17557))
- Speed up responding to media requests. ([\#17558](element-hq/synapse#17558), [\#17561](element-hq/synapse#17561), [\#17564](element-hq/synapse#17564), [\#17566](element-hq/synapse#17566), [\#17567](element-hq/synapse#17567), [\#17569](element-hq/synapse#17569))
- Test github token before running release script steps. ([\#17562](element-hq/synapse#17562))
- Reduce log spam of multipart files. ([\#17563](element-hq/synapse#17563))
- Refactor per-connection state in experimental sliding sync handler. ([\#17574](element-hq/synapse#17574))
- Add histogram metrics for sliding sync processing time. ([\#17593](element-hq/synapse#17593))



### Updates to locked dependencies

* Bump bytes from 1.6.1 to 1.7.1. ([\#17526](element-hq/synapse#17526))
* Bump lxml from 5.2.2 to 5.3.0. ([\#17550](element-hq/synapse#17550))
* Bump phonenumbers from 8.13.42 to 8.13.43. ([\#17551](element-hq/synapse#17551))
* Bump regex from 1.10.5 to 1.10.6. ([\#17527](element-hq/synapse#17527))
* Bump sentry-sdk from 2.10.0 to 2.12.0. ([\#17553](element-hq/synapse#17553))
* Bump serde from 1.0.204 to 1.0.206. ([\#17556](element-hq/synapse#17556))
* Bump serde_json from 1.0.122 to 1.0.124. ([\#17555](element-hq/synapse#17555))
* Bump sigstore/cosign-installer from 3.5.0 to 3.6.0. ([\#17549](element-hq/synapse#17549))
* Bump types-pyyaml from 6.0.12.20240311 to 6.0.12.20240808. ([\#17552](element-hq/synapse#17552))
* Bump types-requests from 2.31.0.20240406 to 2.32.0.20240712. ([\#17524](element-hq/synapse#17524))

# Synapse 1.113.0 (2024-08-13)

No significant changes since 1.113.0rc1.




# Synapse 1.113.0rc1 (2024-08-06)

### Features

- Track which rooms have been sent to clients in the experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint. ([\#17447](element-hq/synapse#17447))
- Add Account Data extension support to experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint. ([\#17477](element-hq/synapse#17477))
- Add receipts extension support to experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint. ([\#17489](element-hq/synapse#17489))
- Add typing notification extension support to experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint. ([\#17505](element-hq/synapse#17505))

### Bugfixes

- Update experimental [MSC3575](matrix-org/matrix-spec-proposals#3575) Sliding Sync `/sync` endpoint to handle invite/knock rooms when filtering. ([\#17450](element-hq/synapse#17450))
- Fix a bug introduced in v1.110.0 which caused `/keys/query` to return incomplete results, leading to high network activity and CPU usage on Matrix clients. ([\#17499](element-hq/synapse#17499))

### Improved Documentation

- Update the [`allowed_local_3pids`](https://element-hq.github.io/synapse/v1.112/usage/configuration/config_documentation.html#allowed_local_3pids) config option's msisdn address to a working example. ([\#17476](element-hq/synapse#17476))

### Internal Changes

- Change sliding sync to use their own token format in preparation for storing per-connection state. ([\#17452](element-hq/synapse#17452))
- Ensure we don't send down negative `bump_stamp` in experimental sliding sync endpoint. ([\#17478](element-hq/synapse#17478))
- Do not send down empty room entries down experimental sliding sync endpoint. ([\#17479](element-hq/synapse#17479))
- Refactor Sliding Sync tests to better utilize the `SlidingSyncBase`. ([\#17481](element-hq/synapse#17481), [\#17482](element-hq/synapse#17482))
- Add some opentracing tags and logging to the experimental sliding sync implementation. ([\#17501](element-hq/synapse#17501))
- Split and move Sliding Sync tests so we have some more sane test file sizes. ([\#17504](element-hq/synapse#17504))
- Update the `limited` field description in the Sliding Sync response to accurately describe what it actually represents. ([\#17507](element-hq/synapse#17507))
- Easier to understand `timeline` assertions in Sliding Sync tests. ([\#17511](element-hq/synapse#17511))
- Reset the sliding sync connection if we don't recognize the per-connection state position. ([\#17529](element-hq/synapse#17529))



### Updates to locked dependencies

* Bump bcrypt from 4.1.3 to 4.2.0. ([\#17495](element-hq/synapse#17495))
* Bump black from 24.4.2 to 24.8.0. ([\#17522](element-hq/synapse#17522))
* Bump phonenumbers from 8.13.39 to 8.13.42. ([\#17521](element-hq/synapse#17521))
* Bump ruff from 0.5.4 to 0.5.5. ([\#17494](element-hq/synapse#17494))
* Bump serde_json from 1.0.120 to 1.0.121. ([\#17493](element-hq/synapse#17493))
* Bump serde_json from 1.0.121 to 1.0.122. ([\#17525](element-hq/synapse#17525))
* Bump towncrier from 23.11.0 to 24.7.1. ([\#17523](element-hq/synapse#17523))
* Bump types-pyopenssl from 24.1.0.20240425 to 24.1.0.20240722. ([\#17496](element-hq/synapse#17496))
* Bump types-setuptools from 70.1.0.20240627 to 71.1.0.20240726. ([\#17497](element-hq/synapse#17497))
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated

| Name | Type | Required | Comment |
| - | - | - | - |
| `name` | `[string \| null]` | No | Room name. `null` if room name has been removed c.f. https://spec.matrix.org/v1.17/client-server-api/#mroomname |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should be more explicit about where this information comes from m.room.name (same with avatar)

Previous discussion, #4186 (comment)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I mean, for room name we literally link to the spec? I don't think its anything but noise to say how room names and avatars work, they're not SSS specific and are general matrix concept?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was more thinking that the "c.f. https://spec.matrix.org/v1.17/client-server-api/#mroomname" was referring to the removed behavior as that's how this description evolved and is part of that sentence.

Could be considered good enough but a bit askew ⏩

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@MadLittleMods could you either make a concrete suggestion, or confirm that you are happy for this thread to be resolved?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"good enough" to resolve but there is room for improvement based on author appetite.

(I can't resolve/unresolve any threads)

joined, been invited, knocked on, left, or been kicked or banned from, sorted by recent activity (see below for exact
ordering semantics).

Note that for rooms the user has been banned from, but never joined, should not be part of the list.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lifting this out of a resolved discussion:

How will forgotten rooms be dealt with here? The old /sync doesn't seem to have a way to synchronize forgotten rooms across clients. It'd be nice if SSS could address this.

-- @Johennes, #4186 (comment)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That could be an extension I think. It's fairly uncommon for clients to want to know about forgotten rooms.

Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment thread proposals/4186-simplified-sliding-sync.md Outdated

A server may decide to "expire" connections, either to free resources or because the server thinks it would be faster
for the client to start from scratch (e.g. because there are many updates to send down). This is done by responding with
a 400 HTTP status and an error code of `M_UNKNOWN_POS`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feels like we may need an unstable namespaced version of M_UNKNOWN_POS

It's only being returned by unstable endpoints but also feels wrong to use an identifier that could mean something else in the future.

Copy link
Copy Markdown
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

I've been through the open threads that are more than a week old, and noted what appears to be outstanding.

I still want to take another look at the changes since my previous review (in December), but I need to take a break.

// The "heroes" for the room, if there is no room name. Only sent initially and when it changes.
"heroes": [
{"user_id":"@alice:example.com","displayname":"Alice","avatar_url":"mxc://..."},
],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This thread hasn't been resolved.

My impression is that the answer to the question ("Do the heroes membership state events need to be included in required_state response") is "no", but I'm not certain of that.

In fact... it seems to be one of the "open questions" at the end of the proposal?

0 to ensure the server doesn't block waiting for new data. This can easily happen if the app starts and sends the first
request with a `since` parameter, if the client shows a spinner but doesn't set a timeout then the request may take a
long time to return (if there were no updates to return).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This thread doesn't appear to have been addressed.

Comment thread proposals/4186-simplified-sliding-sync.md Outdated
Comment on lines +139 to +140
Extensions also allow this proposal to be broken up into more manageable sections. Extensions are requested by the
client in a dedicated extensions block.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is a matter that should be handled in the MSCs for the specific extensions. I think that, if we have a need to tell clients whether a given extension is available, we'll be able come up with some way to do it. AFAIK we don't have that need yet.

@reivilibre are you happy for us to resolve this thread?

Comment thread proposals/4186-simplified-sliding-sync.md Outdated
| `timeline_events` | `[Event]` | No | The latest events in the room. May not include all events if e.g. there were more events than the configured `timeline_limit`, c.f. the `limited` field. <br/><br/> If `limited` is true then we include bundle aggregations for the event, as per `/v3/sync`. <br/><br/> The last event in the list is the most recent. |
| `prev_batch` | `string` | No | A token that can be passed as a start parameter to the `/rooms/<room_id>/messages` API to retrieve earlier messages. |
| `limited` | `bool` | No | True if there are more events since the previous sync than were included in the `timeline_events` field, or that the client should paginate to fetch more events.<br/><br/> Note that server may return fewer than the requested number of events and still set `limited` to true, e.g. because there is a gap in the history the server has for the room. <br/><br/>Absence means `false` |
| `num_live` | `int` | No | The number of timeline events which have "just occurred" and are not historical, i.e. that have happened since the previous sync request. The last `N` events are 'live' and should be treated as such.<br/><br/> This is mostly useful to e.g. determine whether a given `@mention` event should make a noise or not. Clients cannot rely solely on the absence of `initial: true` to determine live events because if a room not in the sliding window bumps into the window because of an `@mention` it will have `initial: true` yet contain a single live event (with potentially other old events in the timeline). |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok. I wonder if it might be worth noting this in the text, but I don't feel strongly.

Comment thread proposals/4186-simplified-sliding-sync.md
> Knock support hasn't been implemented in Synapse.


### `StrippedHero` type
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd be in favour of renaming the type, otherwise it will get carried into the spec and confuse us forever, but 🤷

Comment thread proposals/4186-simplified-sliding-sync.md Outdated
for the client to start from scratch (e.g. because there are many updates to send down). This is done by responding with
a 400 HTTP status and an error code of `M_UNKNOWN_POS`.

A `pos` token can be used in `/messages` and `/relations` APIs.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@erikjohnston can we clarify this?

Copy link
Copy Markdown
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

A few minor things I'd like us to clarify.

- `timeline_limit` — take the maximum timeline limit across all room configs.
- `required_state` — take the union of the required state fields, i.e. if a state event would be returned by any room
config it is returned by the combined room config. The combined config has `lazy_members` set to true if and only if
any of the configs have `lazy_members` set to true.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't this be "all of the configs have lazy_members set to true", given we are trying to derive the superset, and setting lazy_members means we return less data?

Comment thread proposals/4186-simplified-sliding-sync.md Outdated

| Name | Type | Required | Comment |
| - | - | - | - |
| `name` | `[string \| null]` | No | Room name. `null` if room name has been removed c.f. https://spec.matrix.org/v1.17/client-server-api/#mroomname |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@MadLittleMods could you either make a concrete suggestion, or confirm that you are happy for this thread to be resolved?


When combining room configs with different `required_state` fields the result must be the superset of them all. There
are two approaches server-side for handling this: a) keep the `required_state` separate and return any state that
matches any of them, or b) merge the fields together, however care must be taken to correctly account for configs that
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
matches any of them, or b) merge the fields together, however care must be taken to correctly account for configs that
matches any of them, or b) merge the fields together, taking care to correctly account for configs that

| `timeline_events` | `[Event]` | No | The latest events in the room. May not include all events if e.g. there were more events than the configured `timeline_limit`, c.f. the `limited` field. <br/><br/> If `limited` is true then we include bundle aggregations for the event, as per `/v3/sync`. <br/><br/> The last event in the list is the most recent. |
| `prev_batch` | `string` | No | A token that can be passed as a start parameter to the `/rooms/<room_id>/messages` API to retrieve earlier messages. |
| `limited` | `bool` | No | True if there are more events since the previous sync than were included in the `timeline_events` field, or that the client should paginate to fetch more events.<br/><br/> Note that server may return fewer than the requested number of events and still set `limited` to true, e.g. because there is a gap in the history the server has for the room. <br/><br/>Absence means `false` |
| `num_live` | `int` | No | The number of timeline events which have "just occurred" and are not historical, i.e. that have happened since the previous sync request. The last `N` events are 'live' and should be treated as such.<br/><br/> This is mostly useful to e.g. determine whether a given `@mention` event should make a noise or not. Clients cannot rely solely on the absence of `initial: true` to determine live events because if a room not in the sliding window bumps into the window because of an `@mention` it will have `initial: true` yet contain a single live event (with potentially other old events in the timeline). |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[not a blocking comment, feel free to resolve]

> Knock support hasn't been implemented in Synapse.


### `StrippedHero` type
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[not a blocking comment, feel free to resolve]

Comment on lines +657 to +667
1. Duplication between room response `heroes` and `required_state` when specifying `lazy_members`, e.g. should we drop
`heroes` if we are returning membership events?
1. <del>Should the room response include the user's current membership? The client can always request it via
`required_state`, but that may be wasted if the client doesn't need any other information from the member event.</del>
1. Should we remove the `highlight_count` and `notification_count` fields, given clients increasingly must calculate
this themselves, and Synapse currently doesn't implement it nor does Element X use it.
1. Should we support subscribing to rooms the user is not a member of, i.e. peeking for world readable rooms.
1. Should we support timeline filtering?
1. <del>Should we move `pos`, and other URL params, into the request body? This would allow web clients to cache the CORS
requests, rather than having to pre-flight each request.</del>
1. How do we make it so that the clients don't have to send up the same body each time?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I really feel like we ought to clarify the state of these "open questions" before we merge the MSC. I don't think that's very hard: maybe give each of the un-<del>-eted entries a quick bullet point saying something like "not supporting this for now, maybe in a future MSC".


## Changelog

Changes from the initial implementation of simplified sliding sync.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I kinda misunderstood what this actually meant, so:

Suggested change
Changes from the initial implementation of simplified sliding sync.
Differences from the experimental implementation of simplified sliding sync in Synapse v1.151.0.

(Or pick some alternative release or wording).

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

Labels

client-server Client-Server API disposition-merge feature Suggestion for a significant extension which needs considerable consideration kind:core MSC which is critical to the protocol's success matrix-2.0 Required for Matrix 2.0 proposal A matrix spec change proposal proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. unresolved-concerns This proposal has at least one outstanding concern

Projects

Status: Ready for FCP ticks

Development

Successfully merging this pull request may close these issues.

Room List Summary API