ref(relay): Reduce lock contention on Relay usage - #121987
Conversation
github-actions
Bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
15989e6 to
4f17216
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fc4093d. Configure here.
| @@ -101,9 +102,12 @@ def post(self, request: Request) -> Response: | |||
| except RelayUsage.DoesNotExist: | |||
| RelayUsage.objects.create(relay_id=relay_id, version=version, public_key=public_key) | |||
There was a problem hiding this comment.
Concurrent usage creates can 500
Medium Severity
Switching from get_or_create to a bare get plus create reopens a race on (relay_id, version). Concurrent first-time registrations can both miss the row, then the second create hits the unique constraint and returns 500 instead of taking the update path.
Reviewed by Cursor Bugbot for commit fc4093d. Configure here.
There was a problem hiding this comment.
Yeah, but that's not new. Can fix that in a separate PR
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| buffered_last_seen = mock_buffer_incr.call_args.kwargs["extra"]["last_seen"] | ||
| mock_buffer_incr.assert_called_once_with( | ||
| model=RelayUsage, | ||
| columns={}, | ||
| filters={"id": rv1.id}, | ||
| extra={"last_seen": buffered_last_seen, "public_key": str(key_pair[1])}, | ||
| ) | ||
| assert buffered_last_seen > after_second_relay | ||
| assert buffered_last_seen < after_re_register |
There was a problem hiding this comment.
Nit: I think there's a way to force this to process so you can see that the update goes through. Maybe by using with self.tasks():. There are probably some examples in the code, it might be worthwhile to just verify that it works as expected
2 participants
On large request bursts this causes some contention on the
RelayUsagemodel. Debounce the updates a bit.I was also considering a
select for update+skip locked, curious to hear opinions.