-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Update the thread_id right before use (in case the bg update hasn't finished) #14222
Changes from all commits
b43f9c5
ba13b83
0436ed3
0ed08e1
8040f28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Support for thread-specific notifications & receipts ([MSC3771](https://github.com/matrix-org/matrix-spec-proposals/pull/3771) and [MSC3773](https://github.com/matrix-org/matrix-spec-proposals/pull/3773)). |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,11 @@ | |
* limitations under the License. | ||
*/ | ||
|
||
-- The columns can now be made non-nullable. | ||
ALTER TABLE event_push_actions_staging ALTER COLUMN thread_id SET NOT NULL; | ||
ALTER TABLE event_push_actions ALTER COLUMN thread_id SET NOT NULL; | ||
ALTER TABLE event_push_summary ALTER COLUMN thread_id SET NOT NULL; | ||
-- Allow there to be multiple summaries per user/room. | ||
DROP INDEX IF EXISTS event_push_summary_unique_index; | ||
Comment on lines
+16
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dropping this index is somewhat scary, I don't know how well this will handle rolling back. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, we can only rollback to v1.68, which I thought we checked was fine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it will work OK with that, yes. |
||
|
||
INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES | ||
(7306, 'event_push_actions_thread_id_null', '{}', 'event_push_backfill_thread_id'); | ||
|
||
INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES | ||
(7306, 'event_push_summary_thread_id_null', '{}', 'event_push_backfill_thread_id'); | ||
Comment on lines
+19
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I chose not to care about (And the table shouldn't be large so I don't think it can really have non-nulls in it? Maybe I should just do the alter table for that one now?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, we also have a cleanup task for it to ensure that stale entries get removed, so it really should be quick. Though at this point it might be worth just doing the UPDATE when we do the others too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
But we don't need to since we don't join to the table; we could maybe do something when we pull rows from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My only mild concern is what happens when we do a rolling restart or something, but I think that's fine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
(Note that Synapse 1.69 fills in that column in the staging table so shouldn't be an issue) I don't think it will be a problem regardless:
There might be a slight race in here with whether bg update has ran though. 😟 |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe overkill, but it seems good to not constantly do those updates if we don't need to?