Skip to content

Add experimental support for MSC4308: Thread Subscriptions extension to Sliding Sync when MSC4306 and MSC4186 are enabled. #18695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Sep 11, 2025
Merged

Add experimental support for MSC4308: Thread Subscriptions extension to Sliding Sync when MSC4306 and MSC4186 are enabled. #18695

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2961006
Update simplified sliding sync docstring
reivilibre Jun 17, 2025
875dbf7
spelling
reivilibre Jul 10, 2025
09f8633
Add models for Thread Subscriptions extension to Sliding Sync
reivilibre Jul 18, 2025
f1f5657
Add overload for `gather_optional_coroutines`/6
reivilibre Jul 18, 2025
748316c
Add thread subscriptions position to `StreamToken`
reivilibre Jul 18, 2025
4dcd12b
Add `subscribed` and `automatic` to `get_updated_thread_subscriptions…
reivilibre Jul 18, 2025
0ce5dce
Fix thread_subscriptions stream sequence
reivilibre Jul 21, 2025
0c310b9
Add comment to MultiWriterIdGenerator about cursed sequence semantics
reivilibre Aug 20, 2025
18881b1
Add overload for `parse_integer_from_args`
reivilibre Aug 20, 2025
4a34641
Implement sliding sync extension part of MSC4308
reivilibre Aug 20, 2025
e72d6cd
Add companion endpoint for backpagination of thread subscriptions
reivilibre Aug 20, 2025
f4cd180
Newsfile
reivilibre Aug 20, 2025
921cd53
Update tests/rest/client/sliding_sync/test_extension_thread_subscript…
reivilibre Sep 2, 2025
168b67b
Update synapse/handlers/sliding_sync/extensions.py
reivilibre Sep 2, 2025
1374895
Update synapse/handlers/sliding_sync/extensions.py
reivilibre Sep 2, 2025
0cf178a
Add notifier hooks for sliding sync
reivilibre Sep 2, 2025
924c1bf
Use copy_and_replace in get_current_token_for_pagination
reivilibre Sep 3, 2025
fa8e3b6
Simplify if
reivilibre Sep 3, 2025
80679a7
Comment on why we still check limit
reivilibre Sep 9, 2025
00cb14e
Merge branch 'develop' into rei/ssext_threadsubs
reivilibre Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions synapse/storage/util/id_generators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,12 @@ class MultiWriterIdGenerator(AbstractStreamIdGenerator):
Warning: Streams using this generator start at ID 2, because ID 1 is always assumed
to have been 'seen as persisted'.
Unclear if this extant behaviour is desirable for some reason.
When creating a new sequence for a new stream,
it will be necessary to use `START WITH 2`.
When creating a new sequence for a new stream, it will be necessary to advance it
so that position 1 is consumed.
DO NOT USE `START WITH 2` FOR THIS PURPOSE:
see https://github.com/element-hq/synapse/issues/18712
Instead, use `SELECT nextval('sequence_name');` immediately after the
`CREATE SEQUENCE` statement.
Comment on lines +190 to +195
Copy link
Member

Choose a reason for hiding this comment

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

Should we close #18712 once this PR is merged?
Unless we plan to fix it in some other fashion down the line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrmrmrmr. It feels like a design footgun that we could ideally resolve rather than having a hidden comment somewhere about what to do. But not sure if that's worth keeping an issue in the infinite backlog.

Copy link
Member

Choose a reason for hiding this comment

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

IMO there's not too much benefit gained from properly fixing it vs. other things we could work on. So in a purely triage sense, I'd be in favour of closing the issue and leaving the comment as a helpful touch stone (one that may prompt folks to fix it in future if they really wanted to).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somewhat agreed, but the comment is not necessarily in a place you will notice (I'm not sure where you could do that, actually). The risk is that next time we come to adding a new one, we lose another couple of hours trying to track this issue down. But mrm I suppose this is probably the best we can do atm


Args:
db_conn
Expand Down