sentry icon indicating copy to clipboard operation
sentry copied to clipboard

Update sentry_app queries to use the read replica

Open kneeyo1 opened this issue 7 months ago • 6 comments

This updates some of the queries to point to the control db replica.

kneeyo1 avatar Jun 06 '25 22:06 kneeyo1

Codecov Report

Attention: Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/sentry_apps/services/app/impl.py 94.73% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #93081      +/-   ##
==========================================
+ Coverage   84.35%   87.97%   +3.61%     
==========================================
  Files       10319    10326       +7     
  Lines      594698   593516    -1182     
  Branches    23129    22860     -269     
==========================================
+ Hits       501687   522173   +20486     
+ Misses      92518    70899   -21619     
+ Partials      493      444      -49     

codecov[bot] avatar Jun 06 '25 23:06 codecov[bot]

hihi I have no context but why are we doing this change? Is this a pattern we are adopting for more models?

Christinarlong avatar Jun 11 '25 17:06 Christinarlong

hihi I have no context but why are we doing this change? Is this a pattern we are adopting for more models?

This is to help lessen load on the primary control db.

kneeyo1 avatar Jun 11 '25 21:06 kneeyo1

hihi I have no context but why are we doing this change? Is this a pattern we are adopting for more models?

This is to help lessen load on the primary control db.

I'm confused why are we doing this at business logic level? It's also confusing to me if we should be following this pattern for which control models, or how was that decided?

Christinarlong avatar Jun 12 '25 16:06 Christinarlong

hihi I have no context but why are we doing this change? Is this a pattern we are adopting for more models?

This is to help lessen load on the primary control db.

I'm confused why are we doing this at business logic level? It's also confusing to me if we should be following this pattern for which control models, or how was that decided?

We want to follow this model when possible. We do this at a business logic level (routing) because we want to edit it per query, not for all read queries. Some application logic will follow a read-your-writes pattern, and since all writes must go to the primary, if we point all reads to the replica, it can result in a miss due to replication lag.

Thus we only want to point read queries to be reading from the replica when there isn't this constraint.

If we wanted to blanket apply all reads to be from the read query though, it would probably look something like this: https://github.com/getsentry/getsentry/pull/17625

kneeyo1 avatar Jun 12 '25 18:06 kneeyo1

okay, that makes sense. Please make sure to add us for any similar changes in the integrations/notifications domain since any time sensitive models could have issues with reading from replica.

Christinarlong avatar Jun 12 '25 21:06 Christinarlong