Skip to content

Commit 24fe976

Browse files
test(migrations): Scope reverse idempotency snapshot to Snuba databases (#8592)
`test_reverse_idempotency_all` snapshots `system.tables` before and after reversing each group's latest migration twice. The snapshot included ClickHouse's own `system.*` tables, and ClickHouse creates log tables such as `system.query_metric_log` lazily in the background. When one of those appeared mid-test, the before and after snapshots differed and the test failed for reasons unrelated to migrations. That happened on [attempt 1 of this run](https://github.com/getsentry/snuba/actions/runs/39140651685/job/117714460279): the failure diff contains `system.query_metric_log`, and the same image passed on retry. Because the whole workflow was rerun, 34 already-green jobs ran again, which cost about 184 runner-minutes. The snapshot now only covers the databases of Snuba's configured clusters, the same set `_drop_all_tables` uses. It selects `(database, name, create_table_query)` ordered by `(database, name)`. Every group, the double reversal, and the comparison of all Snuba tables across groups are unchanged. A side benefit: the failure output used to print every `system` table definition (about 15k log lines). It now only prints Snuba tables. <!-- junior-request-attribution:start --> via **Joshua Li**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3Ad2c357b94d471741d23c9928 --> -- [View Junior Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3Ad2c357b94d471741d23c9928/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Joshua Li <joshua.li@sentry.io>
1 parent 2bd3e71 commit 24fe976

1 file changed

Lines changed: 9 additions & 8 deletions

File tree

‎tests/migrations/test_runner.py‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,15 @@ def test_reverse_idempotency_all() -> None:
318318
"""
319319
runner = Runner()
320320
groups = get_active_migration_groups()
321+
# Snapshot only Snuba's databases: ClickHouse creates `system.*` log tables
322+
# (e.g. `system.query_metric_log`) lazily in the background, which made the
323+
# before/after comparison flaky. `system.tables` has no guaranteed row
324+
# order, so order by table identity.
325+
snuba_databases = ", ".join(sorted({f"'{cluster.get_database()}'" for cluster in CLUSTERS}))
326+
snapshot_query = (
327+
"SELECT database, name, create_table_query FROM system.tables "
328+
f"WHERE database IN ({snuba_databases}) ORDER BY database, name"
329+
)
321330
group_migrations = {
322331
group.value: runner._get_pending_migrations_for_group(group) for group in groups
323332
}
@@ -349,14 +358,6 @@ def reverse_twice(migration: MigrationKey = migration) -> None:
349358
ClickhouseClientSettings.MIGRATE
350359
)
351360

352-
# `system.tables` has no guaranteed row order, and the
353-
# reverse_twice() cycle in between can perturb the order rows
354-
# are returned in (e.g. ON CLUSTER DDL re-registering tables).
355-
# Order the snapshots so the comparison reflects the set of
356-
# tables and their definitions, not the iteration order.
357-
snapshot_query = (
358-
"SELECT create_table_query FROM system.tables ORDER BY create_table_query"
359-
)
360361
before_state = cluster_connection.execute(snapshot_query).results
361362
reverse_twice()
362363
after_state = cluster_connection.execute(snapshot_query).results

0 commit comments

Comments
 (0)