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

Rate throttle git push heroku master #98

Merged
merged 1 commit into from
Aug 3, 2020

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Aug 3, 2020

When an app is attempted to be deployed it can fail due to an API rate limit like this:

     Hatchet::App::FailedDeployError:
       Could not deploy 'hatchet-t-3ae0c484c6' (rails5_ruby_schema_format) using 'Hatchet::GitApp' at path: 'repo_fixtures/repos/ci/rails5_ruby_schema_format'
       if this was expected add `allow_failure: true` to your deploy hash.
       output:
       Buildpack: nil
       Repo: https://git.heroku.com/hatchet-t-3ae0c484c6.git
       remote: Compressing source files... done.        
       remote: Building source:        
       remote: 
       remote: !    Your account reached the API rate limit Please wait a few minutes before making new requests        
       remote: 
       To https://git.heroku.com/hatchet-t-3ae0c484c6.git
        ! [remote rejected] master -> master (pre-receive hook declined)
       error: failed to push some refs to 'https://git.heroku.com/hatchet-t-3ae0c484c6.git'

This can happen at both the initial deploy and at the release phase. Currently this rate limit case is unhandled. We are using the general/generic retry mechanism of hatchet to account for this case.

This is not great as the retry behavior has no sleep/backoff behavior and will quickly hit the limit again. Also it only retries if the app failed to deploy, if the allow_failure flag is set it will not be retried but any assertions on the output will fail since the app never actually deployed.

This PR proposes that we hook into the existing PlatformAPI rate limits. This is also done manually at

PlatformAPI.rate_throttle.call do
Excon.put(source_put_url,
expects: [200],
body: File.read('slug.tgz'))
end
end
.

The only difference is that the PlatfromAPI.rate_throttle interface expects a response object that includes a status that returns an integer and headers that returns a hash. We can re-use this logic by generating our own FakeResponse object. In the case where the output matches "reached the API rate limit" then we will tell the rate throttle to sleep and try again (by using 429 response code). When it succeeds, we will tell it we got a successful response (200 status code).

This logic will prevent a failure from rate limiting while git push heroku master is being called. Since the rate throttling rate is tuned as the process is running (the sleep value is remembered and adjusted) by using the same rate throttle mechanism here a rate throttle will inform the rest of our system.

@schneems schneems force-pushed the schneems/rate-throttle-deploys branch 2 times, most recently from edd096e to 36c41ee Compare August 3, 2020 16:42
When an app is attempted to be deployed it can fail due to an API rate limit like this:

```
     Hatchet::App::FailedDeployError:
       Could not deploy 'hatchet-t-3ae0c484c6' (rails5_ruby_schema_format) using 'Hatchet::GitApp' at path: 'repo_fixtures/repos/ci/rails5_ruby_schema_format'
       if this was expected add `allow_failure: true` to your deploy hash.
       output:
       Buildpack: nil
       Repo: https://git.heroku.com/hatchet-t-3ae0c484c6.git
       remote: Compressing source files... done.        
       remote: Building source:        
       remote: 
       remote: !    Your account reached the API rate limit Please wait a few minutes before making new requests        
       remote: 
       To https://git.heroku.com/hatchet-t-3ae0c484c6.git
        ! [remote rejected] master -> master (pre-receive hook declined)
       error: failed to push some refs to 'https://git.heroku.com/hatchet-t-3ae0c484c6.git'
```

This can happen at both the initial deploy and at the release phase. Currently this rate limit case is unhandled. We are using the general/generic retry mechanism of hatchet to account for this case.

This is not great as the retry behavior has no sleep/backoff behavior and will quickly hit the limit again. Also it only retries if the app failed to deploy, if the `allow_failure` flag is set it will not be retried but any assertions on the output will fail since the app never actually deployed. 

This PR proposes that we hook into the existing PlatformAPI rate limits. This is also done manually at https://github.com/heroku/hatchet/blob/a8d2c2f0a6a387c70981c1429c24b197baa6c9f9/lib/hatchet/test_run.rb#L184-L189.

The only difference is that the `PlatfromAPI.rate_throttle` interface expects a response object that includes a `status` that returns an integer and `headers` that returns a hash. We can re-use this logic by generating our own `FakeResponse` object. In the case where the output matches "reached the API rate limit" then we will tell the rate throttle to sleep and try again (by using  429 response code). When it succeeds, we will tell it we got a successful response (200 status code).

This logic will prevent a failure from rate limiting while `git push heroku master` is being called. Since the rate throttling rate is tuned as the process is running (the sleep value is remembered and adjusted) by using the same rate throttle mechanism here a rate throttle will inform the rest of our system.
@schneems schneems force-pushed the schneems/rate-throttle-deploys branch from 36c41ee to 5bdd2b7 Compare August 3, 2020 16:54
@schneems schneems merged commit eb0b1e9 into main Aug 3, 2020
@schneems schneems deleted the schneems/rate-throttle-deploys branch August 3, 2020 17:00
@schneems schneems mentioned this pull request Aug 3, 2020
schneems added a commit that referenced this pull request Aug 3, 2020
Building off of #98 we now also throttle calls to `heroku run`.
schneems added a commit to heroku/heroku-buildpack-python that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)
schneems added a commit to heroku/heroku-buildpack-php that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)
edmorley pushed a commit to heroku/heroku-buildpack-python that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)

[skip changelog]
schneems added a commit to heroku/heroku-buildpack-jvm-common that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)
schneems added a commit to heroku/heroku-buildpack-scala that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)
schneems added a commit to heroku/heroku-buildpack-clojure that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)
schneems added a commit to heroku/heroku-buildpack-java that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)
schneems added a commit to heroku/heroku-buildpack-nodejs that referenced this pull request Aug 11, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)
dryan pushed a commit to dryan/heroku-buildpack-python that referenced this pull request Nov 19, 2020
- ActiveSupport's Object#blank? and Object#present? are no longer provided by default (heroku/hatchet#107)
- Remove deprecated support for passing a block to `App#run` (heroku/hatchet#105)
- Ignore  403 on app delete due to race condition (heroku/hatchet#101)
- The hatchet.lock file can now be locked to "main" in addition to "master" (heroku/hatchet#86)
- Allow concurrent one-off dyno runs with the `run_multi: true` flag on apps (heroku/hatchet#94)
- Apps are now marked as being "finished" by enabling maintenance mode on them when `teardown!` is called. Finished apps can be reaped immediately (heroku/hatchet#97)
- Applications that are not marked as "finished" will be allowed to live for a HATCHET_ALIVE_TTL_MINUTES duration before they're deleted by the reaper to protect against deleting an app mid-deploy, default is seven minutes (heroku/hatchet#97)
- The HEROKU_APP_LIMIT env var no longer does anything, instead hatchet application reaping is manually executed if an app cannot be created (heroku/hatchet#97)
- App#deploy without a block will no longer run `teardown!` automatically (heroku/hatchet#97)
- Calls to `git push heroku` are now rate throttled (heroku/hatchet#98)
- Calls to `app.run` are now rate throttled (heroku/hatchet#99)
- Deployment now raises and error when the release failed (heroku/hatchet#93)

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

Successfully merging this pull request may close these issues.

2 participants