This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Try and drop stale extremities. #8929
Try and drop stale extremities. #8929
Changes from all commits
f12cd82
b4d7414
a24c850
3f80eb5
3dae3ea
cb242bc
c12c988
f6c0e6e
4b28064
c79228d
2a08d28
f833656
4b8de3c
082ecbe
5d0629c
9ce4277
62c30e8
a4d8bd9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this now seems to be in conflict with 'the extremity must give the same state as the "current state"'. In the rooms where this is a problem, it's relatively uncommon to make it 20 events without a state change. My concern is therefore that this whole algorithm will never actually be useful.
Why 20 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State changes are fine, what we're looking for is that the resolved state with all extremities is the same as the resolved state without the old extremities. This should be true for simple gaps (rather than forks in the graph for example).
It's a fairly arbitrary low but non-zero number. What I saw on jki.re over the weekend is that we'd get two extremities from matrix.org and then immediately prune one, even though a new event will come along and point to both, which feels a bit wrong. Maybe using depth here is sort of the wrong thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should also add a check for it being an hour old, or something? I guess we're saying dropping events shouldn't be problematic for remote events, but I could imagine a case where we have a lot of events sent all at once and so we end up with lots of extremities that will take some time to be referenced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at this point we should probably just deploy something and maybe iterate on it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, agreed