feat: Add secondary database and update router
We've recently moved crons to a dedicated primary in saas. This has resulted in a tombstone issue as the joins used for tombstones don't work in the crons database. Reproducing this issue in the test suite has been challenging because in tests monitor tables and tombstone tables end up in the same connection.
By adding an additional database, connection and a new db router we're able to more closely represent how the application operates in tests.
@getsentry/crons There are a few failing tests in these results from the monitor details endpoint.
https://github.com/getsentry/sentry/blob/793c62cb6cf3f99dc8fe6d13de5945a333bf09c0/src/sentry/monitors/endpoints/base_monitor_details.py#L181-L247
This passage of code currently makes a cross database transaction. While this code behaves correctly in tests, it is not correct in production as the operations on Rule and Environment cannot occur within a transaction on the crons connection.
I can get that problem addressed, but given that we can't have all of the changes happen within a single transaction anymore, how should it work?
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 80.00%. Comparing base (
abf1a07) to head (17cad13). Report is 243 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #69697 +/- ##
==========================================
+ Coverage 79.98% 80.00% +0.02%
==========================================
Files 6492 6476 -16
Lines 289830 289204 -626
Branches 49943 49868 -75
==========================================
- Hits 231828 231391 -437
+ Misses 57591 57421 -170
+ Partials 411 392 -19
| Files | Coverage Δ | |
|---|---|---|
| src/sentry/db/router.py | 93.57% <100.00%> (+0.50%) |
:arrow_up: |
| src/sentry/monitors/consumers/monitor_consumer.py | 91.11% <100.00%> (+0.08%) |
:arrow_up: |
| .../sentry/monitors/endpoints/base_monitor_details.py | 96.63% <100.00%> (+0.08%) |
:arrow_up: |
| ...sentry/services/hybrid_cloud/import_export/impl.py | 90.43% <100.00%> (+0.08%) |
:arrow_up: |
| src/sentry/tasks/deletion/hybrid_cloud.py | 94.35% <ø> (+4.57%) |
:arrow_up: |
| src/sentry/testutils/pytest/sentry.py | 86.91% <100.00%> (+0.59%) |
:arrow_up: |
@getsentry/crons There are a few failing tests in these results from the monitor details endpoint.
https://github.com/getsentry/sentry/blob/793c62cb6cf3f99dc8fe6d13de5945a333bf09c0/src/sentry/monitors/endpoints/base_monitor_details.py#L181-L247
This passage of code currently makes a cross database transaction. While this code behaves correctly in tests, it is not correct in production as the operations on
RuleandEnvironmentcannot occur within a transaction on thecronsconnection.I can get that problem addressed, but given that we can't have all of the changes happen within a single transaction anymore, how should it work?
I think we should probably just open a transaction on both databases. We don't get any perfect guarantee of transactional consistency, but if any errors occur then we'll roll back both. Really the only failure case is if the last transaction fails to commit, which seems about the best we could do.