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

chore(data-warehouse): Updated billing limits to work per sync #24752

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

Gilbert09
Copy link
Member

@Gilbert09 Gilbert09 commented Sep 2, 2024

Problem

In my opinion, we had a subpar experience for when users hit data warehouse billing limits. We're running a celery task every 20 mins to check whether a team has hit its limits and then "Pause" and "Cancel" existing jobs/schemas. Why was this bad?

  • Users would have to wait 20 mins before they could resync once they upgraded their billing limits (with no info around this)
  • A job being in a "Cancelled" state was confusing to users - already had a support ticket asking what this meant
  • Users could go over their billing limit with subsequent jobs for up to 20 mins

Changes

  • Remove the celery task
  • Added billing limit check at the start of every data warehouse sync - this will cancel the job if the user has hit their billing limits and log this out
  • Re-utilise the "cancelled" status and instead return "billing limits" as the job/schema/source status instead
  • Removed the ad-hoc 500,000,000 monthly sync limit
Screenshot 2024-09-02 at 18 22 50 Screenshot 2024-09-02 at 18 22 44 Screenshot 2024-09-02 at 18 22 38

Does this work well for both Cloud and self-hosted?

Yep

How did you test this code?

Added a unit test to ensure no rows get synced when limits are hit

@Gilbert09 Gilbert09 requested review from EDsCODE and a team September 2, 2024 17:30
Copy link
Contributor

github-actions bot commented Sep 2, 2024

Size Change: 0 B

Total Size: 1.11 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.11 MB

compressed-size-action

Copy link
Contributor

@tomasfarias tomasfarias left a comment

Choose a reason for hiding this comment

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

Just had some open question regarding whether we should instead have a separate status for billing limits reached instead of re-using cancelled.

But overall I don't think it would be a blocker.

@Gilbert09 Gilbert09 merged commit 9842e97 into master Sep 4, 2024
93 checks passed
@Gilbert09 Gilbert09 deleted the tom/dwh-billing-limits branch September 4, 2024 14:21
Copy link

sentry-io bot commented Sep 4, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ActivityError: Activity task timed out posthog.temporal.data_imports.external_data_job... View Issue
  • ‼️ RuntimeError: Ignoring add command while deleting posthog.temporal.data_imports.external_data_job... View Issue
  • ‼️ IntegrityError: insert or update on table "posthog_externaldatajob" violates foreign key constraint "posthog_exte... posthog.warehouse.external_data_source.jobs in ... View Issue
  • ‼️ RuntimeError: Ignoring add command while deleting posthog.temporal.data_imports.external_data_job... View Issue
  • ‼️ ActivityError: Activity task timed out posthog.temporal.data_imports.external_data_job... View Issue

Did you find this useful? React with a 👍 or 👎

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.

2 participants