maxmustermann/synapse
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge pull request #4434 from matrix-org/erikj/fix_user_ips_dedup

Browse source 
Fix bug when removing duplicate rows from user_ips
This commit is contained in:
Erik Johnston 2019-01-22 16:51:57 +00:00 committed by GitHub
parent 388c164aea 7f503f83b9
commit 12699a701f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
changelog.d/4434.misc Normal file
View file

@ -0,0 +1 @@
Apply a unique index to the user_ips table, preventing duplicates.

59
synapse/storage/client_ips.py
View file

@ -110,8 +110,13 @@ class ClientIpStore(background_updates.BackgroundUpdateStore):
@defer.inlineCallbacks
def _remove_user_ip_dupes(self, progress, batch_size):
# This works function works by scanning the user_ips table in batches
# based on `last_seen`. For each row in a batch it searches the rest of
# the table to see if there are any duplicates, if there are then they
# are removed and replaced with a suitable row.
last_seen_progress = progress.get("last_seen", 0)
# Fetch the start of the batch
begin_last_seen = progress.get("last_seen", 0)
def get_last_seen(txn):
txn.execute(
@ -122,29 +127,23 @@ class ClientIpStore(background_updates.BackgroundUpdateStore):
LIMIT 1
OFFSET ?
""",
(last_seen_progress, batch_size)
(begin_last_seen, batch_size)
)
results = txn.fetchone()
return results
row = txn.fetchone()
if row:
return row[0]
else:
return None
# Get a last seen that's sufficiently far away enough from the last one
last_seen = yield self.runInteraction(
# Get a last seen that has roughly `batch_size` since `begin_last_seen`
end_last_seen = yield self.runInteraction(
"user_ips_dups_get_last_seen", get_last_seen
)
if not last_seen:
# If we get a None then we're reaching the end and just need to
# delete the last batch.
last = True
# If it returns None, then we're processing the last batch
last = end_last_seen is None
# We fake not having an upper bound by using a future date, by
# just multiplying the current time by two....
last_seen = int(self.clock.time_msec()) * 2
else:
last = False
last_seen = last_seen[0]
def remove(txn, last_seen_progress, last_seen):
def remove(txn):
# This works by looking at all entries in the given time span, and
# then for each (user_id, access_token, ip) tuple in that range
# checking for any duplicates in the rest of the table (via a join).
@ -153,6 +152,16 @@ class ClientIpStore(background_updates.BackgroundUpdateStore):
# all other duplicates.
# It is efficient due to the existence of (user_id, access_token,
# ip) and (last_seen) indices.
# Define the search space, which requires handling the last batch in
# a different way
if last:
clause = "? <= last_seen"
args = (begin_last_seen,)
else:
clause = "? <= last_seen AND last_seen < ?"
args = (begin_last_seen, end_last_seen)
txn.execute(
"""
SELECT user_id, access_token, ip,
@ -160,13 +169,14 @@ class ClientIpStore(background_updates.BackgroundUpdateStore):
FROM (
SELECT user_id, access_token, ip
FROM user_ips
WHERE ? <= last_seen AND last_seen < ?
WHERE {}
ORDER BY last_seen
) c
INNER JOIN user_ips USING (user_id, access_token, ip)
GROUP BY user_id, access_token, ip
HAVING count(*) > 1""",
(last_seen_progress, last_seen)
HAVING count(*) > 1
""".format(clause),
args
)
res = txn.fetchall()
@ -194,12 +204,11 @@ class ClientIpStore(background_updates.BackgroundUpdateStore):
)
self._background_update_progress_txn(
txn, "user_ips_remove_dupes", {"last_seen": last_seen}
txn, "user_ips_remove_dupes", {"last_seen": end_last_seen}
)
yield self.runInteraction(
"user_ips_dups_remove", remove, last_seen_progress, last_seen
)
yield self.runInteraction("user_ips_dups_remove", remove)
if last:
yield self._end_background_update("user_ips_remove_dupes")