Bound the dependents/suggesters listing queries with a statement timeout - #1842
Open
Seldaek wants to merge 1 commit into
Open
Bound the dependents/suggesters listing queries with a statement timeout#1842Seldaek wants to merge 1 commit into
Seldaek wants to merge 1 commit into
Conversation
Both listings materialise every row for the required package name in a derived table with no LIMIT, join package, and sort the whole joined set before applying LIMIT/OFFSET, so the cost is proportional to the total number of dependents regardless of which page is asked for. There was no statement timeout anywhere - the only timeout configured for Doctrine is PDO::ATTR_TIMEOUT, which is the connect timeout - so a pathological sort held a PHP-FPM worker until the gateway killed it, up to 37s observed. This is a stability guard rather than a throughput win: 99.9% of these requests finish within 1.3s, so the aggregate saving is small. What it buys is bounding worker occupancy, which matters for surviving the loss of one node at peak. 5s sits far above the p99.9, so the requests it cuts short are the ones already holding workers longest. MySQL reports the abort as error 3024, which Doctrine surfaces as a DriverException, so both actions now degrade to a 503 instead of a 500.
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
1 participant
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.
Both listings materialise every row for the required package name in a derived table with no
LIMIT, joinpackage, then sort the whole joined set before applyingLIMIT/OFFSET— so the cost is proportional to the total number of dependents regardless of which page is asked for. There was no statement timeout anywhere (the only timeout configured for Doctrine isPDO::ATTR_TIMEOUT, which is the connect timeout), so a pathological sort held a PHP-FPM worker until the gateway killed it — up to 37s observed.On whether this causes problems
Worth being straight about the size of the win: this is a stability guard, not a throughput win. Per the percentile data (396k requests: p50 9ms, avg 26ms, p95 84ms, p99 321ms, p99.9 1.29s, max 37.1s), the band above 5s is a handful of requests, so the aggregate CPU saving is negligible. What it buys is bounding worker occupancy, which matters for surviving the loss of one node at peak rather than for average CPU.
5s sits far above p99.9, so the requests it cuts short are precisely the ones already holding workers longest. A request that would have completed in 6s now errors — that's the trade.
Also correcting my earlier read of this endpoint: the failures on it are 403s from the
page > 3anonymous gate, i.e. that guard working as designed on crawlers. They were never timeouts.Degradation
MySQL reports the abort as error 3024, which Doctrine surfaces as a
DriverException. Both actions now return a 503 (JSON for the.jsonformat, plain text otherwise) instead of letting it become a 500.Verification
composer phpstanclean (phpstan-dba validates the hinted SQL), full suite green. New repository tests execute the hinted queries against real MySQL 8.4 — an invalid hint would be a syntax error, not a silently ignored comment — and cover ordering, the type filter and pagination. New controller tests cover the 503 degradation across both actions and both formats; confirmed they fail without the catch.