Skip to content

Commit 2da7d38

Browse files
authored
fix(hc): Prevent exception from siloed_atomic (#52326)
An exception rises from `router.db_for_write` if the database doesn't exist in the environment, as observed with `SENTRY_USE_SILOS` set to true. Introduce a custom exception type to mark this condition, and catch it when setting the atomic impl. This should be equivalent behavior, as we only care about whether "missing" database is equal to `using` (which a null value won't be).
1 parent 2725e76 commit 2da7d38

File tree

2 files changed

+24
-15
lines changed

2 files changed

+24
-15
lines changed

src/sentry/db/router.py

+6-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
logger = logging.getLogger(__name__)
1313

1414

15+
class SiloConnectionUnavailableError(ValueError):
16+
pass
17+
18+
1519
class SiloRouter:
1620
"""
1721
Django database router for multi-region deployments.
@@ -75,7 +79,7 @@ def use_simulated(self, value: bool):
7579
raise ValueError("Cannot mutate simulation mode outside of tests")
7680
self.__is_simulated = value
7781

78-
def _resolve_silo_connection(self, silo_modes: Iterable[SiloMode], table: str):
82+
def _resolve_silo_connection(self, silo_modes: Iterable[SiloMode], table: str) -> str:
7983
# XXX This method has an override in getsentry for region silo primary splits.
8084
active_mode = SiloMode.get_current_mode()
8185

@@ -92,7 +96,7 @@ def _resolve_silo_connection(self, silo_modes: Iterable[SiloMode], table: str):
9296
# If we're in tests raise an error, otherwise return 'no decision'
9397
# so that django skips migration operations that won't work.
9498
if "pytest" in sys.modules:
95-
raise ValueError(
99+
raise SiloConnectionUnavailableError(
96100
f"Cannot resolve table {table} in {silo_mode}. "
97101
f"Application silo mode is {active_mode} and simulated silos are not enabled."
98102
)

src/sentry/silo/patches/silo_aware_transaction_patch.py

+18-13
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,28 @@
1-
from typing import Optional
1+
from typing import TYPE_CHECKING, Optional, Type
22

33
from django import get_version
44
from django.db import router, transaction
55
from django.db.transaction import Atomic
66

7+
if TYPE_CHECKING:
8+
from sentry.db.models import Model
9+
710
_default_atomic_impl = transaction.atomic
811

912

1013
class MismatchedSiloTransactionError(Exception):
1114
pass
1215

1316

17+
def _get_db_for_model_if_available(model: Type["Model"]) -> Optional[str]:
18+
from sentry.db.router import SiloConnectionUnavailableError
19+
20+
try:
21+
return router.db_for_write(model)
22+
except SiloConnectionUnavailableError:
23+
return None
24+
25+
1426
def siloed_atomic(using: Optional[str] = None, savepoint: bool = True) -> Atomic:
1527
from sentry.models import ControlOutbox, RegionOutbox
1628
from sentry.silo import SiloMode
@@ -20,24 +32,17 @@ def siloed_atomic(using: Optional[str] = None, savepoint: bool = True) -> Atomic
2032
model_to_route_to = RegionOutbox if current_silo_mode == SiloMode.REGION else ControlOutbox
2133
using = router.db_for_write(model_to_route_to)
2234

23-
both_silos_route_to_same_db = router.db_for_write(ControlOutbox) == router.db_for_write(
24-
RegionOutbox
25-
)
35+
control_db = _get_db_for_model_if_available(ControlOutbox)
36+
region_db = _get_db_for_model_if_available(RegionOutbox)
37+
both_silos_route_to_same_db = control_db == region_db
2638

2739
if both_silos_route_to_same_db or current_silo_mode == SiloMode.MONOLITH:
2840
pass
29-
elif (
30-
using == router.db_for_write(ControlOutbox)
31-
and SiloMode.get_current_mode() != SiloMode.CONTROL
32-
):
41+
elif using == control_db and SiloMode.get_current_mode() != SiloMode.CONTROL:
3342
raise MismatchedSiloTransactionError(
3443
f"Cannot use transaction.atomic({using}) in Control Mode"
3544
)
36-
37-
elif (
38-
using == router.db_for_write(RegionOutbox)
39-
and SiloMode.get_current_mode() != SiloMode.REGION
40-
):
45+
elif using == region_db and SiloMode.get_current_mode() != SiloMode.REGION:
4146
raise MismatchedSiloTransactionError(
4247
f"Cannot use transaction.atomic({using}) in Region Mode"
4348
)

0 commit comments

Comments
 (0)