Backfill from nearby points past pagination token by MadLittleMods · Pull Request #19611 · element-hq/synapse

MadLittleMods · 2026-03-26T19:05:02Z

Backfill from nearby points past pagination token. The juicy details and explanation are in the diff itself.

Split out from #18873 in order to fix paginating from MSC3871 gap tokens actually backfilling history. To be clear, this is a good change to make outside of the MSC3871 use case. For example (as the new Complement test shows), fixes a problem where if you try to paginate /messages from tokens returned by /context, we could fail to backfill anything new and hide away history.

Also fixes matrix-org/complement#853

Dev notes

Room DAG concept docs: docs/development/room-dag-concepts.md

COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh --in-repo -run TestMessagesOverFederation

SYNAPSE_TEST_LOG_LEVEL=INFO poetry run trial tests.handlers.test_federation.FederationTestCase.test_backfill_with_many_backward_extremities

Pull Request Checklist

Pull request is based on the develop branch
Pull request includes a changelog file. The entry should:
- Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
- Use markdown where necessary, mostly for code blocks.
- End with either a period (.) or an exclamation mark (!).
- Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
Code style is correct (run the linters)

… token

… test comment

MadLittleMods · 2026-03-26T20:19:59Z

+	// TODO: Backfill test to make sure we backfill from forks when viewing history (see
+	// docstring above).
+	//
+	//  1. Alice (hs1, engineered homeserver) creates a room with events A, B
+	//  1. Bob (hs2) joins the room
+	//  1. Bob leaves the room
+	//  1. Alice creates a fork from A with some history (1, 2, 3) and connects it back with a new event C
+	//  1. Bob joins back
+	//  1. Bob paginates `/messages?dir=b&from=<token-after-b>`
+	//  1. Ensure Bob sees events: B, 2, 1, A
+	//
+	//   1 <--- 2 <----- 3
+	//  /                 \
+	// A <------- B ▲ <--- C <-- D
+	//              |
+	//              Paginate backwards from this point
+	// t.Run("Backfill from nearby backward extremities past token (fork)", func(t *testing.T) {


Something for a future PR

Only thing to check here is if the premise/plan makes sense

I believe this makes sense. Before the changes in this PR, we wouldn't pick up C as a backfill point (and thus miss events (2,1)), is that right?

Correct 👍 (whatever event that comes before bobs second join event)

MadLittleMods · 2026-03-26T20:21:03Z

+// These utilities match what we're using in the Complement repo (see
+// `matrix-org/complement` -> `tests/csapi/room_messages_test.go`)


No need to scrutinize the following utilities that hard. They already exist in Complement, see https://github.com/matrix-org/complement/blob/3adcab373fa98cad2b0a6979a8f11a679a6f3447/tests/csapi/room_messages_test.go#L485-L637

getStateID is "new" but also part of matrix-org/complement#801

Convenience-wise, it would be nice if we could import them but I'd rather duplicate than create a bad abstraction in Complement.

It does feel a little too test-specific to expose publicly.

Basically reverting back to how this test looked in matrix-org/synapse#11027 This test was updated in matrix-org/synapse#12191 without explanation. And it worked at this point because it didn't actually find any backfill points as no backfill points are <= 1 (`current_depth`).

MadLittleMods · 2026-03-26T22:51:02Z


        self.assertEqual(sg, sg2)

-    def test_backfill_with_many_backward_extremities(self) -> None:


Removed as ultimately, this test isn't useful in it's current form.

The reason it was looked at is because it started failing with this PR. Because it tries to backfill with current_depth = 1, previously, it found nothing and did nothing. Because this PR changes the behavior to look for backfill points <= 101 (current_depth + limit), we actually find backfill points and try to make an actual backfill request. For some reason this fails (.well-known DNS problems) and I don't understand why that's a problem. I would expect this kind of problem to just be caught by our try/except but it seems like it spawns some sort of DNS process that doesn't get cleaned up or something.

poetry run trial .... test failure

SYNAPSE_TEST_LOG_LEVEL=DEBUG poetry run trial tests.handlers.test_federation.FederationTestCase.test_backfill_with_many_backward_extremities tests.handlers.test_federation FederationTestCase test_backfill_with_many_backward_extremities ... [FAIL] [ERROR] [ERROR] =============================================================================== [FAIL] Traceback (most recent call last): File "/home/eric/Documents/github/element/synapse/tests/handlers/test_federation.py", line 325, in test_backfill_with_many_backward_extremities self.get_success( File "/home/eric/Documents/github/element/synapse/tests/unittest.py", line 742, in get_success return self.successResultOf(deferred) File "/home/eric/.cache/pypoetry/virtualenvs/matrix-synapse-xCtC9ulO-py3.14/lib/python3.14/site-packages/twisted/trial/_synctest.py", line 723, in successResultOf self.fail( twisted.trial.unittest.FailTest: Success result expected on <Deferred at 0x7f3469234450>, found no result instead tests.handlers.test_federation.FederationTestCase.test_backfill_with_many_backward_extremities =============================================================================== [ERROR] Traceback (most recent call last): Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean. DelayedCalls: (set twisted.internet.base.DelayedCall.debug = True to debug) <DelayedCall 0x7f34690afc20 [0.9999024868011475s] called=0 cancelled=0 DNSMixin._clearFailed(<Deferred at 0x7f34691d6450>, 27696)> <DelayedCall 0x7f34690fd4f0 [59.990769147872925s] called=0 cancelled=0 Resolver.maybeParseConfig()> tests.handlers.test_federation.FederationTestCase.test_backfill_with_many_backward_extremities =============================================================================== [ERROR] Traceback (most recent call last): Failure: twisted.trial.util.DirtyReactorAggregateError: Reactor was unclean. Selectables: <<class 'twisted.names.dns.DNSDatagramProtocol'> on 1562> tests.handlers.test_federation.FederationTestCase.test_backfill_with_many_backward_extremities ------------------------------------------------------------------------------- Ran 1 tests in 0.488s FAILED (failures=1, errors=2) Exception ignored while closing generator <coroutine object FederationHandler.maybe_backfill at 0x7f346941f340>: Traceback (most recent call last): File "/home/eric/Documents/github/element/synapse/synapse/logging/opentracing.py", line 949, in _wrapper return await func(*args, **kwargs) File "/home/eric/Documents/github/element/synapse/synapse/logging/opentracing.py", line 949, in _wrapper return await func(*args, **kwargs) RuntimeError: coroutine ignored GeneratorExit

Overall, we could keep this test if we figure out the DNS problem (I wouldn't waste time on it if you don't know off the top of your head).

If we really want to keep this kind of thing around, I did fix this test up in dc66977 by basically reverting it back to how it was introduced in matrix-org/synapse#11027. It does stress the backfill/extremity selection but since it doesn't actually make any backfill requests, I don't think it's actually useful and is why I decided to remove the test overall.

To document the history, this test was updated in matrix-org/synapse#12191 (without explanation). Given I was able to revert it back to how it was before, I don't see a need reason this was done. Perhaps they wanted to make it feel more "real".

Also as a note, this test was useful when it was first introduced in matrix-org/synapse#11027 and stressed the maybe_backfill code we cared about but we have since updated to be much smarter with backfill point selection (get_backfill_points_in_room which has its own tests).

I don't recognise the DirtyReactorAggregateError off the top of my head, so I'm going to trust your judgement on removing the test.

MadLittleMods · 2026-03-26T23:41:01Z

+		// after joining but this is valid as well. To illustrate a more real example of
+		// this, someone can use `/timestamp_to_event` to jump back in history and
+		// `/context` to start paginating history.


Added a real example for /timestamp_to_event in matrix-org/complement#853

MadLittleMods · 2026-03-26T23:41:43Z

+	// This test lives in our in-repo Complement tests for Synapse because the Matrix spec
+	// doesn't have any rules for how a homeserver should backfill. Practically speaking,
+	// homeservers that don't do anything for this problem will just hide messages from
+	// clients. This underscores the fact why it's necessary for homeservers indicate that
+	// there is a gap (using MSC3871) at the very least.


^ Explains why we have this test here vs in the Complement repo

…-past-token

See #19611 (comment)

…19630) Incorrectly labeled in matrix-org/synapse#13535. `maybe_backfill` already accurately describes `limit` (introduced in matrix-org/synapse#8349) Spotted in #19611 (comment)

…inating backwards despite the spec saying "A token that can be used to paginate forwards with." (#852) Relevant spec: https://spec.matrix.org/v1.17/client-server-api/#get_matrixclientv3roomsroomidcontexteventid There is no functional change to these tests. This is spawning from [using `/context` in another Complement test](element-hq/synapse#19611) and noticing this oddity.

Copilot

Pull request overview

Adjusts Synapse’s federation backfill heuristics so pagination from tokens near backward extremities (including /context-derived tokens and MSC3871 gap tokens) can still discover and backfill missing history. Adds an in-repo Complement test to cover the real-world /context → /messages pagination scenario.

Changes:

Pad the depth used to select backfill points (“nearby depth”) so extremities slightly after the pagination token can still be used to backfill into the visible window.
Introduce a constant for the /backfill request size and use it consistently in the handler.
Add a new in-repo Complement test exercising /context-token pagination over federation; remove an older Python regression test.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`synapse/storage/databases/main/event_federation.py`	Renames/clarifies the depth parameter (`nearby_depth`) and documents the padded-depth rationale for selecting backfill points.
`synapse/handlers/federation.py`	Pads the depth passed to `get_backfill_points_in_room` and uses a shared constant for `/backfill` size.
`tests/handlers/test_federation.py`	Removes `test_backfill_with_many_backward_extremities` and its now-unused imports.
`complement/tests/room_messages_test.go`	Adds a Complement test validating back-pagination from a `/context` token returns historical events over federation.
`changelog.d/19611.bugfix`	Adds a user-facing changelog entry for the backfill/pagination fix.

Comments suppressed due to low confidence (1)

synapse/storage/databases/main/event_federation.py:1362

The SQL orders by backward_extrem.event_id DESC, but the adjacent comment says the tie-break is “alphabetical order”. Either update the comment to match the descending order, or change the query to ASC if that’s what you intended.

                 * Sort from highest (closest to the `nearby_depth`) to the lowest depth
                 * because the closest are most relevant to backfill from first.
                 * Then tie-break on alphabetical order of the event_ids so we get a
                 * consistent ordering which is nice when asserting things in tests.
                 */
                ORDER BY event.depth DESC, backward_extrem.event_id DESC
                LIMIT ?

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

anoadragon453

So if I understand this correctly, the main change here is moving the depth threshold we use to look for backfill points.

Previously it was just current_depth, now it's current_depth + NUMBER_OF_EVENTS_TO_BACKFILL (100) (we look deeper/newer in the DAG). This helps with a class of edge case where a backfill point that exposes a long chain of events that would otherwise be in the visible window, could be entirely missed.

This doesn't fully fix the gaps problem that MSC3871 aims to solve (clients re-request backfilling of gaps from a certain token) but it is a useful improvement nonetheless.

anoadragon453 · 2026-04-29T10:51:12Z


        self.assertEqual(sg, sg2)

-    def test_backfill_with_many_backward_extremities(self) -> None:


I don't recognise the DirtyReactorAggregateError off the top of my head, so I'm going to trust your judgement on removing the test.

anoadragon453 · 2026-04-29T11:23:20Z

+	// TODO: Backfill test to make sure we backfill from forks when viewing history (see
+	// docstring above).
+	//
+	//  1. Alice (hs1, engineered homeserver) creates a room with events A, B
+	//  1. Bob (hs2) joins the room
+	//  1. Bob leaves the room
+	//  1. Alice creates a fork from A with some history (1, 2, 3) and connects it back with a new event C
+	//  1. Bob joins back
+	//  1. Bob paginates `/messages?dir=b&from=<token-after-b>`
+	//  1. Ensure Bob sees events: B, 2, 1, A
+	//
+	//   1 <--- 2 <----- 3
+	//  /                 \
+	// A <------- B ▲ <--- C <-- D
+	//              |
+	//              Paginate backwards from this point
+	// t.Run("Backfill from nearby backward extremities past token (fork)", func(t *testing.T) {


I believe this makes sense. Before the changes in this PR, we wouldn't pick up C as a backfill point (and thus miss events (2,1)), is that right?

anoadragon453 · 2026-04-29T11:24:27Z

+// These utilities match what we're using in the Complement repo (see
+// `matrix-org/complement` -> `tests/csapi/room_messages_test.go`)


It does feel a little too test-specific to expose publicly.

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…-past-token

See #19611 (comment)

…f github.com:element-hq/synapse into madlittlemods/backfill-from-nearby-points-past-token

MadLittleMods · 2026-04-29T17:41:18Z

@@ -0,0 +1 @@
+Fix Synapse not backfilling new history when attempting to use a pagination token near a backward extremity.


This doesn't fully fix the gaps problem that MSC3871 aims to solve (clients re-request backfilling of gaps from a certain token) but it is a useful improvement nonetheless.

-- @anoadragon453, #19611 (review)

While this is true, to clarify the actual interaction with MSC3871 here; the pagination tokens returned for the gaps are just before/after the backfill points so without this PR, it wouldn't try to backfill anything. (also mentioned in PR description)

Explained more here:

synapse/complement/tests/room_messages_test.go

Lines 69 to 83 in 2cd1f27

// Even with MSC3871 gaps, the tested behavior here is necessary as the gap prev/next

// tokens point before/after the event (remember: tokens are positions between

// events), so if you use `/messages?dir=b&from=<gap prev_pagination_token>`, we can't

// rely on naive depth comparison. MSC3871 Complement tests will also exercise this.

// Example:

//

// t0 t1 t2 t3 t4

// [A] <--- [B] <--- [C] <--- [bob join 4]

//

// When Bob calls `/messages?dir=b&backfill=false`, he sees a gap (`{ event_id: "bob

// join 4", prev_pagination_token: "t3", next_pagination_token: "t4" }`) and tries to

// fill it in with `/messages?dir=b&from=t3&limit=10&backfill=true`. To find backfill

// points, Synapse will compare `t3` with the backward extremity at an approximate

// depth of 4. Which is why we take `t3`, add the `limit=10` and then do the

// comparison (find any backfill points with an approximate depth <= 13).

anoadragon453

A well-thought out change with real impact. Nice work! And thanks for explaining it to myself (and others) in such detail.

MadLittleMods · 2026-05-01T16:42:36Z

Thanks for the review @anoadragon453 🦍

This isn't fixing any particular issue. It's just a follow-up I thought about after merging #19611 since we're now also dealing with backfill points in the nearby range ahead of the `current_depth`. And it's possible that the previous sort could bias to all nearby backfill points ahead of the `current_depth` that don't extend into the visible window of events we're paginating through.

…lement-hq#19630) Incorrectly labeled in matrix-org/synapse#13535. `maybe_backfill` already accurately describes `limit` (introduced in matrix-org/synapse#8349) Spotted in element-hq#19611 (comment)

The juicy details and explanation are in the diff itself. Split out from element-hq#18873 in order to fix paginating from [MSC3871](matrix-org/matrix-spec-proposals#3871) gap tokens actually backfilling history. To be clear, this is a good change to make outside of the [MSC3871](matrix-org/matrix-spec-proposals#3871) use case. For example (as the new Complement test shows), fixes a problem where if you try to paginate `/messages` from tokens returned by `/context`, we could fail to backfill anything new and hide away history. Also fixes matrix-org/complement#853

This isn't fixing any particular issue. It's just a follow-up I thought about after merging element-hq#19611 since we're now also dealing with backfill points in the nearby range ahead of the `current_depth`. And it's possible that the previous sort could bias to all nearby backfill points ahead of the `current_depth` that don't extend into the visible window of events we're paginating through.

MadLittleMods added the A-Messages-Endpoint label Mar 26, 2026

MadLittleMods changed the title ~~Backfill from nearby points past token~~ Backfill from nearby points past pagination token Mar 26, 2026

MadLittleMods force-pushed the madlittlemods/backfill-from-nearby-points-past-token branch 2 times, most recently from a09300d to 633f923 Compare March 26, 2026 20:18

MadLittleMods added 4 commits March 26, 2026 15:18

Add Complement test to backfill from nearby backward extremeties past…

2f2156c

… token

Splat "pad" fix from #18873

66bf5b0

Reword iteration based on the new language crafted for the Complement…

3cf58c5

… test comment

Add changelog

85f3bc2

MadLittleMods force-pushed the madlittlemods/backfill-from-nearby-points-past-token branch from 633f923 to 85f3bc2 Compare March 26, 2026 20:18

MadLittleMods commented Mar 26, 2026

View reviewed changes

MadLittleMods mentioned this pull request Mar 26, 2026

Implement MSC3871: Gappy timelines - indicate gaps in /messages #18873

Draft

8 tasks

MadLittleMods force-pushed the madlittlemods/backfill-from-nearby-points-past-token branch from 307d137 to 2be7684 Compare March 26, 2026 22:47

MadLittleMods commented Mar 26, 2026

View reviewed changes

Remove test

a40e4cc

MadLittleMods force-pushed the madlittlemods/backfill-from-nearby-points-past-token branch from 2be7684 to a40e4cc Compare March 26, 2026 22:51

MadLittleMods mentioned this pull request Mar 26, 2026

TestJumpToDateEndpoint: Add test for seamlessly paginating from /context start token matrix-org/complement#853

Merged

3 tasks

MadLittleMods commented Mar 26, 2026

View reviewed changes

No need to explain

b6c55d3

MadLittleMods mentioned this pull request Mar 26, 2026

TestJumpToDateEndpoint: Clarify why we use /context end for paginating backwards despite the spec saying "A token that can be used to paginate forwards with." matrix-org/complement#852

Merged

1 task

MadLittleMods added the A-Federation label Mar 27, 2026

MadLittleMods marked this pull request as ready for review March 27, 2026 00:54

MadLittleMods requested a review from a team as a code owner March 27, 2026 00:54

MadLittleMods commented Mar 30, 2026

View reviewed changes

Comment thread synapse/handlers/federation.py Outdated

MadLittleMods commented Mar 30, 2026

View reviewed changes

Comment thread synapse/handlers/federation.py Outdated

MadLittleMods mentioned this pull request Mar 30, 2026

Fix docstring for limit argument in _maybe_backfill_inner(...) #19630

Merged

3 tasks

MadLittleMods added 2 commits March 30, 2026 12:51

Merge branch 'develop' into madlittlemods/backfill-from-nearby-points…

b395090

…-past-token

Add NUMBER_OF_EVENTS_TO_BACKFILL

c37655a

See #19611 (comment)

Add docstring

98e1452

anoadragon453 requested a review from Copilot April 29, 2026 09:44

Copilot started reviewing on behalf of anoadragon453 April 29, 2026 09:44 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

anoadragon453 reviewed Apr 29, 2026

View reviewed changes

anoadragon453 and others added 4 commits April 29, 2026 11:30

Get the obvious typos out of the way

2cd1f27

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Merge branch 'develop' into madlittlemods/backfill-from-nearby-points…

3b1b4a8

…-past-token

r0 -> v3 API version

763da02

See #19611 (comment)

Merge branch 'madlittlemods/backfill-from-nearby-points-past-token' o…

3b39534

…f github.com:element-hq/synapse into madlittlemods/backfill-from-nearby-points-past-token

MadLittleMods commented Apr 29, 2026

View reviewed changes

MadLittleMods requested a review from anoadragon453 April 29, 2026 22:37

anoadragon453 approved these changes May 1, 2026

View reviewed changes

MadLittleMods mentioned this pull request May 1, 2026

Note how to paginate using token returned by /context matrix-org/matrix-spec#2372

Merged

3 tasks

MadLittleMods merged commit 6100f6e into develop May 1, 2026
110 of 117 checks passed

MadLittleMods deleted the madlittlemods/backfill-from-nearby-points-past-token branch May 1, 2026 16:42

MadLittleMods mentioned this pull request May 1, 2026

Prefer close backfill points (absolute distance) #19748

Merged

4 tasks

		// These utilities match what we're using in the Complement repo (see
		// `matrix-org/complement` -> `tests/csapi/room_messages_test.go`)


		self.assertEqual(sg, sg2)

		def test_backfill_with_many_backward_extremities(self) -> None:

		@@ -0,0 +1 @@
		Fix Synapse not backfilling new history when attempting to use a pagination token near a backward extremity.

	// Even with MSC3871 gaps, the tested behavior here is necessary as the gap prev/next
	// tokens point before/after the event (remember: tokens are positions between
	// events), so if you use `/messages?dir=b&from=<gap prev_pagination_token>`, we can't
	// rely on naive depth comparison. MSC3871 Complement tests will also exercise this.
	// Example:
	//
	// t0 t1 t2 t3 t4
	// [A] <--- [B] <--- [C] <--- [bob join 4]
	//
	// When Bob calls `/messages?dir=b&backfill=false`, he sees a gap (`{ event_id: "bob
	// join 4", prev_pagination_token: "t3", next_pagination_token: "t4" }`) and tries to
	// fill it in with `/messages?dir=b&from=t3&limit=10&backfill=true`. To find backfill
	// points, Synapse will compare `t3` with the backward extremity at an approximate
	// depth of 4. Which is why we take `t3`, add the `limit=10` and then do the
	// comparison (find any backfill points with an approximate depth <= 13).

Uh oh!

Conversation

MadLittleMods commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev notes

Pull Request Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MadLittleMods Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MadLittleMods Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MadLittleMods Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anoadragon453 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MadLittleMods commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MadLittleMods commented Mar 26, 2026 •

edited

Loading

MadLittleMods Mar 26, 2026 •

edited

Loading

MadLittleMods Mar 26, 2026 •

edited

Loading

MadLittleMods Apr 29, 2026 •

edited

Loading