perf: Improve gapfill logic #1818

Merged
Jade merged 82 commits from nex/perf/get-missing-events into main 2026-07-01 18:45:08 +00:00
Owner

Right now our gapfilling code is pretty awful for performance: for each event we are missing, we make an individual GET /_matrix/federation/v1/event/$eventID call, which is incredibly slow. This also happens for fetching auth events we are missing. Luckily, Matrix defines two bulk-fetch endpoints that help us. So, this PR updates our gapfilling code to be more reliable and effective.

This pull request also tries to address #1844 by sending dummy events when the number of forward extremities exceeds a certain threshold (default 10), which should help reduce the number of prev events being referenced in future events (and also passively heals the room).

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:
Right now our gapfilling code is pretty awful for performance: for each event we are missing, we make an individual `GET /_matrix/federation/v1/event/$eventID` call, which is *incredibly* slow. This *also* happens for fetching auth events we are missing. Luckily, Matrix defines two bulk-fetch endpoints that help us. So, this PR updates our gapfilling code to be more reliable and effective. This pull request also tries to address #1844 by sending dummy events when the number of forward extremities exceeds a certain threshold (default 10), which should help reduce the number of prev events being referenced in future events (and also passively heals the room). **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
nex self-assigned this 2026-05-26 19:16:29 +00:00
perf: Make max gap depth fetch configurable
Some checks failed
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 4s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m33s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m36s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 8s
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
a1eeef95e0
nex force-pushed nex/perf/get-missing-events from a1eeef95e0
Some checks failed
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 4s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m33s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m36s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 8s
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to d15064871e
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 33s
Checks / Prek / Check changed files (pull_request) Successful in 10s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m58s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
2026-05-26 19:22:51 +00:00
Compare
nex force-pushed nex/perf/get-missing-events from c8e1c657ae
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Documentation / Build and Deploy Documentation (pull_request) Successful in 59s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m28s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m35s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m3s
to b7e0d96b91
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Update flake hashes / update-flake-hashes (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
2026-05-31 17:51:53 +00:00
Compare
nex force-pushed nex/perf/get-missing-events from b7e0d96b91
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Update flake hashes / update-flake-hashes (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to c8e1c657ae
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Documentation / Build and Deploy Documentation (pull_request) Successful in 59s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m28s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m35s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m3s
2026-05-31 17:52:52 +00:00
Compare
Author
Owner

So this PR yields unparalleled speed and consistency improvements over the base branch, even in this unoptimised state, however it highlights a problem. Filling gaps can take an extraordinarily long time, especially if servers pushing events to us are unreachable (e.g. ipv6 only server pushes to a server without ipv6 egress), or the room graph is really complex and actively moving (e.g. the ping room). As a result, the median PDU handle time with this PR can easily go from ~4ms to >500ms, with a few minutes being the worst-case observed thus far, since we now try significantly more aggressively to pull in events that we missed. The best-case remains unchanged, and generally speaking consistency is improved too, but in the future, processing missing events and upgrading outlier PDUs to timeline PDUs will probably need to be cast into a background task, as right now the entire room becomes locked while prev events are being fetched, which as stated, can take minutes. Not sure if that'll get squeezed into this PR's scope or whether it'll end up being a separate job.

So this PR yields unparalleled speed and consistency improvements over the base branch, even in this unoptimised state, however it highlights a problem. Filling gaps can take an extraordinarily long time, especially if servers pushing events to us are unreachable (e.g. ipv6 only server pushes to a server without ipv6 egress), or the room graph is really complex and actively moving (e.g. the ping room). As a result, the median PDU handle time with this PR can easily go from ~4ms to >500ms, with a few minutes being the worst-case observed thus far, since we now try significantly more aggressively to pull in events that we missed. The best-case remains unchanged, and generally speaking consistency is improved too, but in the future, processing missing events and upgrading outlier PDUs to timeline PDUs will probably need to be cast into a background task, as right now the entire room becomes locked while prev events are being fetched, which as stated, can take minutes. Not sure if that'll get squeezed into this PR's scope or whether it'll end up being a separate job.
nex force-pushed nex/perf/get-missing-events from c8e1c657ae
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Documentation / Build and Deploy Documentation (pull_request) Successful in 59s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m28s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m35s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m3s
to f19daa2365
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Check changed files (pull_request) Successful in 4s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m25s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m28s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 11m0s
2026-06-07 00:56:05 +00:00
Compare
style: Tidy up
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m24s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m27s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 10m58s
cafb269a6a
Author
Owner

Background management is looking to be massively out of scope for this pull request, so I think it's ready for review. Had this deployed to both unstable.timedout.uk:8448 and explodie.org (thanks Aranje) for some time now and it looks like all of the quirks, bar the aforementioned overzealous mutex lock, have been ironed out. It looks like tuwunel stole our thunder on this one, so I'll go cross-reference with their implementation to see if there's any ideas I can lift, but otherwise this just needs a touch up, and a round of optimisation, and then it's good to go.

Preliminary review greatly appreciated.

Background management is looking to be massively out of scope for this pull request, so I think it's ready for review. Had this deployed to both `unstable.timedout.uk:8448` and `explodie.org` (thanks Aranje) for some time now and it looks like all of the quirks, bar the aforementioned overzealous mutex lock, have been ironed out. It looks like tuwunel stole our thunder on this one, so I'll go cross-reference with their implementation to see if there's any ideas I can lift, but otherwise this just needs a touch up, and a round of optimisation, and then it's good to go. Preliminary review greatly appreciated.
nex force-pushed nex/perf/get-missing-events from cafb269a6a
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m24s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m27s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 10m58s
to 26a4fd5f0f
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 1m5s
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m28s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m9s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m48s
2026-06-15 15:39:51 +00:00
Compare
feat: Automatically squash extremities when they exceed a threshold
All checks were successful
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m11s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m12s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m5s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m35s
13b3023acb
Attempts to tackle #1844
fix: un-forget how streams work
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 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m7s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m12s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m5s
b394a5595d
nex changed title from WIP: perf: Improve gapfill logic to perf: Improve gapfill logic 2026-06-16 17:37:35 +00:00
nex requested review from Owners 2026-06-16 17:37:43 +00:00
@ -102,0 +160,4 @@
// that many events per-request. Synapse returns 20, and conduwuit+ return 50.
// This means with a hard iteration limit, we might give up too early, before
// we get a chance to even come close to max_fetch_prev_events. As such, we'll
// calculate the max limit based on that config option based on these averages.
Owner

this comment seems to be mangled

this comment seems to be mangled
nex marked this conversation as resolved
@ -209,0 +350,4 @@
Ok(discovered)
}
async fn fetch_and_handle_missing_event_via(
Owner

NO DOCSTRING?

NO DOCSTRING?
nex marked this conversation as resolved
@ -110,0 +88,4 @@
for (i, event_id) in to_persist.iter().enumerate() {
debug!(
elapsed=?start.elapsed(),
"[TODO] Persisting fetched prev event: {event_id} ({}/{})",
Owner

"TODO"?

"TODO"?
Author
Owner

ugh knew I missed one of these

ugh knew I missed one of these
nex marked this conversation as resolved
@ -55,0 +28,4 @@
/// them directly.
///
/// The end result is a result containing a map of shortstatekeys to event
/// IDs. The underlying `Option` is always `Some`.
Owner

why return an option then 🗿

why return an option then 🗿
Author
Owner

because that's the type the caller wants 🗿

because that's the type the caller wants 🗿
Owner

change the caller then 🗿

change the caller then 🗿
Author
Owner

unlike someone I want my diff to be a reviewable size 🗿

Context:

debug!(
event_id = %incoming_pdu.event_id,
"Resolving state at event"
);
let mut state_at_incoming_event = if incoming_pdu.prev_events().count() == 1 {
self.state_at_incoming_degree_one(&incoming_pdu).await?
} else {
self.state_at_incoming_resolved(&incoming_pdu, room_id, &room_version_rules)
.await?
};
if state_at_incoming_event.is_none() {
state_at_incoming_event = self
.fetch_state(origin, create_event, room_id, incoming_pdu.event_id())
.await?;
}
let state_at_incoming_event =
state_at_incoming_event.expect("we always set this to some above");

unlike someone I want my diff to be a reviewable size 🗿 Context: https://forgejo.ellis.link/continuwuation/continuwuity/src/commit/0932c929c3d7483bc4f861b9b4fcd8e420bb4163/src/service/rooms/event_handler/upgrade_outlier_pdu.rs#L72-L90
Owner

change line 84 to Some(self.fetch_state(...)) 🗿

change line 84 to `Some(self.fetch_state(...))` 🗿
Owner

also that logic is sketchy in general, it could be a single chain of ifs instead of an if -> assignment -> expect() call

also that logic is sketchy in general, it could be a single chain of ifs instead of an if -> assignment -> expect() call
Author
Owner

womp womp I'll have a play about

womp womp I'll have a play about
nex marked this conversation as resolved
@ -86,0 +282,4 @@
/// state events as outliers, for use later.
///
/// Any events that cannot be persisted are dropped with a warning.
/// TODO: make it noisy?
Owner

another TODO

another TODO
nex marked this conversation as resolved
@ -149,0 +164,4 @@
)));
}
}
// TODO: do events received from auth chain need persisting? that sounds
Owner

another TODO

another TODO
nex marked this conversation as resolved
@ -395,0 +397,4 @@
// sometimes end up sending PDUs to us that we aren't yet ready to accept, and
// consequently drop. Holding the mutex over the room while processing mitigates
// this.
let _room_lock = self
Owner

does this lock need to be held until the end of the function?

does this lock need to be held until the end of the function?
Author
Owner

It needs to be held until we've set the state and forward extremities, which is at the end of the function, so yeah. it could probably be dropped before extremity setting but it's practically the end of the function anyway

It needs to be held until we've set the state and forward extremities, which is at the end of the function, so yeah. it could probably be dropped before extremity setting but it's practically the end of the function anyway
ginger marked this conversation as resolved
fix: Default PDU content to empty object instead of literal NULL
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m25s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m17s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m22s
64d1f6dc66
style: Check power levels before attempting to send extremity squashes
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m8s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m11s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m15s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m37s
a89c80a489
Solves a problem where the console screams in agony when local users can't send dummy events
fix: Prevent arbitrary state injection attack
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m9s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m12s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m9s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
4e2f36f7f1
nex force-pushed nex/perf/get-missing-events from 4e2f36f7f1
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m9s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m12s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m9s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to f2aca3e5b1
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m8s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m12s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m18s
2026-06-17 15:20:12 +00:00
Compare
nex requested reviews from ginger, Owners 2026-06-17 15:21:29 +00:00
@ -16,3 +27,2 @@
/// Find the event and auth it. Once the event is validated (steps 1 - 8)
/// it is appended to the outliers Tree.
const GET_MISSING_EVENTS_MAX_BATCH_SIZE: u16 = 50; // matches src/server/get_missing_events.rs#LIMIT_MAX
Owner

why not use the same constant, then?

why not use the same constant, then?
nex marked this conversation as resolved
@ -71,0 +78,4 @@
.copied()
.unwrap_or_else(|| int!(0))
.to_string()
.parse::<u64>()
Owner

🤨 what's this string roundtrip for

🤨 what's this string roundtrip for
Author
Owner

without looking at the source this is commenting on, ruma::UInt doesn't implement From<ruma::Int>, or some other equally absurd From/Into trait isn't satisfied that allows converting from Int to UInt. Why MilliSecondsSinceUnixEpoch is even a UInt under the hood is beyond me, because unix timestamps can be negative. But whatever.

without looking at the source this is commenting on, `ruma::UInt` doesn't implement `From<ruma::Int>`, or some other equally absurd `From`/`Into` trait isn't satisfied that allows converting from Int to UInt. Why `MilliSecondsSinceUnixEpoch` is even a `UInt` under the hood is beyond me, because unix timestamps can be negative. But whatever.
Owner

I'd suggest converting the Int into a u64 and calling new_saturating

I'd suggest converting the `Int` into a `u64` and calling [`new_saturating`](https://docs.rs/js_int/0.2.2/js_int/struct.UInt.html#method.new_saturating)
nex marked this conversation as resolved
@ -293,0 +248,4 @@
debug!("Fetching and persisting any missing prev events");
self.fetch_prevs(room_id, create_event, &incoming_pdu, origin)
.await
.debug_inspect_err(|e| {
Owner

why's this debug_inspect_err and not inspect_err?

why's this `debug_inspect_err` and not `inspect_err`?
Author
Owner

Because it's usually a transient failure (we were unable to ask the origin for missing events) and the fetch will be re-tried by a later PDU handle. That information usually isn't important to operation and there's not much that can be done about it as a user, but it may serve to be useful in a debug scenario.
TL;DR the log isn't useful for non-debugging ops

Because it's usually a transient failure (we were unable to ask the origin for missing events) and the fetch will be re-tried by a later PDU handle. That information usually isn't important to operation and there's not much that can be done about it as a user, but it may serve to be useful in a debug scenario. TL;DR the log isn't useful for non-debugging ops
ginger marked this conversation as resolved
@ -298,0 +278,4 @@
.await;
let mut local_users = self.services.state_cache.local_users_in_room(room_id);
while let Some(user_id) = local_users.next().await {
if power_levels.for_user(&user_id)
Owner

this could be a call to user_can_send_message

this could be a call to [`user_can_send_message`](https://docs.rs/ruma/latest/ruma/events/room/power_levels/struct.RoomPowerLevels.html#method.user_can_send_message)
nex marked this conversation as resolved
@ -298,0 +298,4 @@
&state_lock,
)
.await
.inspect(|_| debug!(sender=%user_id, "Successfully sent a dummy event"))
Owner

nit: this log line could go above the break

nit: this log line could go above the `break`
Author
Owner

I don't really see a reason to

I don't really see a reason to
ginger marked this conversation as resolved
@ -23,3 +25,3 @@
room_id: &'a RoomId,
mut value: CanonicalJsonObject,
auth_events_known: bool,
_auth_events_known: bool,
Owner

why's there an unused parameter here?

why's there an unused parameter here?
Author
Owner

it used to be used, I never checked how many callsites I'd need to refactor to remove it entirely

it used to be used, I never checked how many callsites I'd need to refactor to remove it entirely
Author
Owner

removed

removed
nex marked this conversation as resolved
@ -133,0 +151,4 @@
.map_err(|e| err!(Request(BadJson("Invalid PDU {auth_event_id}: {e}"))))?;
auth_chain_map.insert(auth_event_id, auth_pdu);
}
for aid in pdu_event.auth_events() {
Owner

nit: we aren't rationing letters, this would be more clear as auth_event_id

nit: we aren't rationing letters, this would be more clear as `auth_event_id`
nex marked this conversation as resolved
@ -298,0 +264,4 @@
.count()
.await;
if extremities_count >= self.services.server.config.dummy_event_threshold.into() {
Owner

Should probably throttle such that it only sends after a minute, at most once a minute, as long as there are still extremities

Should probably throttle such that it only sends after a minute, at most once a minute, as long as there are still extremities
nex marked this conversation as resolved
@ -236,3 +231,3 @@
}
// Skip old events
// Skip events sent before we joined (they need to be persisted as backfilled
Owner

I get the feeling that this would be the correct place to use depth if it was actually trustable, as if it is less than the join, the event will always have happened before or at "the same time as the join". IIRC there isn't a way to make depth go down. Although given this is existing code, might be OK to leave it for now

I get the feeling that this would be the correct place to use depth if it was actually trustable, as if it is less than the join, the event will always have happened before or at "the same time as the join". IIRC there isn't a way to make depth go down. Although given this is existing code, might be OK to leave it for now
Author
Owner

Yeah, I did initially remove it, but it caused problems where we would then start upgrading events that were backfilled. I never bothered investigating why we even tried to do that, but it seemed like a load-bearing awful check, so I've retained it for now. Probably worth looking into it in the future though. As you mentioned, depth can't be trusted, and comparing the DAG position lexicographically is incredibly expensive

Yeah, I did initially remove it, but it caused problems where we would then start upgrading events that were backfilled. I never bothered investigating *why* we even tried to do that, but it seemed like a load-bearing awful check, so I've retained it for now. Probably worth looking into it in the future though. As you mentioned, depth can't be trusted, and comparing the DAG position lexicographically is incredibly expensive
nex marked this conversation as resolved
fix: Fall back to legacy behaviour when prev events are missed from get_missing_events
All checks were successful
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m13s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m3s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 10m12s
c340b26fd1
perf: Don't try to fetch prevs we already fetched
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 1m9s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m26s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m17s
b422c3aa6c
graphs are hard
Also fixes a couple bugs where events were being misattributed
perf: Use a hashmap for full-state filtering
Some checks failed
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m11s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m12s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
45861cb06d
chore: Drop unused param from handle_outlier_pdu
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m12s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m25s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m32s
5337293d05
Jade approved these changes 2026-06-18 23:02:59 +00:00
Jade left a comment

With that fallback for the forked depth case, I think this should be good. Testing on my instance now, anyway.

With that fallback for the forked depth case, I think this should be good. Testing on my instance now, anyway.
nex force-pushed nex/perf/get-missing-events from 5337293d05
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m12s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m25s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m32s
to 7ca00e4ab9
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 1m0s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m13s
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m8s
2026-06-20 16:08:07 +00:00
Compare
perf: Throttle dummy events to prevent stampeding
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 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m5s
Update flake hashes / update-flake-hashes (pull_request) Successful in 54s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m4s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 11m40s
3b6858e936
@ -298,0 +280,4 @@
let squash_timings = self.last_extremity_squash.read();
squash_timings.get(room_id).copied()
};
if last_squash.is_some_and(|s| s.elapsed() < Duration::from_mins(1)) {
Owner

Here you're throttling, I'd kind of prefer to debounce avoid situations where a later event deals with extremities itself. I'd also like to skip if the previous even is itself a dummy event, to fully prevent loops rather than just throttling them.

Here you're throttling, I'd kind of prefer to [debounce](https://css-tricks.com/debouncing-throttling-explained-examples/) avoid situations where a later event deals with extremities itself. I'd also like to skip if the previous even is itself a dummy event, to fully prevent loops rather than just throttling them.
nex marked this conversation as resolved
@ -56,6 +56,7 @@ impl crate::Service for Service {
Ok(Arc::new(Self {
mutex_federation: RoomMutexMap::new(),
federation_handletime: HandleTimeMap::new().into(),
last_extremity_squash: SyncRwLock::new(HashMap::new()),
Owner

This is technically the correct lock for the way you're using it, although in this case it's not so much of a difference from a mutex. additionally, you've technically left a race condition in between releasing the read and acquiring the write lock. you would have to use double-checked locking to take advantage of a rwlock safely in this case.

This is technically the correct lock for the way you're using it, although in this case it's not so much of a difference from a mutex. additionally, you've technically left a race condition in between releasing the read and acquiring the write lock. you would have to use double-checked locking to take advantage of a rwlock safely in this case.
Owner

double checked locking being:

  • acquire read
  • check
  • release read
  • acquire upgradable read
  • check
  • do work
  • upgrade to write
  • upgrade
  • release write
double checked locking being: - acquire read - check - release read - acquire upgradable read - check - do work - upgrade to write - upgrade - release write
Owner

Also a concurrent map would be the ideal tool over a coarse lock, but idrc that much

Also a concurrent map would be the ideal tool over a coarse lock, but idrc that much
nex marked this conversation as resolved
Jade force-pushed nex/perf/get-missing-events from 3b6858e936
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 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m5s
Update flake hashes / update-flake-hashes (pull_request) Successful in 54s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m4s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 11m40s
to 729cbbd7da
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 1m34s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 12m42s
2026-06-27 00:27:41 +00:00
Compare
Jade force-pushed nex/perf/get-missing-events from 729cbbd7da
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 1m34s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 12m42s
to bdd8ad1413
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 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m23s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m3s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m19s
2026-06-27 19:40:57 +00:00
Compare
feat: Debounce extremity squashing
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 4s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m11s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m14s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m25s
3649f76045
Additionaly circuit-breaks it if the squash would have only
b triggered by dummy events / other squashes
@ -312,0 +338,4 @@
let mut closing = false;
let waker = tokio::time::sleep(Duration::from_mins(2));
Author
Owner

every 2 minutes is really lax, a room with extremities that need squashing is probably a really active room (like the ping room). I'd say 1 minute at most

every 2 minutes is really lax, a room with extremities that need squashing is probably a really active room (like the ping room). I'd say 1 minute at most
@ -92,2 +95,4 @@
fn interrupt(&self) { self.server_shutdown.notify_waiters(); }
async fn clear_cache(&self) {}
Author
Owner

this should just be removed if it's going to be a no-op

this should just be removed if it's going to be a no-op
nex marked this conversation as resolved
Fix: Correct bad merge
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Has been cancelled
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Check changed files (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
f9a43abb7f
I honestly have no idea what happened here
Jade force-pushed nex/perf/get-missing-events from f9a43abb7f
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Has been cancelled
Documentation / Build and Deploy Documentation (pull_request) Has been cancelled
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Check changed files (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to b6bc7dfc16
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m12s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 1m11s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m15s
2026-06-28 00:38:12 +00:00
Compare
feat: Allow sending dummy events to clients
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 1m11s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m12s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m41s
5c48b99f47
Jade force-pushed nex/perf/get-missing-events from 5c48b99f47
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 1m11s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m12s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m41s
to 5199cde870
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 5s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m13s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m55s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m33s
2026-06-28 02:45:15 +00:00
Compare
Owner

Last thing for me to do here if I have time is add some random jitter to the debounce

Last thing for me to do here if I have time is add some random jitter to the debounce
Author
Owner

I'll do it now along with resolving remaining review comments

I'll do it now along with resolving remaining review comments
Previously the function assumed the caller had performed proper validation on the inputs (and all current callers do), but this is a poor reason to panic when sane error handling is available.
Events with no prev events now return an error, and prev events which are illegal are simply skipped.
feat: Add jitter to extremity squasher
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m11s
Documentation / Build and Deploy Documentation (pull_request) Successful in 2m7s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
2bfd678c71
style: Add docstrings to squasher functions
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m0s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m4s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m19s
19a83c6891
style: De-duplicate build_local_dag definitions, replace outdated comment on unwrap safety
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) Successful in 1m11s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m16s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
9f468edb01
fix: Don't create fake objects when defaulting a partial PDU build
Some checks failed
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 10s
Checks / Prek / Check changed files (pull_request) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m3s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m9s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 4m24s
d2f563a340
feat: Prioritise sending dummy events when the extremity count reaches 20 or more
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
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m14s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m15s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 4m44s
cd931ca7e2
fix: Build errors
All checks were successful
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) Successful in 1m14s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m32s
c16de5570b
Clippy wasn't running on all crates, oops
Jade force-pushed nex/perf/get-missing-events from c16de5570b
All checks were successful
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) Successful in 1m14s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m32s
to 62e0b53f52
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 4s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m24s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m29s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m1s
2026-07-01 18:16:19 +00:00
Compare
Jade scheduled this pull request to auto merge when all checks succeed 2026-07-01 18:20:29 +00:00
Jade merged commit eee4ef50d2 into main 2026-07-01 18:45:08 +00:00
Jade deleted branch nex/perf/get-missing-events 2026-07-01 18:45:09 +00:00
ginger added this to the 26.6.0 milestone 2026-07-04 15:58:10 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
continuwuation/continuwuity!1818
No description provided.