From 058a1df7bfd86fb3fbff83fb0e6a89b7d05482c5 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Wed, 8 Apr 2026 16:19:18 +0200 Subject: [PATCH] Review comments --- contrib/grafana/synapse.json | 151 +++++++++++++++++++++++- tests/storage/test_msc4242_state_dag.py | 29 +++-- 2 files changed, 171 insertions(+), 9 deletions(-) diff --git a/contrib/grafana/synapse.json b/contrib/grafana/synapse.json index ceacc10369..a67d1aba0b 100644 --- a/contrib/grafana/synapse.json +++ b/contrib/grafana/synapse.json @@ -6809,6 +6809,155 @@ ], "title": "Stale extremity dropping", "type": "timeseries" + }, + { + "datasource": { + "uid": "${DS_PROMETHEUS}" + }, + "description": "For a given percentage P, the number X where P% of events were persisted to rooms with X state DAG forward extremities or fewer.", + "fieldConfig": { + "defaults": { + "links": [] + }, + "overrides": [] + }, + "gridPos": { + "h": 8, + "w": 12, + "x": 12, + "y": 50 + }, + "id": 181, + "options": { + "alertThreshold": true + }, + "pluginVersion": "9.2.2", + "targets": [ + { + "datasource": { + "uid": "${DS_PROMETHEUS}" + }, + "expr": "histogram_quantile(0.5, rate(synapse_storage_msc4242_state_dag_forward_extremities_persisted_bucket{server_name=\"$server_name\"}[$bucket_size]) and on (instance, job, index) (synapse_storage_events_persisted_events_total > 0))", + "format": "time_series", + "intervalFactor": 1, + "legendFormat": "50%", + "refId": "A" + }, + { + "datasource": { + "uid": "${DS_PROMETHEUS}" + }, + "expr": "histogram_quantile(0.75, rate(synapse_storage_msc4242_state_dag_forward_extremities_persisted_bucket{server_name=\"$server_name\"}[$bucket_size]) and on (instance, job, index) (synapse_storage_events_persisted_events_total > 0))", + "format": "time_series", + "intervalFactor": 1, + "legendFormat": "75%", + "refId": "B" + }, + { + "datasource": { + "uid": "${DS_PROMETHEUS}" + }, + "expr": "histogram_quantile(0.90, rate(synapse_storage_msc4242_state_dag_forward_extremities_persisted_bucket{server_name=\"$server_name\"}[$bucket_size]) and on (instance, job, index) (synapse_storage_events_persisted_events_total > 0))", + "format": "time_series", + "intervalFactor": 1, + "legendFormat": "90%", + "refId": "C" + }, + { + "datasource": { + "uid": "${DS_PROMETHEUS}" + }, + "expr": "histogram_quantile(0.99, rate(synapse_storage_msc4242_state_dag_forward_extremities_persisted_bucket{server_name=\"$server_name\"}[$bucket_size]) and on (instance, job, index) (synapse_storage_events_persisted_events_total > 0))", + "format": "time_series", + "intervalFactor": 1, + "legendFormat": "99%", + "refId": "D" + } + ], + "title": "Events persisted, by number of state DAG forward extremities in room (quantiles)", + "type": "timeseries" + }, + { + "datasource": { + "uid": "${DS_PROMETHEUS}" + }, + "description": "Colour reflects the number of events persisted to rooms with the given number of state DAG forward extremities, or fewer.", + "fieldConfig": { + "defaults": { + "custom": { + "hideFrom": { + "legend": false, + "tooltip": false, + "viz": false + }, + "scaleDistribution": { + "type": "linear" + } + } + }, + "overrides": [] + }, + "gridPos": { + "h": 8, + "w": 12, + "x": 0, + "y": 50 + }, + "id": 127, + "options": { + "calculate": false, + "calculation": {}, + "cellGap": 1, + "cellValues": {}, + "color": { + "exponent": 0.5, + "fill": "#5794F2", + "min": 0, + "mode": "opacity", + "reverse": false, + "scale": "exponential", + "scheme": "Oranges", + "steps": 128 + }, + "exemplars": { + "color": "rgba(255,0,255,0.7)" + }, + "filterValues": { + "le": 1e-9 + }, + "legend": { + "show": true + }, + "rowsFrame": { + "layout": "auto" + }, + "showValue": "never", + "tooltip": { + "show": true, + "yHistogram": true + }, + "yAxis": { + "axisPlacement": "left", + "decimals": 0, + "reverse": false, + "unit": "short" + } + }, + "pluginVersion": "9.2.2", + "targets": [ + { + "datasource": { + "uid": "${DS_PROMETHEUS}" + }, + "expr": "rate(synapse_storage_msc4242_state_dag_forward_extremities_persisted_bucket{server_name=\"$server_name\"}[$bucket_size]) and on (instance, job, index) (synapse_storage_events_persisted_events_total > 0)", + "format": "heatmap", + "intervalFactor": 1, + "legendFormat": "{{le}}", + "refId": "A" + } + ], + "title": "Events persisted, by number of state DAG forward extremities in room (heatmap)", + "type": "heatmap" } ], "title": "Extremities", @@ -7711,4 +7860,4 @@ "uid": "000000012", "version": 1, "weekStart": "" -} +} \ No newline at end of file diff --git a/tests/storage/test_msc4242_state_dag.py b/tests/storage/test_msc4242_state_dag.py index d544f17469..4a1e91461f 100644 --- a/tests/storage/test_msc4242_state_dag.py +++ b/tests/storage/test_msc4242_state_dag.py @@ -63,7 +63,9 @@ class MSC4242StateDagsTests(HomeserverTestCase): second_id = self.helper.send(self.room_id, body="test2")["event_id"] second_prev_state_events = self._get_prev_state_events(second_id) assert len(second_prev_state_events) == 1 - self.assertEquals(first_prev_state_events, second_prev_state_events) + self.assertIncludes( + set(first_prev_state_events), set(second_prev_state_events), exact=True + ) # send an auth event, which should change the prev_state_events on *subsequent* events join_rule_state_event_id = self.helper.send_state( @@ -77,12 +79,18 @@ class MSC4242StateDagsTests(HomeserverTestCase): join_rule_prev_state_event_ids = self._get_prev_state_events( join_rule_state_event_id ) - self.assertEquals(second_prev_state_events, join_rule_prev_state_event_ids) + self.assertIncludes( + set(second_prev_state_events), + set(join_rule_prev_state_event_ids), + exact=True, + ) # prev_state_events should always point to the join rule now third_event_id = self.helper.send(self.room_id, body="test3")["event_id"] third_prev_state_events = self._get_prev_state_events(third_event_id) - self.assertEquals(third_prev_state_events, [join_rule_state_event_id]) + self.assertIncludes( + set(third_prev_state_events), {join_rule_state_event_id}, exact=True + ) # and non-auth state should also update prev_state_events name_state_event_id = self.helper.send_state( self.room_id, @@ -93,10 +101,14 @@ class MSC4242StateDagsTests(HomeserverTestCase): tok="nope", )["event_id"] name_prev_state_event_ids = self._get_prev_state_events(name_state_event_id) - self.assertEquals(name_prev_state_event_ids, [join_rule_state_event_id]) + self.assertIncludes( + set(name_prev_state_event_ids), {join_rule_state_event_id}, exact=True + ) fourth_event_id = self.helper.send(self.room_id, body="test4")["event_id"] fourth_prev_state_events = self._get_prev_state_events(fourth_event_id) - self.assertEquals(fourth_prev_state_events, [name_state_event_id]) + self.assertIncludes( + set(fourth_prev_state_events), {name_state_event_id}, exact=True + ) class MSC4242EventPersistenceStateDagsStoreTestCase(HomeserverTestCase): @@ -183,10 +195,11 @@ class MSC4242EventPersistenceStateDagsStoreTestCase(HomeserverTestCase): return new_extrems = set(self.get_success(coroutine)) - self.assertEqual( + self.assertIncludes( new_extrems, - want_new_extrems, - f"want_new_extrems={want_new_extrems} got={new_extrems}", + set(want_new_extrems), + exact=True, + message=f"want_new_extrems={want_new_extrems} got={new_extrems}", ) def test_calculate_new_state_dag_extremities_simple(self) -> None: