Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

event_forward_extremities are not correctly cleared by persistence of successor events #8913

Open
richvdh opened this issue Dec 10, 2020 · 8 comments
Labels
A-DAG Directed acyclic graph of events (events connected by prev_events) A-Federation A-Performance Performance, both client-facing and admin-facing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@richvdh
Copy link
Member

richvdh commented Dec 10, 2020

Suppose:

  • we have a linear event DAG, A <- B <- ... <- C, where C is a regular event, which references B in its state.
  • our homeserver has persisted up to event A, hence has a single event_forward_extremity of A.
  • our homeserver goes offline for a bit, so misses out on B and immediate successors
  • our homeserver receives C over federation, hence pulls in B to complete the state at C

Empirically:

  • we have all the state we need for B: it is not marked as an outlier
  • we end up with event_forward_extremities of A and C

We have all the successors for A we should ever expect to receive, so I assert that it should no longer be considered an extremity. In particular, there is no way it will ever cease to be a forward extremity, so it contributes to the accumulation of forward extremities (cf #1760).

@richvdh
Copy link
Member Author

richvdh commented Dec 10, 2020

(in the above example, I would expect to end up with event_forward_extremities of B and C.)

@clokep clokep added z-bug (Deprecated Label) z-p2 (Deprecated Label) A-Performance Performance, both client-facing and admin-facing labels Dec 10, 2020
@MadLittleMods
Copy link
Contributor

Closing in favor of #6607

@richvdh
Copy link
Member Author

richvdh commented Apr 16, 2023

I'm not convinced they are quite the same. #6607 is specific to de-outliering; this doesn't involve outliers.

That said, both issues are pretty old; it's entirely possible that one or both have been fixed.

@DMRobertson DMRobertson added T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. A-DAG Directed acyclic graph of events (events connected by prev_events) and removed z-bug (Deprecated Label) z-p2 (Deprecated Label) labels Apr 24, 2023
@DMRobertson
Copy link
Contributor

We have all the successors for A we should ever expect to receive, so I assert that it should no longer be considered an extremity.

Agreed!

In particular, there is no way it will ever cease to be a forward extremity, so it contributes to the accumulation of forward extremities (cf #1760).

Is this still true in the light of the dummy events created by #5884 ?

@DMRobertson DMRobertson added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Apr 24, 2023
@DMRobertson
Copy link
Contributor

We have all the successors for A we should ever expect to receive, so I assert that it should no longer be considered an extremity.

Agreed!

Aside: isn't it enough to see a single ancestor of A for A to no longer be considered an extremity?

@richvdh
Copy link
Member Author

richvdh commented Apr 25, 2023

In particular, there is no way it will ever cease to be a forward extremity, so it contributes to the accumulation of forward extremities (cf #1760).

Is this still true in the light of the dummy events created by #5884 ?

No. Nevertheless dummy events are a bit of a kludge and there could be fewer of them if we didn't create spurious forward extremities.

Aside: isn't it enough to see a single ancestor of A for A to no longer be considered an extremity?

I'm not really following. No number of ancestors of A changes whether A is a forward extremity. Maybe you mean backward extremities but they are different anyway.

@DMRobertson
Copy link
Contributor

Aside: isn't it enough to see a single ancestor of A for A to no longer be considered an extremity?

I'm not really following. No number of ancestors of A changes whether A is a forward extremity. Maybe you mean backward extremities but they are different anyway.

I was writing nonsense. What I should have written is:

Isn't it enough to see a single ancestor descendant of A for A to no longer be considered an forward extremity?

@richvdh
Copy link
Member Author

richvdh commented Apr 25, 2023

Isn't it enough to see a single ancestor descendant of A for A to no longer be considered an forward extremity?

ah. Then yes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-DAG Directed acyclic graph of events (events connected by prev_events) A-Federation A-Performance Performance, both client-facing and admin-facing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

4 participants