From 8e8dfb4803b9ed8c39f38f6e603b1d8015032cfa Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 10 Sep 2018 23:59:16 +0100 Subject: [PATCH 1/8] disable LL for incr syncs, and log incr sync stats See vector-im/riot-web#7211 for details --- synapse/handlers/sync.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 39ef2af08e8f..2407d4278f5d 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -736,6 +736,13 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke lazy_load_members=lazy_load_members, ) elif batch.limited: + # for now, we disable LL for gappy syncs - see + # https://github.com/vector-im/riot-web/issues/7211#issuecomment-419976346 + # N.B. this slows down incr syncs as we are now processing way + # more state in the server than if we were LLing. + types = None + filtered_types = None + state_at_previous_sync = yield self.get_state_at( room_id, stream_position=since_token, types=types, filtered_types=filtered_types, @@ -756,13 +763,13 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke timeline_start=state_at_timeline_start, previous=state_at_previous_sync, current=current_state_ids, - lazy_load_members=lazy_load_members, + lazy_load_members=False, # N.B. overridden to disable LL ) else: state_ids = {} if lazy_load_members: if types: - # We're returning an incremental (or initial) sync, with no + # We're returning an incremental sync, with no # "gap" since the previous sync, so normally there would be # no state to return. # But we're lazy-loading, so the client might need some more @@ -1681,6 +1688,16 @@ def _generate_room_entry(self, sync_result_builder, ignored_users, unread_notifications["highlight_count"] = notifs["highlight_count"] sync_result_builder.joined.append(room_sync) + + if batch.limited: + user_id = sync_result_builder.sync_config.user.to_string() + logger.info( + "Incremental syncing room %s for user %s with %d state events" % ( + room_id, + user_id, + len(state), + ) + ) elif room_builder.rtype == "archived": room_sync = ArchivedSyncResult( room_id=room_id, From 18dfb4a03fb810b32ce28fb6a3eae50aeaf6263a Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 11 Sep 2018 00:01:29 +0100 Subject: [PATCH 2/8] changelog --- changelog.d/3840.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3840.misc diff --git a/changelog.d/3840.misc b/changelog.d/3840.misc new file mode 100644 index 000000000000..b9585ae9bee4 --- /dev/null +++ b/changelog.d/3840.misc @@ -0,0 +1 @@ +Disable lazy loading for incremental syncs for now From 92c918d25375cecceb2f8dee6cc059ad18dff35f Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 11 Sep 2018 00:37:57 +0100 Subject: [PATCH 3/8] We actually have to include state for LL members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit as the user might not have heard of them before, even though we’re not LLing members in incremental syncs --- synapse/handlers/sync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 2407d4278f5d..c8cf5aa9ab53 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -763,8 +763,8 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke timeline_start=state_at_timeline_start, previous=state_at_previous_sync, current=current_state_ids, - lazy_load_members=False, # N.B. overridden to disable LL - ) + # we have to include LL members in case LL initial sync missed them + lazy_load_members=lazy_load_members, else: state_ids = {} if lazy_load_members: From a55facc887fcdbd7d7fd90a1f5395f3124291359 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 11 Sep 2018 00:40:58 +0100 Subject: [PATCH 4/8] fix github-on-phone typo --- synapse/handlers/sync.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index c8cf5aa9ab53..0d4c1178fd5b 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -765,6 +765,7 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke current=current_state_ids, # we have to include LL members in case LL initial sync missed them lazy_load_members=lazy_load_members, + ) else: state_ids = {} if lazy_load_members: From 220166206045be3eecaf928b1474f48fb69493aa Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 11 Sep 2018 11:09:48 +0100 Subject: [PATCH 5/8] correctly disable LL for incr sync --- synapse/handlers/sync.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 0d4c1178fd5b..93d37a47644d 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -736,10 +736,18 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke lazy_load_members=lazy_load_members, ) elif batch.limited: + state_at_timeline_start = yield self.store.get_state_ids_for_event( + batch.events[0].event_id, types=types, + filtered_types=filtered_types, + ) + # for now, we disable LL for gappy syncs - see # https://github.com/vector-im/riot-web/issues/7211#issuecomment-419976346 # N.B. this slows down incr syncs as we are now processing way # more state in the server than if we were LLing. + # + # We still have to filter timeline_start to LL entries (above) in order + # for _calculate_state's LL logic to work. types = None filtered_types = None @@ -753,10 +761,11 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke filtered_types=filtered_types, ) - state_at_timeline_start = yield self.store.get_state_ids_for_event( - batch.events[0].event_id, types=types, - filtered_types=filtered_types, - ) + # pp = pprint.PrettyPrinter(indent=4) + # logger.info("timeline_contains: %s", pp.pformat(timeline_state)) + # logger.info("timeline_start: %s", pp.pformat(state_at_timeline_start)) + # logger.info("previous: %s", pp.pformat(state_at_previous_sync)) + # logger.info("current: %s", pp.pformat(current_state_ids)) state_ids = _calculate_state( timeline_contains=timeline_state, @@ -765,7 +774,9 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke current=current_state_ids, # we have to include LL members in case LL initial sync missed them lazy_load_members=lazy_load_members, - ) + ) + + # logger.info("result: %s", pp.pformat(state_ids)) else: state_ids = {} if lazy_load_members: From c81dba4a5d9d7653e924b984ca9a9136b85e85a8 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 11 Sep 2018 11:59:24 +0100 Subject: [PATCH 6/8] improve comment --- synapse/handlers/sync.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 93d37a47644d..cd219504d8a7 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -747,7 +747,12 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke # more state in the server than if we were LLing. # # We still have to filter timeline_start to LL entries (above) in order - # for _calculate_state's LL logic to work. + # for _calculate_state's LL logic to work, as we have to include LL + # members for timeline senders in case they weren't loaded in the initial + # sync. We do this by (counterintuitively) by filtering timeline_start + # members to just be ones which were timeline senders, which then ensures + # all of the rest get included in the state block (if we need to know about + # them). types = None filtered_types = None From 1483fc6660fe8f4c2b7d4a90dcb5847571228f39 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 11 Sep 2018 12:10:04 +0100 Subject: [PATCH 7/8] remove matthewdebug at erik's req --- synapse/handlers/sync.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index cd219504d8a7..639da19e7295 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -766,12 +766,6 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke filtered_types=filtered_types, ) - # pp = pprint.PrettyPrinter(indent=4) - # logger.info("timeline_contains: %s", pp.pformat(timeline_state)) - # logger.info("timeline_start: %s", pp.pformat(state_at_timeline_start)) - # logger.info("previous: %s", pp.pformat(state_at_previous_sync)) - # logger.info("current: %s", pp.pformat(current_state_ids)) - state_ids = _calculate_state( timeline_contains=timeline_state, timeline_start=state_at_timeline_start, @@ -780,8 +774,6 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke # we have to include LL members in case LL initial sync missed them lazy_load_members=lazy_load_members, ) - - # logger.info("result: %s", pp.pformat(state_ids)) else: state_ids = {} if lazy_load_members: From 6167eb822c8f911b219d841c9ad5ab90c09dfc4e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 11 Sep 2018 12:17:55 +0100 Subject: [PATCH 8/8] fix pep8 --- synapse/handlers/sync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 639da19e7295..23983a51ab18 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -751,8 +751,8 @@ def compute_state_delta(self, room_id, batch, sync_config, since_token, now_toke # members for timeline senders in case they weren't loaded in the initial # sync. We do this by (counterintuitively) by filtering timeline_start # members to just be ones which were timeline senders, which then ensures - # all of the rest get included in the state block (if we need to know about - # them). + # all of the rest get included in the state block (if we need to know + # about them). types = None filtered_types = None