Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Commit c9d981f

Browse files
erikjohnstonerikjohnston
committed
Add tests and fixup
1 parent d744e92 commit c9d981f

File tree

2 files changed

+213
-6
lines changed

2 files changed

+213
-6
lines changed

‎synapse/storage/persist_events.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,14 @@ async def _persist_events(
463463
)
464464
current_state, delta_ids, new_latest_event_ids = res
465465

466+
# there should always be at least one forward extremity.
467+
# (except during the initial persistence of the send_join
468+
# results, in which case there will be no existing
469+
# extremities, so we'll `continue` above and skip this bit.)
470+
assert new_latest_event_ids, "No forward extremities left!"
471+
472+
new_forward_extremeties[room_id] = new_latest_event_ids
473+
466474
# If either are not None then there has been a change,
467475
# and we need to work out the delta (or use that
468476
# given)
@@ -760,22 +768,28 @@ async def _get_new_state_after_events(
760768

761769
dropped = set(new_latest_event_ids) - new_new_extrems
762770

771+
logger.debug("Might drop events: %s", dropped)
772+
763773
# We only drop events if:
764774
# 1. we're not currently persisting them;
765775
# 2. they're not our own events (or are dummy events); and
766-
# 3. they're either over N minutes old or we have newer events
767-
# from the same domain.
776+
# 3. they're either:
777+
# 1. over N hours old and more than N events ago (we use depth
778+
# to calculate); or
779+
# 2. we are persisting an event from the same domain.
768780
#
769781
# The idea is that we don't want to drop events that are "legitmate"
770782
# extremities (that we would want to include as prev events), only
771783
# "stuck" extremities that are e.g. due to a gap in the graph.
772784
#
773785
# Note that we either drop all of them or none of them. If we only
774-
# drop some of the events we don't know if state res would come to the same conclusion.
786+
# drop some of the events we don't know if state res would come to
787+
# the same conclusion.
775788

776789
drop = True
777790
for ev, _ in events_context:
778791
if ev.event_id in dropped:
792+
logger.debug("Not dropping as in to persist list")
779793
drop = False
780794
break
781795

@@ -786,26 +800,37 @@ async def _get_new_state_after_events(
786800
redact_behaviour=EventRedactBehaviour.AS_IS,
787801
)
788802

789-
new_senders = {e.sender for e, _ in events_context}
803+
new_senders = {get_domain_from_id(e.sender) for e, _ in events_context}
790804

791-
twenty_minutes_ago = self._clock.time_msec() - 20 * 60 * 1000
805+
one_day_ago = self._clock.time_msec() - 20 * 60 * 1000
806+
current_depth = max(e.depth for e, _ in events_context)
792807
for event in dropped_events.values():
793808
if (
794809
self.is_mine_id(event.sender)
795810
and event.type != "org.matrix.dummy_event"
796811
):
812+
logger.debug("Not dropping own event")
797813
drop = False
798814
break
799815

800-
if event.origin_server_ts < twenty_minutes_ago:
816+
if (
817+
event.origin_server_ts < one_day_ago
818+
and event.depth < current_depth - 100
819+
):
801820
continue
802821
if get_domain_from_id(event.sender) in new_senders:
803822
continue
804823

824+
logger.debug(
825+
"Not dropping as too new and not in new_senders: %s",
826+
new_senders,
827+
)
828+
805829
drop = False
806830
break
807831

808832
if drop:
833+
logger.debug("Dropping events %s", dropped)
809834
new_latest_event_ids = new_new_extrems
810835

811836
return res.state, None, new_latest_event_ids

‎tests/storage/test_events.py

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
# -*- coding: utf-8 -*-
2+
# Copyright 2020 The Matrix.org Foundation C.I.C.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
17+
from synapse.api.constants import EventTypes, Membership
18+
from synapse.api.room_versions import RoomVersions
19+
from synapse.federation.federation_base import event_from_pdu_json
20+
from synapse.rest import admin
21+
from synapse.rest.client.v1 import login, room
22+
23+
from tests.unittest import HomeserverTestCase
24+
25+
26+
class ExtremPruneTestCase(HomeserverTestCase):
27+
servlets = [
28+
admin.register_servlets,
29+
room.register_servlets,
30+
login.register_servlets,
31+
]
32+
33+
def test_prune_gap(self):
34+
"""Test that we drop extremities after a gap when we see an event from
35+
the same domain.
36+
"""
37+
38+
state = self.hs.get_state_handler()
39+
persistence = self.hs.get_storage().persistence
40+
store = self.hs.get_datastore()
41+
42+
self.register_user("user", "pass")
43+
token = self.login("user", "pass")
44+
room_id = self.helper.create_room_as(
45+
"user", room_version=RoomVersions.V6.identifier, tok=token
46+
)
47+
48+
body = self.helper.send(room_id, body="Test", tok=token)
49+
local_message_event_id = body["event_id"]
50+
51+
# Fudge a remote event an persist it. This will be the extremity before
52+
# the gap.
53+
remote_event_1 = event_from_pdu_json(
54+
{
55+
"type": EventTypes.Message,
56+
"content": {},
57+
"room_id": room_id,
58+
"sender": "@user:other",
59+
"depth": 5,
60+
"prev_events": [local_message_event_id],
61+
"auth_events": [],
62+
"origin_server_ts": self.clock.time_msec(),
63+
},
64+
RoomVersions.V6,
65+
)
66+
67+
context = self.get_success(state.compute_event_context(remote_event_1))
68+
self.get_success(persistence.persist_event(remote_event_1, context))
69+
70+
# Check that the current extremities is the remote event.
71+
extremities = self.get_success(store.get_prev_events_for_room(room_id))
72+
self.assertCountEqual(extremities, [remote_event_1.event_id])
73+
74+
# Fudge a second event which points to an event we don't have. This is a
75+
# state event so that the state changes (otherwise we won't prune the
76+
# extremity as they'll have the same state group).
77+
remote_event_2 = event_from_pdu_json(
78+
{
79+
"type": EventTypes.Member,
80+
"state_key": "@user:other",
81+
"content": {"membership": Membership.JOIN},
82+
"room_id": room_id,
83+
"sender": "@user:other",
84+
"depth": 10,
85+
"prev_events": ["$some_unknown_message"],
86+
"auth_events": [],
87+
"origin_server_ts": self.clock.time_msec(),
88+
},
89+
RoomVersions.V6,
90+
)
91+
92+
state_before_gap = self.get_success(state.get_current_state(room_id))
93+
94+
context = self.get_success(
95+
state.compute_event_context(
96+
remote_event_2, old_state=state_before_gap.values()
97+
)
98+
)
99+
100+
self.get_success(persistence.persist_event(remote_event_2, context))
101+
102+
# Check the new extremity is just the new remote event.
103+
extremities = self.get_success(store.get_prev_events_for_room(room_id))
104+
self.assertCountEqual(extremities, [remote_event_2.event_id])
105+
106+
def test_dont_prune_gap_if_state_different(self):
107+
"""Test that we don't prune extremities after a gap if the resolved
108+
state is different.
109+
"""
110+
111+
state = self.hs.get_state_handler()
112+
persistence = self.hs.get_storage().persistence
113+
store = self.hs.get_datastore()
114+
115+
self.register_user("user", "pass")
116+
token = self.login("user", "pass")
117+
room_id = self.helper.create_room_as(
118+
"user", room_version=RoomVersions.V6.identifier, tok=token
119+
)
120+
121+
body = self.helper.send(room_id, body="Test", tok=token)
122+
local_message_event_id = body["event_id"]
123+
124+
# Fudge a remote event an persist it. This will be the extremity before
125+
# the gap.
126+
remote_event_1 = event_from_pdu_json(
127+
{
128+
"type": EventTypes.Message,
129+
"state_key": "@user:other",
130+
"content": {},
131+
"room_id": room_id,
132+
"sender": "@user:other",
133+
"depth": 5,
134+
"prev_events": [local_message_event_id],
135+
"auth_events": [],
136+
"origin_server_ts": self.clock.time_msec(),
137+
},
138+
RoomVersions.V6,
139+
)
140+
141+
context = self.get_success(state.compute_event_context(remote_event_1))
142+
self.get_success(persistence.persist_event(remote_event_1, context))
143+
144+
# Check that the current extremities is the remote event.
145+
extremities = self.get_success(store.get_prev_events_for_room(room_id))
146+
self.assertCountEqual(extremities, [remote_event_1.event_id])
147+
148+
# Fudge a second event which points to an event we don't have.
149+
remote_event_2 = event_from_pdu_json(
150+
{
151+
"type": EventTypes.Message,
152+
"state_key": "@user:other",
153+
"content": {},
154+
"room_id": room_id,
155+
"sender": "@user:other",
156+
"depth": 10,
157+
"prev_events": ["$some_unknown_message"],
158+
"auth_events": [],
159+
"origin_server_ts": self.clock.time_msec(),
160+
},
161+
RoomVersions.V6,
162+
)
163+
164+
# Now we persist it with state with a dropped history visibility
165+
# setting. The state resolution across the old and new event will then
166+
# include it, and so the resolved state won't match the new state.
167+
state_before_gap = dict(self.get_success(state.get_current_state(room_id)))
168+
state_before_gap.pop(("m.room.history_visibility", ""))
169+
170+
context = self.get_success(
171+
state.compute_event_context(
172+
remote_event_2, old_state=state_before_gap.values()
173+
)
174+
)
175+
176+
self.get_success(persistence.persist_event(remote_event_2, context))
177+
178+
# Check that we haven't dropped the old extremity.
179+
extremities = self.get_success(store.get_prev_events_for_room(room_id))
180+
self.assertCountEqual(
181+
extremities, [remote_event_1.event_id, remote_event_2.event_id]
182+
)

0 commit comments

Comments
 (0)