This repository has been archived by the owner on Jul 23, 2024. It is now read-only.
Reduce API calls needed to discover apps & services #29
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The API affordances really encourage writing chatty clients 😕 |
richardTowers
force-pushed
the
reduce-number-api-calls-per-refresh
branch
2 times, most recently
from
4804aaf
to
0c77d4c
Compare
These tests demonstrate the client's current behaviour of making one API call per application and per service. I've used jarcoal/httpmock for the tests here - we could use httptest's Server instead, but the mock one makes our life marginally easier. Future commits will refactor the code so it makes fewer API calls.
Previously this made two API calls for every application the user was able to see: - 1 to get the Space - 1 to get the Org Some orgs have large numbers of apps (65 apps in one org -> 130 API calls). Because there are no sleeps between calls, this ends up putting a lot of traffic through the API all at once, which then gets queued. Once API requests start getting queued, the healthcheck starts failing (because the healthcheck requests time out while they're in the queue). Once the healthcheck starts failing the API servers start getting taken out of the load balancer, at which point we start having Bad Vibes. This commit changes the API calls we make so that there's one call to get all the orgs the user can see, one to get all the spaces the user can see, and one to get all the apps the user can see. Because this doesn't vary with the number of apps the performance should be much more predictable.
richardTowers
force-pushed
the
reduce-number-api-calls-per-refresh
branch
from
0c77d4c
to
5325814
Compare
richardTowers
changed the title
tlwr
approved these changes
Jun 17, 2019
AP-Hunt
added a commit
to alphagov/paas-cf
that referenced
this pull request
Oct 31, 2019
The previous limit in Ireland - 55k - was deliberately inflated to accommodate a very heavy usage deployment of `paas-prometheus-exporter` that was running the region. Since then, the exporter has been made less chatty (alphagov/paas-prometheus-exporter#29), causing the deployment in question to drop to a more manageable number of calls per hour. However, we forgot to reduce the rate limit once the fix was in place. This commit resolves that outstanding action. The figure of 20k/h is informed by this Kibana chart [1], which shows the top non-admin CC api caller to be making 15k/h. I've chosen 20k here to allow some wiggle room. [1] https://kibana.logit.io/s/665ca355-efc3-46a2-96cf-21d31a5305bb/goto/8e10b7eb77da26e444553501aa70f56a
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Previously this made two API calls for every application the user was
able to see:
Some orgs have large numbers of apps (65 apps in one org -> 130 API
calls).
Because there are no sleeps between calls, this ends up putting a lot of
traffic through the API all at once, which then gets queued.
Once API requests start getting queued, the healthcheck starts failing
(because the healthcheck requests time out while they're in the queue).
Once the healthcheck starts failing the API servers start getting taken
out of the load balancer, at which point we start having Bad Vibes.
This PR changes the API calls we make so that there's one call to
get all the orgs the user can see, one to get all the spaces the user
can see, and one to get all the apps the user can see. Because this
doesn't vary with the number of apps the performance should be much more
predictable.
I've also added some tests that mock the HTTP traffic so it's more obvious
what API calls our client makes (this should be enough to prevent a regression,
or someone accidentally making a new API call using the overly chatty go-cfclient).
How to review