Update some sentry_app queries to use the read replica #93667
Conversation
github-actions
bot
added
the
Scope: Backend
| ] | ||
|
|
||
| def get_sentry_app_by_id(self, *, id: int) -> RpcSentryApp | None: | ||
| try: | ||
| sentry_app = SentryApp.objects.get(id=id) | ||
| sentry_app = SentryApp.objects.using_replica().get(id=id) |
There was a problem hiding this comment.
So on the surface, this makes sense and doesn't seem to have an effect on the calling code, but I do have a couple of questions:
- Do we have an idea of how much replication lag we'll have to deal with on average?
- Is there maybe some way in the RPC name itself we can denote that this is using a replica? Having an RPC exhibit eventually consistent behavior can be pretty jarring from the developer perspective if you're not expecting it.
There was a problem hiding this comment.
Going through datadog the average replication delay is around 100 MS for this db.
I could change the function signature to more explicitly call out that this is reading from the db replica, if you'd like?
There was a problem hiding this comment.
Going through datadog the average replication delay is around 100 MS for this db.
Got it, that's not too bad then.
Re: the RPC name, I don't want to create a ton more work for you since this would require multiple PRs to accomplish, but I do wonder if philosophically we need be more explicit about when we are pushing services calls to read from replicas. Just something to consider.
There was a problem hiding this comment.
hmm.. for now I can add some docs around the function signature to indidcate that this function is reading from a replica?
There was a problem hiding this comment.
A docstring would be a good start. I wouldn't recommend renaming methods as it is a complicated process.
This RPC is mostly called from regions in Endpoint.convert_args(), there is some risk of read-your-writes but the endpoints where it is used are going to be used infrequently.
| ] | ||
|
|
||
| def get_sentry_app_by_id(self, *, id: int) -> RpcSentryApp | None: | ||
| try: | ||
| sentry_app = SentryApp.objects.get(id=id) | ||
| sentry_app = SentryApp.objects.using_replica().get(id=id) |
There was a problem hiding this comment.
A docstring would be a good start. I wouldn't recommend renaming methods as it is a complicated process.
This RPC is mostly called from regions in Endpoint.convert_args(), there is some risk of read-your-writes but the endpoints where it is used are going to be used infrequently.
This updates some of the queries to point to the control db replica. see #93081, breaking that change apart so we can better see the effects of these on the db.
This updates some of the queries to point to the control db replica. see #93081, breaking that change apart so we can better see the effects of these on the db.
This updates some of the queries to point to the control db replica.
see #93081, breaking that change apart so we can better see the effects of these on the db.