Repository navigation
Fail fast on oversized geoid requests and add a statement timeout for interactive endpoints - #101
Merged
Merged
Conversation
…urn 503 on timeout
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why
The origin VM was pegged on CPU (netdata alerts). Cloudflare analytics show most uncached
/1.0/data/show/*traffic that reaches the origin is 400s averaging 2-3s of origin time each, plus a few dozen requests that run ~125s and end as Cloudflare 524s.The 400s come from requests with 10-15
sumlevel|parent_geoidgroups (e.g.040|01000US,950|01000US,150|01000US,...). Each group was expanded against Postgres, and forlatestthe whole thing was repeated for every release, before the request was finally rejected for exceedingMAX_GEOIDS_TO_SHOW.Changes
expand_geoids: takes the endpoint'smax_geoids, dedupes repeated entries, and aborts with a 400 as soon as the running total goes over the limit. Used bydata/show,data/downloadandgeo/show.KNOWN_LARGE_EXPANSIONSlookup: expansions we already know are huge (e.g. block groups or tracts for the nation, block groups for large states) are rejected without querying. Counts are compared against the configured limit, apply to all releases, and are documented as an optional refresh inDATA_UPDATES.md.SET LOCALin abefore_requesthook (so it can't leak through the connection pool)./1.0/data/download/,/1.0/aggregate/and/1.0/user_geo/are exempt, andINTERACTIVE_STATEMENT_TIMEOUT_MS = 0disables it. A cancelled query now returns a JSON 503 instead of a 500.Things to watch after deploy
data/show,geo/show,geo/search,table/search) into 503s. Check the 503 rate and raiseINTERACTIVE_STATEMENT_TIMEOUT_MSif it's too aggressive.Testing
Unit tests for the early abort, dedupe, known-large lookup, timeout hook and 503 handler (35 pass with the existing tests). Not yet exercised against a real database.