Handle gappy state blocks in a performant way #329

DMRobertson · 2023-10-04T19:29:51Z

#310 was getting out of control. I did an interactive rebase, threw some things away, and wrote some new things. This is the result.

This PR:

Removes the mechanism of prepending state events to the timeline, creating new snapshots instead.
Emits cache invalidation messages when doing so.
Updates the user cache in response to an invalidation payload.

(3) was the hard part. When I looked at the data in the user cache, the important bits to get right after an invalidation were membership-related. Everything else either didn't look like it ought to be affected by an invalidation, or I didn't care enough to reload them (Spaces). This needs proper testing and I haven't got anything which does that yet.

We could maybe land (3) separately, but either way we need (3) before (1) and (2).

This lacks testing. We probably want an integration test to test a bunch of membership transitions in the gap:

unjoined -> join
unjoined -> invite
invite -> join
invite -> leave
join -> join {displayname change?}
join -> leave
join -> ban
join -> invite (presumably with a leave in the middle)

sync3/handler/handler.go

Remove PrependTimelineEvents Update unit test Tweak variable names, add ReplacedExistingSnapshot

Remove redundant comment Improve errmsg

DMRobertson · 2023-10-10T13:58:37Z

tests-integration/poller_test.go

+
+	t.Log("Alice should see the two most recent message in the timeline only. The room name should change too.")
+	res = v3.mustDoV3RequestWithPos(t, aliceToken, res.Pos, sync3.Request{})
+	m.MatchResponse(t, res, m.MatchRoomSubscription(roomID,


also check the prev batch token

state/invites_table.go

Needs proxy fixes to the prev_batch token handling

even if the initial load returns no rooms. This is an exceedingly rare situation since most people have accounts with rooms before using the proxy. But it does help first-time users and naive test authors.

DMRobertson · 2023-11-01T19:58:59Z

pubsub/v2.go

@@ -25,6 +25,7 @@ type V2Listener interface {
 	OnDeviceMessages(p *V2DeviceMessages)
 	OnExpiredToken(p *V2ExpiredToken)
 	OnInvalidateRoom(p *V2InvalidateRoom)
+	OnStateRedaction(p *V2StateRedaction)


Pulled out to #359

state/accumulator.go

DMRobertson · 2023-11-01T20:00:19Z

state/accumulator_test.go

Probably want to have a unit test of the new snapshotting logic in Intialise, if I didn't add one already.

DMRobertson · 2023-11-01T20:00:47Z

state/storage.go

+	joins map[string]Event,
+	invites map[string][]json.RawMessage,
+	leaves map[string]json.RawMessage,


May not need the data here---just the user IDs.

sync3/caches/global.go

DMRobertson · 2023-11-01T20:01:35Z

sync3/caches/global.go

+	// ForcePrevBatch is nonempty when the proxy doesn't know the timeline event immediately
+	// prior to this one. Its value is the prevBatch for this event. If set, this token MUST
+	// be sent to sync3 clients (or else there'll be a gap in their timeline).
+	ForcePrevBatch string


TODO: do we still need this? Should it be pulled out to another PR?

The motivation was:

sliding-sync/sync3/handler/connstate_live.go

Lines 237 to 243 in b8e34a7

// Normally live events don't need a prevBatch because they follow up

// from a previous sync response with a prevBatch. BUT after a gappy poll

// where the user has joined a room, the first time they see a timeline event

// for this room is here. So we need to provide them a prevBatch token.

if roomEventUpdate.EventData.ForcePrevBatch != "" {

r.PrevBatch = roomEventUpdate.EventData.ForcePrevBatch

}

So the scenario here is seeing in a live update that you've joined a room via a gappy poll.

I think this should actually be impossible now, because we'll kill your connection.

DMRobertson · 2023-11-01T20:03:09Z

sync3/handler/connstate.go

-	if s.anchorLoadPosition <= 0 {
+	if s.anchorLoadPosition < 0 {


Why did we do this again?

DMRobertson · 2023-11-01T20:03:45Z

sync3/handler/connstate_test.go

+		RoomID: roomA.RoomID,
+		JSON:   newEvent,
+	}
+	dispatcher.OnNewEvent(context.Background(), roomA.RoomID, newEventWrapped, 1)


Do we still need this change to OnNewEvent?

sync3/handler/handler.go

DMRobertson · 2023-11-01T20:05:25Z

tests-e2e/gappy_state_test.go

 	m.MatchResponse(
 		t,
 		syncResp,
 		m.MatchRoomSubscription(
 			roomID,
 			m.MatchRoomName("potato"),
-			MatchRoomTimelineMostRecent(1, []Event{{ID: latestMessageID}}),
+			MatchRoomTimelineMostRecent(1, []Event{{ID: lastMsgID}}),


Need to take another look at this, I have no memory of this place.

tests-integration/v3_test.go

Not used yet; pulled out of #329.

Pulled out of #329.

DMRobertson · 2023-11-03T17:24:27Z

Superseded by #363.

I don't fully remember the details, but may as well pluck this from the previous PR.

DMRobertson changed the title ~~Second attempt to update the u~~ Handle gappy state blocks in a performant way Oct 4, 2023

DMRobertson mentioned this pull request Oct 4, 2023

Initialise: make snapshots instead of prepending state events #310

Closed

DMRobertson force-pushed the dmr/resnapshot-2 branch from 9055551 to 91432b1 Compare October 4, 2023 19:34

DMRobertson commented Oct 4, 2023

View reviewed changes

sync3/handler/handler.go Outdated Show resolved Hide resolved

David Robertson added 3 commits October 10, 2023 12:11

Tweak existing E2E test

d4eace7

Write integration test too

1bd4f12

move keys() helper to internal

44ef0c3

DMRobertson force-pushed the dmr/resnapshot-2 branch from 6167aad to 6db1d22 Compare October 10, 2023 11:30

David Robertson added 6 commits October 10, 2023 12:30

New Storage functions

c1b41f4

Handle state in Initialise after first snapshot

638554c

Remove PrependTimelineEvents Update unit test Tweak variable names, add ReplacedExistingSnapshot

Emit cache invalidation for a replaced snapshot

2f6a5cf

Reminder for me in the future

d08f82a

Move NewEventData to free function

32479e2

Make UserCaches respond to gappy memberships

db7e94d

Remove redundant comment Improve errmsg

DMRobertson force-pushed the dmr/resnapshot-2 branch from 6db1d22 to db7e94d Compare October 10, 2023 11:30

DMRobertson commented Oct 10, 2023

View reviewed changes

state/invites_table.go Outdated Show resolved Hide resolved

David Robertson and others added 9 commits October 10, 2023 15:13

Fix stupid syntax

c2f99a6

Fixes to the test

19859fe

Add comments for our future selves

4cc3c31

Select less data

6e03c2d

Fix stupid bug in internal.Keys

899000c

Merge remote-tracking branch 'origin/main' into dmr/resnapshot-2

08d1034

Update tests. prevBatches are broken

a1ded0c

Add nil guards to loops

abeb273

Merge branch 'main' into dmr/resnapshot-2

ac17cb4

DMRobertson mentioned this pull request Oct 17, 2023

Don't leak pre-emptive bans #342

Merged

David Robertson added 3 commits October 17, 2023 18:42

Change the test to have a timeline with 10 msgs

c3dfd66

Needs proxy fixes to the prev_batch token handling

ConnState: only call load once

ae9777d

even if the initial load returns no rooms. This is an exceedingly rare situation since most people have accounts with rooms before using the proxy. But it does help first-time users and naive test authors.

WIP big-ass table test

4b3a617

Update existing integration tests

5b0c8e3

DMRobertson mentioned this pull request Nov 1, 2023

Add a separate payload for redacting state #359

Merged

DMRobertson commented Nov 1, 2023

View reviewed changes

state/accumulator.go Show resolved Hide resolved

DMRobertson commented Nov 1, 2023

View reviewed changes

sync3/caches/global.go Show resolved Hide resolved

DMRobertson commented Nov 1, 2023

View reviewed changes

sync3/handler/handler.go Show resolved Hide resolved

DMRobertson commented Nov 1, 2023

View reviewed changes

tests-integration/v3_test.go Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into dmr/resnapshot-2

ebe2641

DMRobertson force-pushed the dmr/resnapshot-2 branch from 786691a to ebe2641 Compare November 2, 2023 13:05

David Robertson added 4 commits November 2, 2023 13:43

Fix test typo

9a40fb7

Also test that bob sees prev batch

8ba3661

Simplify FetchMembership

b72d682

Merge branch 'main' into dmr/resnapshot-2

518f1a6

DMRobertson pushed a commit that referenced this pull request Nov 2, 2023

Add the ability to reset the JoinedRoomsTracker

4897acf

Not used yet; pulled out of #329.

DMRobertson mentioned this pull request Nov 2, 2023

Add the ability to reset the JoinedRoomsTracker #361

Merged

DMRobertson pushed a commit that referenced this pull request Nov 2, 2023

Add FetchMemberships function

f0ea7cb

Pulled out of #329.

DMRobertson mentioned this pull request Nov 2, 2023

Add FetchMemberships function #362

Merged

David Robertson added 3 commits November 2, 2023 22:31

Merge remote-tracking branch 'origin/main' into dmr/resnapshot-2

eaeff79

Fix bad merge

fe7c5a2

Remove uninsteresting gofmt change

b8e34a7

DMRobertson mentioned this pull request Nov 3, 2023

Handle gappy state with a single snapshot, attempt 3 #363

Merged

DMRobertson closed this Nov 3, 2023

DMRobertson pushed a commit that referenced this pull request Nov 3, 2023

E2E test changes from #329.

2052632

I don't fully remember the details, but may as well pluck this from the previous PR.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle gappy state blocks in a performant way #329

Handle gappy state blocks in a performant way #329

DMRobertson commented Oct 4, 2023 •

edited

Loading

DMRobertson Oct 10, 2023

DMRobertson Nov 1, 2023

DMRobertson Nov 1, 2023

DMRobertson Nov 1, 2023

DMRobertson Nov 1, 2023

DMRobertson Nov 3, 2023

DMRobertson Nov 3, 2023

DMRobertson Nov 1, 2023

DMRobertson Nov 1, 2023

DMRobertson Nov 1, 2023

DMRobertson commented Nov 3, 2023

	// Normally live events don't need a prevBatch because they follow up
	// from a previous sync response with a prevBatch. BUT after a gappy poll
	// where the user has joined a room, the first time they see a timeline event
	// for this room is here. So we need to provide them a prevBatch token.
	if roomEventUpdate.EventData.ForcePrevBatch != "" {
	r.PrevBatch = roomEventUpdate.EventData.ForcePrevBatch
	}

Handle gappy state blocks in a performant way #329

Handle gappy state blocks in a performant way #329

Conversation

DMRobertson commented Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DMRobertson commented Nov 3, 2023

DMRobertson commented Oct 4, 2023 •

edited

Loading