feat: PDU handler refresh #1944

Merged
nex merged 16 commits from nex/fix/pdu-handler-refresh into main 2026-07-11 16:28:28 +00:00
Owner

This pull request layers a fresh coat of paint over the federation PDU handler. This is primarily intended to improve maintainability and reusability (monolithic functions have been broken down into several smaller, more easily digestible functions), but also fixes a few bugs along the way, such as ACLs being applied too aggressively.

This is part of a series of changes I'm planning to make to how PDU lifetimes are handled, from creation to persistence. To keep PR sizes reviewable, each "milestone" will get a new PR sequentially.

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:
This pull request layers a fresh coat of paint over the federation PDU handler. This is primarily intended to improve maintainability and reusability (monolithic functions have been broken down into several smaller, more easily digestible functions), but also fixes a few bugs along the way, such as ACLs being applied too aggressively. This is part of a series of changes I'm planning to make to how PDU lifetimes are handled, from creation to persistence. To keep PR sizes reviewable, each "milestone" will get a new PR sequentially. **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
* Ignores any events pushed without a room ID
* Removes needless clones for PDU size checking
* Removes incorrect ACL check in incoming handler
* Fast-path handling already handled outlier events
* Remove redundant same-room checks, replace with useful ones
* Combine event rejection and persistence in a single function
* Additional safety assertions in upgrade task
* Split upgrade task into multiple subroutines for reduced cognitive complexity
* Only mutate current state and forward extremities in tandem
* Improve code documentation to better explain the logic flow
perf: Change signature of build_local_dag to remove refmap hacks
All checks were successful
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m24s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m11s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 11m7s
beed83855f
nex changed title from feat: PDU handler refersh to WIP: feat: PDU handler refersh 2026-07-11 00:38:10 +00:00
Author
Owner

Marking as WIP until this has been tested

Marking as WIP until this has been tested
nex changed title from WIP: feat: PDU handler refersh to WIP: feat: PDU handler refresh 2026-07-11 01:01:19 +00:00
nex added this to the (deleted) milestone 2026-07-11 01:02:40 +00:00
Tracking down weird soft-fails
fix: Invert boolean returned by policy server check
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m3s
Checks / Prek / Check changed files (pull_request) Successful in 4s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 54s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m54s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 5s
8688a66df7
nex changed title from WIP: feat: PDU handler refresh to feat: PDU handler refresh 2026-07-11 01:39:15 +00:00
ginger requested changes 2026-07-11 02:24:35 +00:00
Dismissed
@ -87,9 +87,7 @@ impl super::Service {
let to_persist = if mapped.len() <= 1 {
mapped.keys().map(ToOwned::to_owned).collect()
Owner

seems to me like this check for an empty/one-item map should be part of build_local_dag

seems to me like this check for an empty/one-item map should be part of build_local_dag
nex marked this conversation as resolved
@ -128,2 +68,2 @@
if !meta_exists {
if !room_exists && !is_interesting_member_event {
if is_interesting_member_event {
Owner

what's going on with the logic here? this if can only run if is_interesting_member_event && !is_interesting_member_event

what's going on with the logic here? this `if` can only run if `is_interesting_member_event && !is_interesting_member_event`
Author
Owner

that is a good question

that is a good question
nex marked this conversation as resolved
@ -155,2 +98,3 @@
// 8. if not timeline event: stop
// If this is not a timeline event, stop now, as we don't want to de-outlier it.
if !is_timeline_event {
debug!("Not promoting incoming event as it is not a timeline event");
Owner

"timeline event" is a really unhelpful name for this, that parameter would better be called is_backfilled_event (which would also invert its meaning). honestly the fact that this early return exists at all suggests that this function should be split into two functions

"timeline event" is a really unhelpful name for this, that parameter would better be called `is_backfilled_event` (which would also invert its meaning). honestly the fact that this early return exists at all suggests that this function should be split into two functions
Author
Owner

That would make more sense actually, it looks like the only time this condition is met is in backfill.rs

That would make more sense actually, it looks like the only time this condition is met is in backfill.rs
nex marked this conversation as resolved
@ -171,3 +116,2 @@
// 9. Fetch any missing prev events doing all checks listed here starting at 1.
// These are timeline events
// Fetch any missing prev events doing all checks listed here starting at 1.
Owner

those checks are gone from the docstring so this comment is confusing now

those checks are gone from the docstring so this comment is confusing now
nex marked this conversation as resolved
@ -33,2 +47,3 @@
if !Self::pdu_format_check_1(&value) {
warn!(
"dropping incoming PDU {event_id} in room {room_id} from {origin} because it \
"Dropping incoming PDU {event_id} in room {room_id} from {origin} because it \
Owner

this warn should say "does not follow room version rules"

this warn should say "does not follow room version rules"
nex marked this conversation as resolved
@ -382,2 +216,3 @@
if soft_fail {
if should_soft_fail {
self.services
Owner

This can probably be done in append_incoming_pdu, which returns early if the event is soft failed

This can probably be done in `append_incoming_pdu`, which returns early if the event is soft failed
nex marked this conversation as resolved
ginger modified the milestone from (deleted) to 26.6.1 2026-07-11 02:27:17 +00:00
fix: Address review feedback
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 1m21s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m10s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m52s
eb9a478f00
style: Accept event id reference in pdu_format_check_1
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 1m1s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m18s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m46s
4f7436a58f
ginger requested changes 2026-07-11 04:43:21 +00:00
Dismissed
@ -297,11 +297,7 @@ async fn handle_room(
// failure (e.g., cycles). This is best-effort; proper ordering is the sender's
// responsibility.
let sorted_event_ids = if pdu_map.len() >= 2 {
Owner

this if should no longer be needed

this if should no longer be needed
nex marked this conversation as resolved
@ -385,3 +173,2 @@
.mark_event_soft_failed(incoming_pdu.event_id());
info!(
if should_soft_fail {
Owner

could this log be moved into append_incoming_pdu?

could this log be moved into `append_incoming_pdu`?
Author
Owner

It could, but then there'd be eight parameters on the function, one of those just being an Instant. I don't think it's worth it, there's already too many params

It *could*, but then there'd be eight parameters on the function, one of those just being an `Instant`. I don't think it's worth it, there's already too many params
ginger marked this conversation as resolved
style: Remove redundant length check for dag build
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 1m6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m0s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m47s
985b45d038
nex requested review from Owners 2026-07-11 07:46:42 +00:00
ginger approved these changes 2026-07-11 15:05:01 +00:00
nex force-pushed nex/fix/pdu-handler-refresh from 985b45d038
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 1m6s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m0s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m47s
to 9d2ce72f73
Some checks failed
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 1m5s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m10s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m0s
Checks / Prek / Check changed files (push) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (push) Has been cancelled
Release Docker Image / Build linux-amd64 (release) (push) Has been cancelled
Release Docker Image / Build linux-arm64 (release) (push) Has been cancelled
Release Docker Image / Create Multi-arch Release Manifest (push) Has been cancelled
Release Docker Image / Build linux-amd64 (max-perf) (push) Has been cancelled
Release Docker Image / Build linux-arm64 (max-perf) (push) Has been cancelled
Release Docker Image / Create Max-Perf Manifest (push) Has been cancelled
Release Docker Image / Release Binaries (push) Has been cancelled
Release Docker Image / Mirror Images (push) Has been cancelled
Documentation / Build and Deploy Documentation (push) Has been cancelled
Checks / Prek / Pre-commit & Formatting (push) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m3s
2026-07-11 16:27:46 +00:00
Compare
nex merged commit 9d2ce72f73 into main 2026-07-11 16:28:28 +00:00
nex deleted branch nex/fix/pdu-handler-refresh 2026-07-11 16:28:28 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 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!1944
No description provided.