Fix RewritesState bug #1557
Fix RewritesState bug #1557
Conversation
| @@ -125,7 +124,8 @@ func (u *latestEventsUpdater) doUpdateLatestEvents() error { | |||
| // then start with an empty set - none of the forward extremities | |||
| // that we knew about before matter anymore. | |||
| oldLatest := []types.StateAtEventAndReference{} | |||
| if !u.stateAtEvent.Overwrite { | |||
| if !u.rewritesState { | |||
| u.oldStateNID = u.updater.CurrentStateSnapshotNID() | |||
There was a problem hiding this comment.
Whilst this makes sense, why did you change it so it's only set if we don't rewrite state? I think updater.CurrentStateSnapshotNID() just returns an int - the New... function did the DB query so it's not even an optimisation. Maybe to act as a guard against accidentally fiddling with it and calculating deltas against things we shouldn't?
There was a problem hiding this comment.
The idea is that, if we are rewriting state, we want the state snapshot NID to be zero because that means that the deltas produced later on will be against an empty snapshot (effectively a complete rewrite, where AddsStateEventIDs will contain the entire new state).
If we aren't rewriting state, we want to know what the current state is so that we produce a real delta.
Apparently we were overwriting
RewritesStatewhen generating new output events and didn't bother to checku.rewritesStatethe second time. This meant that we were probably sending rewrite output events far more often than we intended to, confusing downstream component state.This also updates the sync API to only discard state, rather than the entire room history, since that can cause problems with events going missing from the timeline, and also fixes a bug that can result in a panic if the federation sender gets the same joined hosts more than once.