fix(rate_limit): raise regex match timeout to 50ms

Nidhey Indurkarrequested to merge
nindurkar/raise-regex-match-timeout-50ms into master

What

Raises Matcher::MATCH_TIMEOUT_SECONDS from 5ms to 50ms, and replaces the benchmark note above it. Agreed with @reprazent in Slack.

Why

The 5ms budget fired 9,511 times in the 24h after !338 (merged) deployed, and has run at roughly 624/hour since. Every one fails open, so those requests go unlimited.

The matches are not the slow part. Benchmarked against the 6935 distinct real GitLab.com paths that timed out, using the production pattern:
Median match 0.087 ms
Worst match 1.484 ms
Paths above 2 ms 0

Ruby's regex timeout counts wall-clock time, not CPU time. Confirmed directly: a match needing 0.92s of CPU under a 2s timeout completes, but freezing the process for 3s mid-match raises after 3.27s wall clock having used 0.26s of CPU. So a match billed for a stop-the-world pause exceeds its budget without doing regex work, and GC pauses on GitLab.com's git fleet have a p50 of ~72ms, well over the old 5ms budget.

Why 50ms

Re-running the same corpus under 4x CPU oversubscription (16 cores, 64 busy loops):

Budget Matches timing out
idle, any budget 0
5 ms 1.33%
10 ms 1.36%
25 ms 0.61%
50 ms 0.16%

The stalls are scheduler-scale, so 10ms and 20ms buy nothing. 50ms is 34x the slowest real path, cuts the fail-opens roughly 8x, and still bounds catastrophic backtracking, which !338 (merged) showed none of our patterns can produce anyway.

It does not eliminate them. A median GC pause exceeds 50ms too. The complementary fix is to stop evaluating the expensive pattern on requests that obviously cannot match, tracked in gitlab-com/gl-infra/production-engineering#29581.

The comment it replaces

The old note claimed 5ms was "~119x the slowest real match ... worst: 0.0419ms". That measurement never included Gitlab::PathRegex.repository_git_lfs_route_regex, which is the pattern that actually times out in production, so the stated headroom was wrong.

Testing

No spec changes needed: the timing assertions derive from the constant (matcher_spec.rb:379, evaluator_spec.rb:588) rather than hardcoding 5ms, so they scale.

bundle exec rspec spec/labkit/rate_limit/ passes, 345 examples, and rubocop is clean on the changed file.

Related

Merge request reports