Skip to content

Conversation

@emma-sg
Copy link
Member

@emma-sg emma-sg commented Dec 8, 2025

To be merged into #2944

Adds a causally consistent session specifically to the quota update method so that its three operations can operate more-or-less atomically.

We'd looked at doing this with a transaction, but they require a replica set or sharding, which isn't always included in self-hosted Browsertrix deployments. For now this should improve reliability for the multi-operation update_quotas method, even if it's not perfectly atomic — it's pretty unlikely at the scale we're working with here for there to be multiple simultaneous operations for a single org.

Tested with local deployment.

@emma-sg emma-sg requested a review from ikreymer December 8, 2025 20:06
@emma-sg emma-sg marked this pull request as ready for review December 8, 2025 20:06
@emma-sg emma-sg requested a review from tw4l December 8, 2025 20:06
@emma-sg emma-sg force-pushed the additional-minutes--quota-transaction branch from 12992e3 to 6f2c805 Compare December 9, 2025 22:34
@emma-sg emma-sg changed the title Add transaction to quota update method Add session to quota update method Dec 9, 2025
@emma-sg emma-sg changed the title Add session to quota update method Use dedicated mongo session in quota update method Dec 9, 2025
@emma-sg emma-sg force-pushed the additional-minutes-addon-purchase branch from 261f9b8 to 43bb583 Compare December 9, 2025 23:09
@emma-sg emma-sg force-pushed the additional-minutes--quota-transaction branch from 6f2c805 to 702100c Compare December 9, 2025 23:11
org_with_quotas, admin_auth_headers, preshared_secret_auth_headers
):
r = requests.post(
f"{API_PREFIX}/orgs/{org_with_quotas}/quotas/add",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I removed the /quotas/add endpoint, as we weren't using it directly anymore (using /subscription/addMinutes instead), but could still test this with /quotas/update

Copy link
Member

@ikreymer ikreymer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and makes sense.
(Remove extra nightly test that was probably readded during merge - that functionality is now being tested in regular tests)

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.

3 participants