Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch user notifications together #4486

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

999eagle
Copy link

This PR changes how user notifications are handled. Both the RefreshChannelsJob and incoming pubsub webhooks are changed here to not directly persist notifications or the feed update flag to the database. A new job (or rather, a new fiber in the NotificationJob as I thought that'd be a good place to put this) handles all notifications and periodically writes them to the database batched by channels. With this change the RefreshChannelsJob now also correctly (at least that's my understanding) notifies the /api/v1/auth/notifications endpoint through the PostgreSQL NOTIFY/LISTEN mechanism already in use.
This greatly reduces the write load on the database when a channel has multiple video updates within the job timeout (i.e. 1 minute currently). From my testing this does seem to happen pretty often, especially when the RefreshChannelsJob runs at instance restart after the previous instance's backoff time for the job got too high.

@999eagle 999eagle requested a review from a team as a code owner March 15, 2024 22:58
@999eagle 999eagle requested review from SamantazFox and removed request for a team March 15, 2024 22:58
@unixfox
Copy link
Member

unixfox commented Apr 15, 2024

Could you take a look at @syeopite and @SamantazFox :)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants