Skip to content

Commit 16108c5

Browse files
authored
Fix SQL delta file taking a long time to run (matrix-org#9516)
Fixes matrix-org#9504
1 parent f00c4e7 commit 16108c5

File tree

4 files changed

+60
-5
lines changed

4 files changed

+60
-5
lines changed

changelog.d/9516.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a bug where users' pushers were not all deleted when they deactivated their account.

scripts/synapse_port_db

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ from synapse.storage.databases.main.events_bg_updates import (
4747
from synapse.storage.databases.main.media_repository import (
4848
MediaRepositoryBackgroundUpdateStore,
4949
)
50+
from synapse.storage.databases.main.pusher import PusherWorkerStore
5051
from synapse.storage.databases.main.registration import (
5152
RegistrationBackgroundUpdateStore,
5253
find_max_generated_user_id_localpart,
@@ -177,6 +178,7 @@ class Store(
177178
UserDirectoryBackgroundUpdateStore,
178179
EndToEndKeyBackgroundStore,
179180
StatsStore,
181+
PusherWorkerStore,
180182
):
181183
def execute(self, f, *args, **kwargs):
182184
return self.db_pool.runInteraction(f.__name__, f, *args, **kwargs)

synapse/storage/databases/main/pusher.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ def __init__(self, database: DatabasePool, db_conn: Connection, hs: "HomeServer"
3939
db_conn, "pushers", "id", extra_tables=[("deleted_pushers", "stream_id")]
4040
)
4141

42+
self.db_pool.updates.register_background_update_handler(
43+
"remove_deactivated_pushers",
44+
self._remove_deactivated_pushers,
45+
)
46+
4247
def _decode_pushers_rows(self, rows: Iterable[dict]) -> Iterator[PusherConfig]:
4348
"""JSON-decode the data in the rows returned from the `pushers` table
4449
@@ -284,6 +289,54 @@ async def set_throttle_params(
284289
lock=False,
285290
)
286291

292+
async def _remove_deactivated_pushers(self, progress: dict, batch_size: int) -> int:
293+
"""A background update that deletes all pushers for deactivated users.
294+
295+
Note that we don't proacively tell the pusherpool that we've deleted
296+
these (just because its a bit off a faff to do from here), but they will
297+
get cleaned up at the next restart
298+
"""
299+
300+
last_user = progress.get("last_user", "")
301+
302+
def _delete_pushers(txn) -> int:
303+
304+
sql = """
305+
SELECT name FROM users
306+
WHERE deactivated = ? and name > ?
307+
ORDER BY name ASC
308+
LIMIT ?
309+
"""
310+
311+
txn.execute(sql, (1, last_user, batch_size))
312+
users = [row[0] for row in txn]
313+
314+
self.db_pool.simple_delete_many_txn(
315+
txn,
316+
table="pushers",
317+
column="user_name",
318+
iterable=users,
319+
keyvalues={},
320+
)
321+
322+
if users:
323+
self.db_pool.updates._background_update_progress_txn(
324+
txn, "remove_deactivated_pushers", {"last_user": users[-1]}
325+
)
326+
327+
return len(users)
328+
329+
number_deleted = await self.db_pool.runInteraction(
330+
"_remove_deactivated_pushers", _delete_pushers
331+
)
332+
333+
if number_deleted < batch_size:
334+
await self.db_pool.updates._end_background_update(
335+
"remove_deactivated_pushers"
336+
)
337+
338+
return number_deleted
339+
287340

288341
class PusherStore(PusherWorkerStore):
289342
def get_pushers_stream_token(self) -> int:

synapse/storage/databases/main/schema/delta/59/08delete_pushers_for_deactivated_accounts.sql

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@
1414
*/
1515

1616

17-
-- We may not have deleted all pushers for deactivated accounts. Do so now.
18-
--
19-
-- Note: We don't bother updating the `deleted_pushers` table as it's just use
20-
-- to stop pushers on workers, and that will happen when they get next restarted.
21-
DELETE FROM pushers WHERE user_name IN (SELECT name FROM users WHERE deactivated = 1);
17+
-- We may not have deleted all pushers for deactivated accounts, so we set up a
18+
-- background job to delete them.
19+
INSERT INTO background_updates (ordering, update_name, progress_json) VALUES
20+
(5908, 'remove_deactivated_pushers', '{}');

0 commit comments

Comments
 (0)