Skip to content

Commit 51a0e96

Browse files
committed
fix(ci): deep copy database configs in configure_split_db for xdist isolation
.copy() (shallow) caused default/control/secondary to share the same TEST dict object. When pytest-django set per-worker TEST.NAME (_gw0), the last write won and all three databases got the same name. Workers shared one Postgres DB instead of getting isolated test_region_gw0, test_control_gw0 etc. Changed to copy.deepcopy().
1 parent a0877c7 commit 51a0e96

File tree

3 files changed

+17
-7
lines changed

3 files changed

+17
-7
lines changed

docs/notes.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ Verified empirically: `socket.setdefaulttimeout(5)` → listening socket `.getti
3333

3434
This is a subtle interaction: `pytest-rerunfailures` assumes sockets block indefinitely, but Sentry's global timeout silently changes that contract on accepted connections only.
3535

36+
## configure_split_db shallow copy bug
37+
38+
`configure_split_db()` used `.copy()` (shallow) to create `control` and `secondary` from `default`. The `TEST` dict inside each was the **same Python object**. When pytest-django's `_set_suffix_to_test_databases` iterated databases and set `TEST.NAME` for each, the last write overwrote all three. All workers ended up using the same Postgres database instead of per-worker isolation (`test_region_gw0`, `test_control_gw0`, etc.). Fixed with `copy.deepcopy()`.
39+
40+
This bug was latent on master (no xdist, no TEST.NAME collision) and only manifested under xdist where per-worker DB names are required.
41+
3642
## Why hash-based sharding beats algorithmic LPT
3743

3844
With 17+ shards and ~32K tests, the law of large numbers gives hash-based (`sha256(nodeid) % N`) sharding good-enough balance (~90-130s spread). LPT algorithms failed because:

docs/tiered-xdist-changes.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,13 @@ Normal `--reruns` is unaffected — each worker retries failed tests locally via
9393

9494
The `calculate-shards` job now has a fast path: when selective testing isn't active (push to master/branch), it outputs static defaults (22 shards) without checkout, setup-sentry, or `pytest --collect-only`. Saves ~3 min on the critical path. When selective testing IS active (PR), the full collection pipeline still runs to compute the right shard count.
9595

96-
### 2g. Snowflake test fix
96+
### 2g. Fix shared TEST dict in configure_split_db
97+
98+
**Modified:** `src/sentry/testutils/pytest/sentry.py` (`configure_split_db`)
99+
100+
Changed `.copy()` to `copy.deepcopy()` when creating `control` and `secondary` database configs. The shallow copy caused all three databases to share the same `TEST` dict object. When pytest-django's xdist suffix fixture set `TEST.NAME` for each database, the last write won — all three databases got the same test DB name. Workers shared a single Postgres database instead of getting isolated `test_region_gw0`, `test_control_gw0`, etc.
101+
102+
### 2h. Snowflake test fix
97103

98104
**Modified:** `tests/sentry/utils/test_snowflake.py`
99105

src/sentry/testutils/pytest/sentry.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,14 @@ def configure_split_db() -> None:
4242
if already_configured or _use_monolith_dbs():
4343
return
4444

45-
# Add connections for the region & control silo databases.
46-
settings.DATABASES["control"] = settings.DATABASES["default"].copy()
45+
import copy
46+
47+
settings.DATABASES["control"] = copy.deepcopy(settings.DATABASES["default"])
4748
settings.DATABASES["control"]["NAME"] = "control"
4849

49-
# Use the region database in the default connection as region
50-
# silo database is the 'default' elsewhere in application logic.
5150
settings.DATABASES["default"]["NAME"] = "region"
5251

53-
# Add a connection for the secondary db
54-
settings.DATABASES["secondary"] = settings.DATABASES["default"].copy()
52+
settings.DATABASES["secondary"] = copy.deepcopy(settings.DATABASES["default"])
5553
settings.DATABASES["secondary"]["NAME"] = "secondary"
5654

5755
settings.DATABASE_ROUTERS = ("sentry.db.router.TestSiloMultiDatabaseRouter",)

0 commit comments

Comments
 (0)