detect_stops: bound scan work — fix the 100k-point DoS - #65
Merged
Conversation
detect_stops re-grows a spatial cluster from each start sample when the previous one is rejected (too short, or progressive movement). A crafted single giant cluster (a slow drift, or a same-timestamp mass) is never accepted, so the scan runs O(n*cluster_size) — ~70s of CPU at the 100k-point cap, from one unauthenticated POST /v1/insights (an availability/DoS hole the 30/min rate limit doesn't close). Progressive-movement rejection (ADR 0014) widened it. Two bounded, real-data-safe guards (ADR 0016): - Zero-duration skip (exact): a cluster whose samples share one timestamp can never be a stop, so it's skipped wholesale, not re-grown. 100k same-ts body: ~70s -> ~0.8s. Never affects a real track. - Absolute scan-work budget (_MAX_STOP_SCAN_WORK=12M, ~5s), where work counts grown samples + the window size of every exact-radius recompute (the real time driver). compute_insights catches StopScanBudgetExceeded and degrades gracefully: no stops (nor trips) + a quality note, but still returns distance, speed, bbox, hotspots. Absolute (not per-point) so it never false-positives a small dense track whose absolute time is fine. Verified: 100k slow-drift 73s -> 5.5s (skipped+noted), 100k same-ts -> 0.8s, 41k real drive+stop+drive 0.77s (1 stop), 6k dense 0.62s — both unaffected. Full suite: +4 DoS tests, zero new failures. Residual (flagged): ~5s is a bound, not a true O(n) fix. Follow-up = an O(n) sliding-window detect_stops (hard) or lowering MAX_PUBLIC_POINTS (product call). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Bound
detect_stopsscan work — the 100k-point DoSThe availability hole found while measuring #64:
detect_stopsre-grows a spatial cluster from each start sample when the previous one is rejected. A crafted single giant cluster (a very slow drift, or a same-timestamp mass) is never accepted, so the scan runs O(n·cluster_size) — ~70 s of CPU at the 100k-point cap, from one unauthenticatedPOST /v1/insights(the 30/min rate limit doesn't close it). Progressive-movement rejection (#63) widened it.Two bounded, real-data-safe guards (ADR 0016):
12Munits, ≈ 5 s), where work counts grown samples + the window size of every exact-radius recompute (the real time driver).compute_insightscatches it and degrades gracefully — no stops (nor trips) + aqualitynote, but still returns distance, speed, bbox, hotspots. Absolute (not per-point) so it never false-positives a small dense track whose absolute time is fine.Verified
Full suite: +4 DoS tests, zero new failures.
~5 s is a bound, not a real O(n) fix — a provably-O(n)
detect_stopsneeds an incrementally-maintained centroid spread (dynamic farthest-point) or a results-changing metric, out of proportion here. Follow-up options: the O(n) sliding-window rewrite, or loweringMAX_PUBLIC_POINTS(a product call) to shrink the absolute worst case. Noted in ADR 0016 + memory.