From 21e9fc5aa7cd1d71de93b50fa1883c1ee056fe07 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 8 Jul 2022 14:52:27 +0100 Subject: [PATCH 1/2] Ensure portdb selects rows with negative rowids The `backward_select` query uses the `backward_chunk` variable as an inclusive upper bound on the rowids it selects. It is initially 0 (see `setup_table`). It is then set to ``` backward_chunk = min(row[0] for row in brows) - 1 ``` where `brows` is the result of running the `backwards_select` query. For this to make sense, we need to ensure that `backwards_select` picks rows in descending order. Otherwise we'll jump right to the bottom of the rowids, pick out the lowest batch only and discard everything we skipped over. This is a Bad Thing. I've tested this locally with the reproduction case reported in #13191. Without the patch, I could reproduce the reported failure; with the patch, the portdb script completes successfully. --- synapse/_scripts/synapse_port_db.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 642fd41629b1..26834a437e9c 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -418,12 +418,15 @@ async def handle_table( self.progress.update(table, table_size) # Mark table as done return + # We sweep over rowids in two directions: one forwards (rowids 1, 2, 3, ...) + # and another backwards (rowids 0, -1, -2, ...). forward_select = ( "SELECT rowid, * FROM %s WHERE rowid >= ? ORDER BY rowid LIMIT ?" % (table,) ) backward_select = ( - "SELECT rowid, * FROM %s WHERE rowid <= ? ORDER BY rowid LIMIT ?" % (table,) + "SELECT rowid, * FROM %s WHERE rowid <= ? ORDER BY rowid DESC LIMIT ?" + % (table,) ) do_forward = [True] From d1decf65b051d4f57cd2d2fd372e5261c4a6f31e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 8 Jul 2022 15:18:28 +0100 Subject: [PATCH 2/2] Changelog --- changelog.d/13226.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13226.bugfix diff --git a/changelog.d/13226.bugfix b/changelog.d/13226.bugfix new file mode 100644 index 000000000000..df96d41f3779 --- /dev/null +++ b/changelog.d/13226.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where the `synapse_port_db` script could fail to copy rows with negative row ids.