From 71d86452d7989404de9448e92f684e86c965aa5c Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 24 Oct 2023 20:20:14 -0500 Subject: [PATCH 1/6] Exclude sending a database query that can't exist Auth chain chain_id's and sequence numbers can't be zero, but that can appear in the query here. Stop that from showing up. --- synapse/storage/databases/main/event_federation.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 4f80ce75ccb4..dcb06ccc1dde 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -298,9 +298,13 @@ def _get_auth_chain_ids_using_cover_index_txn( ) # Add the initial set of chains, excluding the sequence corresponding to - # initial event. + # initial event. Ultimately, the chains dict will be what is pulled from the + # database, there is a chance that a sequence number here will end up being a + # '0', which doesn't exist. Don't bother sending that to the database query. for chain_id, seq_no in event_chains.items(): - chains[chain_id] = max(seq_no - 1, chains.get(chain_id, 0)) + max_sequence_result = max(seq_no - 1, chains.get(chain_id, 0)) + if max_sequence_result > 0: + chains[chain_id] = max_sequence_result # Now for each chain we figure out the maximum sequence number reachable # from *any* event ID. Events with a sequence less than that are in the From 98f253e0c80fc4f45b5a7046056723b14494e67b Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 24 Oct 2023 20:24:54 -0500 Subject: [PATCH 2/6] [REVERT] Add in metrics counter to prove this gets hit --- synapse/storage/databases/main/event_federation.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index dcb06ccc1dde..70618fd8bcc4 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -73,6 +73,9 @@ "The number of events in the inbound federation staging that have been " "pruned due to the queue getting too long", ) +excluded_zeros_from_auth_chain_query = Counter( + "synapse_temp_auth_chain_zero_sequence", "" +) logger = logging.getLogger(__name__) @@ -305,6 +308,8 @@ def _get_auth_chain_ids_using_cover_index_txn( max_sequence_result = max(seq_no - 1, chains.get(chain_id, 0)) if max_sequence_result > 0: chains[chain_id] = max_sequence_result + else: + excluded_zeros_from_auth_chain_query.inc() # Now for each chain we figure out the maximum sequence number reachable # from *any* event ID. Events with a sequence less than that are in the From 76e2acaf1f845dacc28519b844b32133d1fd921c Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 25 Oct 2023 19:26:22 -0500 Subject: [PATCH 3/6] changelog --- changelog.d/16552.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16552.misc diff --git a/changelog.d/16552.misc b/changelog.d/16552.misc new file mode 100644 index 000000000000..73a0376df097 --- /dev/null +++ b/changelog.d/16552.misc @@ -0,0 +1 @@ +Reduce a little database load while processing state auth chains. From 5791fa25aef1887fd66d028d4f4a2e664f8b5867 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Wed, 25 Oct 2023 19:26:48 -0500 Subject: [PATCH 4/6] Revert "[REVERT] Add in metrics counter to prove this gets hit" This reverts commit 98f253e0c80fc4f45b5a7046056723b14494e67b. --- synapse/storage/databases/main/event_federation.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 70618fd8bcc4..dcb06ccc1dde 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -73,9 +73,6 @@ "The number of events in the inbound federation staging that have been " "pruned due to the queue getting too long", ) -excluded_zeros_from_auth_chain_query = Counter( - "synapse_temp_auth_chain_zero_sequence", "" -) logger = logging.getLogger(__name__) @@ -308,8 +305,6 @@ def _get_auth_chain_ids_using_cover_index_txn( max_sequence_result = max(seq_no - 1, chains.get(chain_id, 0)) if max_sequence_result > 0: chains[chain_id] = max_sequence_result - else: - excluded_zeros_from_auth_chain_query.inc() # Now for each chain we figure out the maximum sequence number reachable # from *any* event ID. Events with a sequence less than that are in the From 94e22c2ae9ce122478513b7a2fa03a98aa6a0ede Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 21 Nov 2023 03:50:02 -0600 Subject: [PATCH 5/6] Just continue the for loop if the sequence number is 1, since that will be the first item in that chain anyways --- synapse/storage/databases/main/event_federation.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 18f5884fc540..8cea879b9ece 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -303,9 +303,12 @@ def _get_auth_chain_ids_using_cover_index_txn( # database, there is a chance that a sequence number here will end up being a # '0', which doesn't exist. Don't bother sending that to the database query. for chain_id, seq_no in event_chains.items(): - max_sequence_result = max(seq_no - 1, chains.get(chain_id, 0)) - if max_sequence_result > 0: - chains[chain_id] = max_sequence_result + # Check if the initial event is the first item in the chain. If so, then + # there is nothing new to add from this chain. + if seq_no == 1: + continue + + chains[chain_id] = max(seq_no - 1, chains.get(chain_id, 0)) # Now for each chain we figure out the maximum sequence number reachable # from *any* event ID. Events with a sequence less than that are in the From 3eaeaff9e810a8f5e0ed0fa5188bb727b860b0ea Mon Sep 17 00:00:00 2001 From: Jason Little Date: Tue, 21 Nov 2023 18:22:34 -0600 Subject: [PATCH 6/6] Remove comment --- synapse/storage/databases/main/event_federation.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 8cea879b9ece..d5fb6ecc73bc 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -299,9 +299,7 @@ def _get_auth_chain_ids_using_cover_index_txn( ) # Add the initial set of chains, excluding the sequence corresponding to - # initial event. Ultimately, the chains dict will be what is pulled from the - # database, there is a chance that a sequence number here will end up being a - # '0', which doesn't exist. Don't bother sending that to the database query. + # initial event. for chain_id, seq_no in event_chains.items(): # Check if the initial event is the first item in the chain. If so, then # there is nothing new to add from this chain.