Fix state not showing up fast enough with TestDelayedEvents#865
Conversation
Follow-up to #830 As experienced in https://github.com/element-hq/synapse-rust-apps/actions/runs/24910122124/job/72949760158?pr=360 (element-hq/synapse-rust-apps#360) ``` ❌ TestDelayedEvents/delayed_state_events_are_kept_on_server_restart (10.12s) delayed_event_test.go:425: StopServer hs1 delayed_event_test.go:429: StartServer hs1 delayed_event_test.go:443: CSAPI.MustDo GET http://127.0.0.1:32978/_matrix/client/v3/rooms/%21MbDncghrqxTzEmQhCP:hs1/state/com.example.test/1 returned non-2xx code: 404 Not Found - body: {"errcode":"M_NOT_FOUND","error":"Event not found."} ```
| @@ -371,6 +371,11 @@ func TestDelayedEvents(t *testing.T) { | |||
|
|
|||
There was a problem hiding this comment.
As experienced when running this test against the worker-based Synapse setup we use alongside the Synapse Pro Rust apps, https://github.com/element-hq/synapse-rust-apps/actions/runs/24910122124/job/72949760158?pr=360 (https://github.com/element-hq/synapse-rust-apps/pull/360)
Error encountered:
❌ TestDelayedEvents/delayed_state_events_are_kept_on_server_restart (10.12s)
delayed_event_test.go:425: StopServer hs1
delayed_event_test.go:429: StartServer hs1
delayed_event_test.go:443: CSAPI.MustDo GET http://127.0.0.1:32978/_matrix/client/v3/rooms/%21MbDncghrqxTzEmQhCP:hs1/state/com.example.test/1 returned non-2xx code: 404 Not Found - body: {"errcode":"M_NOT_FOUND","error":"Event not found."}
I haven't actually checked whether this PR fixes the problem there (just theory)
There was a problem hiding this comment.
Why does this happen?
I guess this happens because the worker that processes delayed events and updates the rooms state, isn't necessarily the one that serves state requests. Is this even true?
It looks like the main process in Synapse handles processing delayed events.
And it looks like /state requests can be handled by workers. Although, the regex there is slightly strange as ^/_matrix/client/(api/v1|r0|v3|unstable)/rooms/.*/state$ doesn't cover /state/{eventType}/{stateKey} requests (only /state). ^/_matrix/client/(api/v1|r0|v3|unstable)/rooms/.*/state/ is also there but listed as "event sending requests" probably because the GET vs PUT is the same path. In the actual worker config we are using in the workerized Synapse image for Complement, /state/{eventType}/{stateKey} isn't covered by any workers.
So I guess both are processed on the Synapse main process and this shouldn't be a problem? Perhaps this is a problem with Synapse itself 🤔
It shouldn't have anything to do with running with the Synapse rust apps as those currently only cover state federation servlets (/_matrix/federation/v1/state_ids/{roomId, /_matrix/federation/v1/state/{roomId}, /_matrix/federation/v1/event/{eventId}) which isn't the client API.
There was a problem hiding this comment.
It looks like the main process in Synapse handles processing delayed events.
Even if so, the state will have to get persisted on the correct event_persister for the room and the main process might be serving stale data until the invalidation comes through. So that could explain the difference.
There was a problem hiding this comment.
Ahh, good catch on the event_persister wrench being thrown in here @reivilibre!
I'm going to merge as this is already a step forward and got review attention/approval but we should also update the other spots that try to check /state as a one-off (wherever user.Do(t, "GET", getPathForState(...)) is happening). The negative assertions are a bit tricky.
reivilibre
left a comment
reivilibre
left a comment
There was a problem hiding this comment.
This seems correct to me
MadLittleMods
deleted the
madlittlemods/more-robust-delayed-event-state-change
branch
2 participants
Fix state not showing up fast enough with
TestDelayedEvents.Follow-up to #830
As experienced when running this test against the worker-based Synapse setup we use alongside the Synapse Pro Rust apps, https://github.com/element-hq/synapse-rust-apps/actions/runs/24910122124/job/72949760158?pr=360 (https://github.com/element-hq/synapse-rust-apps/pull/360)
Why does this happen?
Discussed in #865 (comment)
Dev notes
MSC4140 Synapse implementation added in element-hq/synapse#17326
Pull Request Checklist