feat: Only track appended extremities #2356

Open
nex wants to merge 10 commits from nex/feat/intuitive-extremity-tracking into main
Owner

Previously, it was possible for any arbitrary event to be added to our extremity table. Now, it is only possible while appending events. This PR replaces extremity tracking in the following ways:

  1. Extremities are added non-atomically, rather than whole-sale replacements. This reduces the margin for error.
  2. Events being "referenced" is now synonymous with being a forward extremity - if an event is not in the extremity set, then it has already been referenced.
  3. Incoming events are always added as extremities, and predecessors are always removed.
  4. The unaccepted tree walk process was also simplified.

Basically, this PR just makes us do it right. Again.
We also get a free perf boost with lock-free timeline events, due to the non-atomicity of extremities!

TODO write a migration that drops referencedevents

Pull request checklist:

  • This pull request targets the main branch, and the branch is named something other than
    main.
  • I have written an appropriate pull request title and my description is clear.
  • I understand I am responsible for the contents of this pull request.
  • I have followed the contributing guidelines:
Previously, it was possible for any arbitrary event to be added to our extremity table. Now, it is only possible while appending events. This PR replaces extremity tracking in the following ways: 1. Extremities are added non-atomically, rather than whole-sale replacements. This reduces the margin for error. 2. Events being "referenced" is now synonymous with being a forward extremity - if an event is *not* in the extremity set, then it has already been referenced. 3. Incoming events are *always* added as extremities, and predecessors are *always* removed. 4. The unaccepted tree walk process was also simplified. Basically, this PR just makes us do it right. Again. We also get a free perf boost with lock-free timeline events, due to the non-atomicity of extremities! **TODO** write a migration that drops `referencedevents` <!-- Example: This pull request allows us to warp through time and space ten times faster than before by double-inverting the warp drive with hyperheated jump fluid, both making the drive faster and more efficient. This resolves the common issue where we have to wait more than 10 milliseconds to engage, use, and disengage the warp drive when travelling between galaxies. --> <!-- Closes: #... --> <!-- Fixes: #... --> <!-- Uncomment the above line(s) if your pull request fixes an issue or closes another pull request by superseding it. Replace `#...` with the issue/pr number, such as `#123`. --> **Pull request checklist:** <!-- You need to complete these before your PR can be considered. If you aren't sure about some, feel free to ask for clarification in #dev:continuwuity.org. --> - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [x] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. <!-- Notes on these requirements: - While not required, we encourage you to sign your commits with GPG or SSH to attest the authenticity of your changes. - While we allow LLM-assisted contributions, we do not appreciate contributions that are low quality, which is typical of machine-generated contributions that have not had a lot of love and care from a human. Please do not open a PR if all you have done is asked ChatGPT to tidy up the codebase with a +-100,000 diff. - In the case of code style violations, reviewers may leave review comments/change requests indicating what the ideal change would look like. For example, a reviewer may suggest you lower a log level, or use `match` instead of `if/else` etc. - In the case of code style violations, pre-commit check failures, minor things like typos/spelling errors, and in some cases commit format violations, reviewers may modify your branch directly, typically by making changes and adding a commit. Particularly in the latter case, a reviewer may rebase your commits to squash "spammy" ones (like "fix", "fix", "actually fix"), and reword commit messages that don't satisfy the format. - Pull requests MUST pass the `Checks` CI workflows to be capable of being merged. This can only be bypassed in exceptional circumstances. If your CI flakes, let us know in matrix:r/dev:continuwuity.org. - Pull requests have to be based on the latest `main` commit before being merged. If the main branch changes while you're making your changes, you should make sure you rebase on main before opening a PR. Your branch will be rebased on main before it is merged if it has fallen behind. - We typically only do fast-forward merges, so your entire commit log will be included. Once in main, it's difficult to get out cleanly, so put on your best dress, smile for the cameras! --> [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages
feat: Only track appended extremities
All checks were successful
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 3s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m5s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m15s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 21m12s
2f781a2324
perf: Don't hold state lock when unnecessary
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m3s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
10ca2f5175
feat: Always run mutex assertion outside maxperf
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
9ae7ed4db3
Also makes it not a panic
nex requested review from Owners 2026-10-13 00:15:34 +00:00
nex force-pushed nex/feat/intuitive-extremity-tracking from 9ae7ed4db3
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
to 4b65a412a8
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m3s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m20s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 3m1s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 20m55s
2026-10-13 00:17:15 +00:00
Compare
fix: Remove dead references to mark_event_referenced
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 10s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m4s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m8s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 40m23s
643526fce7
fix: Correctly remove extremities after creating a new event
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
bb23b1fdd9
style: reformat
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 12s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m10s
Documentation / Build and Deploy Documentation (pull_request) Successful in 10m32s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 40m36s
3998c3da06
fix: Make migration handle excessive extremities too
Some checks failed
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m12s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m5s
Documentation / Build and Deploy Documentation (pull_request) Successful in 10m30s
412be27df2
fix: Only consider accepted PDUs in extremity consistency check
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 20m52s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 40m35s
0bfa0f77a8
fix: Make the extremity check consistency migration faster
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m7s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m15s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 20m51s
c5dd81fc4c
@ -454,3 +454,3 @@
sender_user,
replacement_room_id.as_deref(),
&new_room_state_lock,
Some(&new_room_state_lock),
Owner

I feel like you can deal with the TODO above this comment now

I feel like you can deal with the TODO above this comment now
Jade left a comment
  • I think there's a race between append_incoming_pdu and append_pdu wheretwo events are added quickly in sequence. and the add_forward_extremity call in append_pdu might not be needed?
  • I think you can probably avoid looking up all prev_events for events that are already accepted, too, which should make fast-path event insertion cheaper
- I think there's a race between append_incoming_pdu and append_pdu wheretwo events are added quickly in sequence. and the add_forward_extremity call in append_pdu might not be needed? - I think you can probably avoid looking up all prev_events for events that are already accepted, too, which should make fast-path event insertion cheaper
Lines 1210-1219
@ -1208,2 +1207,13 @@
}
pub(super) async fn room_extremities(&self, room_id: OwnedRoomId) -> Result {
let mut extremities = self
.services
.rooms
.state
.get_forward_extremities(&room_id)
.broad_filter_map(|event_id| async move {
self.services.rooms.timeline.get_pdu(&event_id).await.ok()
})
.collect::<Vec<_>>()
.await;
Owner

Why not show <failed to load PDU> or something idk, seems like a good idea in admin commands rather than filtering. Might be a later thing

Why not show `<failed to load PDU>` or something idk, seems like a good idea in admin commands rather than filtering. Might be a later thing
Author
Owner

I originally did, but then sort_by_key became complicated so I figured it was a bit pointless. Plus, the migration drops any unaccepted/unknown PDUs, and now the only way to become an extremity is to become a non-outlier, so I don't really see this being a problem anymore

I originally did, but then `sort_by_key` became complicated so I figured it was a bit pointless. Plus, the migration drops any unaccepted/unknown PDUs, and now the only way to become an extremity is to become a non-outlier, so I don't really see this being a problem anymore
Owner

Fair

Fair
Jade marked this conversation as resolved
@ -980,0 +1048,4 @@
.for_each(|(_, pdu)| {
let room_id = &room_id;
async move {
if pdu.origin_server_ts < epoch_ts {
Owner

This seems like a bad idea, given this is untrusted data and gapfill exists.

This seems like a bad idea, given this is untrusted data *and* gapfill exists.
Author
Owner

It's trusted data - incoming PDUs before this epoch have to be backfilled, and we set that epoch when joining a room. It's not possible for an event before this timestamp to be an extremity as it will be backfilled

Using the timestamp at all for a Boundary like this is ass but 1. I don't see a better solution and 2. a better solution would be out of scope

It's trusted data - incoming PDUs before this epoch [have to be backfilled](https://forgejo.ellis.link/continuwuation/continuwuity/src/commit/08301fba2abd967525db051b0d23094d4d830216/src/service/rooms/event_handler/handle_incoming_pdu.rs#L139-L154), and [we set that epoch when joining a room](https://forgejo.ellis.link/continuwuation/continuwuity/src/commit/08301fba2abd967525db051b0d23094d4d830216/src/service/rooms/membership/mod.rs#L349-L356). It's not possible for an event before this timestamp to be an extremity as it will be backfilled Using the timestamp at all for a Boundary like this is ass but 1. I don't see a better solution and 2. a better solution would be out of scope
@ -431,0 +429,4 @@
for event_id in event_ids {
let key = (room_id, event_id);
self.db.roomid_pduleaves.put_raw(key, event_id);
Owner

You're writing different values in different places to this table

You're writing different values in different places to this table
@ -173,3 +142,1 @@
self.derive_new_state(incoming_pdu, room_version_rules, state_before, state_lock)
.await?;
}
pub async fn progress_extremities(&self, incoming_pdu: &PduEvent) -> Result<()> {
Owner

Why not call this inside append_pdu to avoid events?

Why not call this inside append_pdu to avoid events?
@ -215,0 +159,4 @@
// referenced extremities that we can now remove. Doing the walk
// from the prev events also means they will all automatically be
// removed from the extremity set.
let mut ancestors_todo = VecDeque::from_iter(incoming_pdu.prev_events.clone());
Owner

This might visit the same event multiple times if the DAG is, like, diamond shaped or something.

This might visit the same event multiple times if the DAG is, like, diamond shaped or something.
Author
Owner

Uhhh the only solution I can think to that is to track every event ID we walk? Or maybe

Uhhh the only solution I can think to that is to track every event ID we walk? Or maybe
@ -238,2 +188,3 @@
self.services.pdu_metadata.is_event_accepted(&prev_id),
);
(exists && !referenced && (is_outlier || is_soft_failed || is_rejected))
(exists && !matches!(count, PduCount::Backfilled(_)) && !is_accepted)
Owner

Surely this means accepted events can't get through this, so they're never removed when superseded by a soft-failed event?

Surely this means accepted events can't get through this, so they're never removed when superseded by a soft-failed event?
Author
Owner

Soft-failed events don't become extremities themselves, and consequently aren't supposed to be referenced either, but since soft-fails can differ between servers following a soft-fail/rejection chain can lead to accepted events that we did not mark as referenced because the ancestors were soft-failed, so this is intentional unless I'm misunderstanding?

Soft-failed events don't become extremities themselves, and consequently aren't supposed to be referenced either, but since soft-fails can differ between servers following a soft-fail/rejection chain can lead to accepted events that we did not mark as referenced because the ancestors were soft-failed, so this is intentional unless I'm misunderstanding?
nex force-pushed nex/feat/intuitive-extremity-tracking from c5dd81fc4c
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m7s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m15s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 20m51s
to 411114fe1e
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m10s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m14s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m8s
2026-10-16 19:03:22 +00:00
Compare
Author
Owner

(force-push was to drop the commits now included in #2371, it does not address any review comments)

(force-push was to drop the commits now included in #2371, it does not address any review comments)
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Required
Details
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m10s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Required
Details
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m14s
Required
Details
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m8s
Required
Details
This pull request has changes conflicting with the target branch.
  • src/service/rooms/membership/mod.rs
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin nex/feat/intuitive-extremity-tracking:nex/feat/intuitive-extremity-tracking
git switch nex/feat/intuitive-extremity-tracking
Sign in to join this conversation.
No reviewers
No milestone
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
continuwuation/continuwuity!2356
No description provided.