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
scheduled task to conditionally scale up the houndigrade cluster #229
Conversation
Codecov Report
@@ Coverage Diff @@
## master #229 +/- ##
=========================================
+ Coverage 98.94% 99.15% +0.2%
=========================================
Files 48 50 +2
Lines 1807 2012 +205
Branches 94 99 +5
=========================================
+ Hits 1788 1995 +207
+ Misses 13 11 -2
Partials 6 6
Continue to review full report at Codecov.
|
cloudigrade/config/settings/base.py
Outdated
) | ||
HOUNDIGRADE_AWS_VOLUME_BATCH_SIZE = env.int( | ||
'HOUNDIGRADE_AWS_VOLUME_BATCH_SIZE', | ||
default=5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not want to do 32 like we originally talked about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. 👍 I accidentally overlooked that requirement.
) | ||
|
||
if len(messages) == 0: | ||
# Quietly exit and let a future run check for messages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is the right time, but I'd love to see some debug logging happen in situations like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd considered about the same thing when I wrote this. I think I'll add some in now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
info logs added in the latest commits. I figure start louder at info and we can lower it to debug when we decide it's too noisy.
cloudigrade/config/settings/base.py
Outdated
CELERYBEAT_SCHEDULE = { | ||
'scale_up_inspection_cluster_every_5_min': { | ||
'task': 'account.tasks.scale_up_inspection_cluster', | ||
'schedule': 60 * 5, # seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding timing, I think we originally were talking about trying to run the cluster once every 30 mins or an hour, maybe have it be configurable? Either way 5 minutes seems awfully small with how long snapshot copy operations can take.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing request changes on here until individual comments are resolved.
42edf6e
to
cc237c1
Compare
… because pypy3 doesn't (yet?) support the former
8d6da12
to
d5f3cae
Compare
Recorded demos here: #176 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the test for the limit 👍
https://asciinema.org/a/178750 - limit message dequeue to batch size https://asciinema.org/a/178752 - successful scale up https://asciinema.org/a/178755 - empty queue exits https://asciinema.org/a/178767 - non-zero scale exits cloudigrade/cloudigrade#229 cloudigrade/cloudigrade#176
#176
This PR adds our first support for scheduled Celery tasks using beat. As described in #176, this task checks the current cluster auto scaling size, checks the message queue for volume IDs, scales up to 1, and runs a (currently stubbed-out) async task that will be responsible for actually "running" houndigrade in the cluster.
Note: this PR builds upon the
organize-aws-helpers
branch and needs to merge after #224 merges.Demo: coming soon!