Track notification counts per thread (implement MSC3773) (redo) #13776

clokep · 2022-09-12T13:11:24Z

Track notification counts on a per-thread basis, implementing MSC3773.

The overall design of this is to add a thread_id column to the event_push_actions (+ event_push_actions_staging) and event_push_summary tables. This allows the results to be segmented by the thread (by storing either the root event ID in the thread_id column or the special value "main").

When retrieving counts of notifications we can then segment based on the thread, this is opt-in for the client by providing a sync flag. In the case the client doesn't want separate threads we simply sum across all threads + the main timeline.

The summarization code also needs to be updated to be per thread, instead of just per room.

Please see MSC3773 for a description of the API changes.

Part of #12550

This is essentially a rebase of #13181 on top of #13753 (see #13181 (comment) for the rationale why it was separated).

We want this to land in Synapse v1.69.0 the earliest since it forces an updated schema which is landing in v1.68.0.

synapse/storage/databases/main/event_push_actions.py

…e non-null.

…ounts.

…if-2

clokep · 2022-09-16T16:39:03Z

@erikjohnston I set you as the reviewer since most of this PR is the same as #13181, which you had previously reviewed. You can push it back to the group if you'd like.

clokep · 2022-09-19T12:27:29Z

synapse/storage/schema/__init__.py

+    # The threads_id column must exist for event_push_actions, event_push_summary,
+    # receipts_linearized, and receipts_graph.
+    73


We might be able to defer actually bumping the minimum schema a bit, but results would be inconsistent if you rolled back (you'd end up with nulls in some places which would make the joins not work). Not sure if that's more desirable though.

We discussed this a bit and think it is OK.

Note that schema 73 was introduced in v1.68.0rc1.

…if-2

…are being updated.

clokep · 2022-09-22T15:13:14Z

@erikjohnston I think this is ready for another review!

erikjohnston

Woop

clokep · 2022-09-22T16:40:23Z

Annoyingly I think we want to wait another version before merging? Will ask folks.

…if-2

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))

clokep force-pushed the clokep/threads-notif-2 branch from ab4004d to d3918d8 Compare September 12, 2022 13:12

This was referenced Sep 12, 2022

Track notification counts per thread (implement MSC3773) #13181

Closed

Store thread IDs on receipts (implement MSC3771) (redo) #13782

Merged

clokep force-pushed the clokep/threads-notif-1 branch from 16b1f7f to fc8e0fa Compare September 12, 2022 17:37

clokep force-pushed the clokep/threads-notif-2 branch from d3918d8 to a5a11c7 Compare September 12, 2022 17:38

clokep commented Sep 13, 2022

View reviewed changes

synapse/storage/databases/main/event_push_actions.py Outdated Show resolved Hide resolved

clokep commented Sep 13, 2022

View reviewed changes

synapse/storage/databases/main/event_push_actions.py Outdated Show resolved Hide resolved

clokep mentioned this pull request Sep 13, 2022

MSC3773: Notifications for threads matrix-org/matrix-spec-proposals#3773

Merged

clokep force-pushed the clokep/threads-notif-2 branch from fad9dc0 to b00b4c1 Compare September 14, 2022 15:59

Base automatically changed from clokep/threads-notif-1 to develop September 14, 2022 17:11

clokep force-pushed the clokep/threads-notif-2 branch from b00b4c1 to 0a52fbf Compare September 14, 2022 17:17

clokep added 4 commits September 15, 2022 14:48

Update filtering to include the thread notifications flag.

9cb167c

Ensure that the thread_id column is non-null and then require it to b…

8ac2f32

…e non-null.

Add infrastructure to pass notifications per thread.

111fe57

Calculate thread specific notification counts.

62aa85b

clokep force-pushed the clokep/threads-notif-2 branch from 0a52fbf to 62aa85b Compare September 15, 2022 18:48

clokep mentioned this pull request Sep 15, 2022

Recursively fetch the thread for receipts & notifications. #13824

Merged

clokep requested review from a team and removed request for a team September 15, 2022 19:26

clokep added 4 commits September 16, 2022 11:27

Clarify comment.

cb679e2

Simplify handling of summaries with neither notifications or unread c…

ba00c5f

…ounts.

Delete old push summaries.

eb56567

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

e6f97ec

…if-2

clokep marked this pull request as ready for review September 16, 2022 16:38

clokep requested a review from a team as a code owner September 16, 2022 16:38

clokep requested review from erikjohnston and removed request for a team September 16, 2022 16:38

clokep commented Sep 19, 2022

View reviewed changes

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

4f4711a

…if-2

clokep added 2 commits September 22, 2022 09:55

Threads must already be summarized between the stream orderings that …

55d15a3

…are being updated.

Don't delete empty push summaries.

56c21e4

clokep mentioned this pull request Sep 22, 2022

Clear out empty event_push_summary rows #13875

Open

clokep requested a review from erikjohnston September 22, 2022 15:13

erikjohnston approved these changes Sep 22, 2022

View reviewed changes

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

241b40c

…if-2

clokep mentioned this pull request Sep 23, 2022

Mark events as read using threaded read receipts from MSC3771 #13877

Merged

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

a04258f

…if-2

clokep mentioned this pull request Sep 27, 2022

Delete push actions on receipt #13834

Closed

4 tasks

clokep and others added 6 commits September 27, 2022 14:46

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

ddbb644

…if-2

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

79452e9

…if-2

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

b0d9008

…if-2

Merge remote-tracking branch 'origin/develop' into clokep/threads-not…

f279a15

…if-2

Merge branch 'develop' into clokep/threads-notif-2

f416be9

Update for changes in develop.

0b1b432

clokep mentioned this pull request Oct 4, 2022

Fix backwards compatibility with upcoming threads schema changes #14045

Merged

Update background index numbers.

bd6c80c

clokep merged commit b4ec4f5 into develop Oct 4, 2022

clokep deleted the clokep/threads-notif-2 branch October 4, 2022 13:47

clokep mentioned this pull request Oct 11, 2022

Unexplained unique constraint violation on event_push_summary_unique_index #14120

Closed

reivilibre mentioned this pull request Dec 8, 2022

Background process rotate_notifs fails due to unique constraint violation #14641

Closed

This was referenced Mar 29, 2023

Update the thread_id right before use (in case the bg update hasn't finished) #14222

Merged

Make thread_id required in event_push tables #14225

Closed

Set thread_id column to non-null for event_push_{actions,actions_staging,summary} #15350

Merged

clokep mentioned this pull request Apr 14, 2023

Set thread_id column to non-null for event_push_{actions,actions_staging,summary} #15437

Merged

clokep mentioned this pull request May 15, 2023

Set thread_id column to non-null for event_push_{actions,actions_staging,summary} #15597

Merged

matrixbot mentioned this pull request Dec 21, 2023

Clear out empty event_push_summary rows element-hq/synapse#13875

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track notification counts per thread (implement MSC3773) (redo) #13776

Track notification counts per thread (implement MSC3773) (redo) #13776

clokep commented Sep 12, 2022 •

edited

Loading

clokep commented Sep 16, 2022

clokep Sep 19, 2022

clokep Sep 22, 2022

clokep commented Sep 22, 2022

erikjohnston left a comment

clokep commented Sep 22, 2022

Track notification counts per thread (implement MSC3773) (redo) #13776

Track notification counts per thread (implement MSC3773) (redo) #13776

Conversation

clokep commented Sep 12, 2022 • edited Loading

clokep commented Sep 16, 2022

clokep Sep 19, 2022

Choose a reason for hiding this comment

clokep Sep 22, 2022

Choose a reason for hiding this comment

clokep commented Sep 22, 2022

erikjohnston left a comment

Choose a reason for hiding this comment

clokep commented Sep 22, 2022

clokep commented Sep 12, 2022 •

edited

Loading