Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore more stray timelines in Accumulate #255

Merged
merged 4 commits into from
Aug 21, 2023
Merged

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Aug 17, 2023

Described in #211 (comment).

This is essentially cherry picked from #248, in particular the commit
8c7046e

This should prevent creating new snapshots that don't reflect the state of the room. We'll need a followup task to clean up bad snapshots.

Described in #211 (comment).

This is essentially cherry picked from #248, in particular the commit
    8c7046e

This should prevent creating new snapshots that don't reflect the state
of the room. We'll need a followup task to clean up bad snapshots.
@DMRobertson
Copy link
Contributor Author

I will need to

@DMRobertson DMRobertson marked this pull request as ready for review August 18, 2023 12:39
@DMRobertson
Copy link
Contributor Author

Easier than I expected to get the tests happy.

It looks like the test changes in #248 are fixups to ensuring that we never send a nonempty state block with an empty timeline. This PR targets empty state blocks and nonempty timelines, so I think that is irrelevant here.

@DMRobertson DMRobertson requested a review from kegsay August 18, 2023 12:46
Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems sane.

@DMRobertson DMRobertson merged commit 25abf05 into main Aug 21, 2023
@DMRobertson DMRobertson deleted the dmr/dont-snapshot branch August 21, 2023 09:40
DMRobertson pushed a commit that referenced this pull request Aug 22, 2023
These rooms have invalid state snapshots, leading to correctness
problems. The existence of such snapshots leaves us open to performance
problems due to gappy state updates.

We have prevented invalid state snapshots from being formed (#255, #266)
in the future. The remaining hole is to discard existing invalid
snapshots.

Closes #256.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants