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

Enforce quota and hard cap for monthly execution minutes #1284

Merged
merged 44 commits into from
Oct 26, 2023

Conversation

tw4l
Copy link
Member

@tw4l tw4l commented Oct 13, 2023

Fixes #1261 Closes #1092

The quota for monthly execution minutes is treated as a hard cap. Once it is exceeded, an alert indicating that an org has exceeded its monthly execution minutes will display and the user will be unable to start new crawls. Any running crawls will be stopped once the quota is exceeded.

An execution minutes meter bar is also added in the Org Dashboard and displayed if a quota is set. More detail in #1305, which was merged into this branch.

Changes

  • Enable setting Max Execution Minutes Per Month in orgs list quotas:
Screen Shot 2023-10-26 at 12 45 25 PM
  • Enforce quota by stopping crawls in operator once quota is reached
  • Show alert banner once execution time quota is hit:
Screen Shot 2023-10-17 at 2 15 55 PM
  • Once quota is hit, disable Run Crawl buttons in frontend, return 403 message with execution_minutes_quota_reached detail in backend from crawl config /run endpoint, and don't run new workflows on creation (similar to storage quota)
  • Display execution time for crawls in the crawl details overview, immediately below Duration:
Screen Shot 2023-10-17 at 2 56 24 PM

To test

Manually

  1. In org quotas, set the Org Monthly Execution Minutes Quota to a low number of minutes
  2. Start a big crawl (e.g. https://webrecorder.net with extra hops enabled and no page limit)
  3. Verify that the big crawl starts to stop once the quota has been passed
  4. Verify that once the crawl finishes, it has Partial Complete status and the execution minutes alert appears at the top of the screen
  5. Verify that you can no longer run crawls
  6. Verify that the execution time displays in the crawl detail Overview page
  7. Verify that execution minutes meter in Org Dashboard displays and is accurate

Nightly tests

To run the nightly tests locally (requires pytest): pytest backend/test_nightly/test_execution_minutes_quota.py

These tests may take 5 or so minutes to complete.

@tw4l tw4l force-pushed the issue-1261-execution-time-exceeded branch 6 times, most recently from a2a94de to d5cca67 Compare October 17, 2023 05:05
@tw4l tw4l changed the title Enforce soft and hard caps for monthly execution minutes Enforce quota and hard cap for monthly execution minutes Oct 17, 2023
@tw4l tw4l force-pushed the issue-1261-execution-time-exceeded branch 3 times, most recently from 764a867 to 9949676 Compare October 17, 2023 18:56
@tw4l tw4l marked this pull request as ready for review October 17, 2023 19:07
@tw4l tw4l requested review from ikreymer and SuaYoo October 17, 2023 19:07
@tw4l
Copy link
Member Author

tw4l commented Oct 17, 2023

@Shrinks99 Assigned you for a copy and docs check :)

Copy link
Member

@Shrinks99 Shrinks99 left a comment

Choose a reason for hiding this comment

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

Have left a few suggestions for fixes, copy mostly good!

Once the above is dealt with, looks good to me! :D

Excited to flesh out the billing page in the future!

@tw4l
Copy link
Member Author

tw4l commented Oct 17, 2023

@Shrinks99 ready for re-review :)

@tw4l tw4l requested a review from Shrinks99 October 17, 2023 21:06
@Shrinks99
Copy link
Member

Change tab title from "Billing" to "Org Limits" for now until we sort out / add more billing features to the app.

tw4l added 7 commits October 19, 2023 12:23
@ikreymer
Copy link
Member

Thanks for this! The keeping track of current running exec time is neat but has two potential issues:

  • Multiple crawls may get different estimates based on when their execTime was last committed
  • Month rollover will count towards the new month for long-running crawls (this is a tricky one).

I've attempted to refactor this a bit to instead force increments in short intervals to avoid tracking uncommitted exec time altogether.

This adds an slight refactor to #1284 to ensure execution minutes are
updated incrementally while a crawl is running.
The execution minutes are updated on the org and on the crawl if:
- 60 seconds elapsed from last update
- any pod has newly exited
- crawl has been canceled (compute to current time)
- if month has changed

This should ensure that the execution time quota is accurate within 1
minute, and also that crawls running at the end of the month properly
count towards old and new month quotas (a bit of an edge case yes, but
bound to happen!)

other cleanup:
- add background tasks to a set to avoid premature garbage collection
(see: https://stackoverflow.com/a/74059981)
- use dt_now() consistently instead of datetime.utcnow() to store
second-rounded dates
@tw4l
Copy link
Member Author

tw4l commented Oct 25, 2023

With #1314 merged in, I think we might be finally go to go on this one!

@tw4l tw4l force-pushed the issue-1261-execution-time-exceeded branch from 489d8b2 to d268922 Compare October 26, 2023 16:10
ikreymer and others added 5 commits October 26, 2023 12:10
- Remove user-set overage
- Just have a 'single execution minutes' quota, rename hard cap to be
the quota (for now)
- have backend compute storage and exec minutes quotas, report to
frontend (will eventually involve additional variables)
@tw4l tw4l force-pushed the issue-1261-execution-time-exceeded branch from 081ee64 to cee0c85 Compare October 26, 2023 16:44
@tw4l
Copy link
Member Author

tw4l commented Oct 26, 2023

@ikreymer @Shrinks99 Pulled in the org dashboard execution time meter into this PR, should all be ready for a last review together now

Copy link
Member

@Shrinks99 Shrinks99 left a comment

Choose a reason for hiding this comment

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

Tested: Meter updating, crawl stopping gracefully after limit reached, frontend behaves properly after limit reached (crawl workflow switch included!)

LGTM! 🚀

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.

At long last, I think this is good to go!

@ikreymer ikreymer merged commit 38f32f1 into main Oct 26, 2023
6 checks passed
@ikreymer ikreymer deleted the issue-1261-execution-time-exceeded branch October 26, 2023 22:38
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.

Execution time: handle monthly execution time exceeded [Feature]: Org Dashboard
4 participants