From 7d03de9a07228d954a2f983950828ce0b1ea2cf1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 3 Jul 2026 11:43:07 +0000 Subject: [PATCH] Commit id-generator construction in the multi-writer id-gen tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constructing a `MultiWriterIdGenerator` prunes stale `stream_positions` rows for writers no longer in the config. The test harness built each generator inside a `runWithConnection` callback and never committed, so that cleanup only persisted because adbapi keeps one connection per thread and leaves its transaction open across calls — a later generator on the same thread then saw the uncommitted delete. The native Rust pool hands out a fresh connection per call and rolls back any left mid-transaction, so the cleanup was lost and test_writer_config_change read a stale writer position (persisted-upto 3 instead of 6). Commit the connection after constructing the generator, so the cleanup persists regardless of pool semantics; the delete is legitimate work that should be committed anyway. Fixes test_writer_config_change on the Rust backend; psycopg2 and sqlite still pass (16/16 on both). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01W3G4M92AmwSSZCbmtMJU3d --- tests/storage/test_id_generators.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/storage/test_id_generators.py b/tests/storage/test_id_generators.py index 051c5de44d..c8cdd9ed53 100644 --- a/tests/storage/test_id_generators.py +++ b/tests/storage/test_id_generators.py @@ -78,7 +78,7 @@ class MultiWriterIdGeneratorBase(HomeserverTestCase): writers: list[str] | None = None, ) -> MultiWriterIdGenerator: def _create(conn: LoggingDatabaseConnection) -> MultiWriterIdGenerator: - return MultiWriterIdGenerator( + id_gen = MultiWriterIdGenerator( db_conn=conn, db=self.db_pool, notifier=self.hs.get_replication_notifier(), @@ -90,6 +90,15 @@ class MultiWriterIdGeneratorBase(HomeserverTestCase): writers=writers or ["master"], positive=self.positive, ) + # Constructing the generator prunes stale `stream_positions` rows + # (writers no longer in the config); commit so that persists for the + # next generator we create. Without this the test relies on the + # connection pool keeping the uncommitted transaction open and + # visible across `runWithConnection` calls, which adbapi happens to + # do (one connection per thread) but the native pool does not (a + # fresh connection each call, rolled back if left mid-transaction). + conn.commit() + return id_gen self.instances[instance_name] = self.get_success_or_raise( self.db_pool.runWithConnection(_create)