fix(fetch): bound EDGAR fetches in flight, not just their start rate - #287
Merged
Conversation
`sec spac process` over a list of CIKs exhausts the process's file descriptor table once it leaves the already-downloaded issuers. The fetch queue caps STARTS per second and nothing else. Its worker dispatches each claimed job in the background and immediately loops for the next, and the rate limiter's window is pruned by age rather than by completion, so a slot frees one second after a fetch begins no matter how long it runs. In-flight work is therefore `rate x latency`: fine while EDGAR is sub-second, but a slow spell serving multi-MB full-submission `.txt` documents at 30s each admits ~240 concurrent requests. Each holds roughly two descriptors and the pool only releases them after an idle period, so the peak is what runs the table dry — measured at ~418 descriptors for 200 concurrent fetches, which crosses macOS's default `ulimit -n` of 256 at about 128. It is not a leak: at a fixed concurrency the count is flat across rounds and returns to baseline once the pool goes idle. Only the unbounded peak needed capping, so add a ConcurrencyLimiter that holds its slot until the job reaches a terminal state. It sits ahead of the rate limiters in the composite because it is an in-process counter — a claim it rejects under saturation costs nothing, where acquiring the cluster limiter first would spend a reserve/release round trip against Postgres on every rollback. Default 16 (SEC_FETCH_MAX_CONCURRENT, clamped 1..64) so the cap binds only once a fetch averages over two seconds: a healthy sweep runs at exactly the speed it does today, and a degraded EDGAR costs throughput instead of the whole process. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SUkk4LNNdcYUD2v1FMvkvL
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.
sec spac process <ciks...>exhausts the process's file-descriptor table once it leaves the already-downloaded issuers.Cause
The fetch queue caps starts per second and nothing else.
JobQueueWorkerdispatches each claimed job in the background and immediately loops for the next, and the rate limiter's window is pruned by age rather than by completion, so a slot frees one second after a fetch begins no matter how long it runs.In-flight work is therefore
rate x latency. While EDGAR is sub-second that is ~8; a slow spell serving multi-MB full-submission.txtdocuments at 30s each admits ~240 concurrent requests. Each holds roughly two descriptors and the pool releases them only after an idle period, so the peak is what runs the table dry.Measured on Bun 1.3.11, driving the real
safeFetchagainst a slow local server:At ~2 descriptors per in-flight request, macOS's default
ulimit -nof 256 is crossed at about 128 concurrent fetches.This is not a leak: at fixed concurrency the count is flat across repeated rounds (98 → 98 over 10 rounds) and returns to baseline after ~20s idle. Only the unbounded peak needed capping.
Cache-hit issuers never fetch, which is why a single already-downloaded issuer does not reproduce it.
Fix
Add a
ConcurrencyLimiterto the queue'sCompositeLimiter. Unlike the rate limiters it holds its token until the job reaches a terminal state, which is what makes it a concurrency bound rather than a second rate cap.It sits ahead of the rate limiters: it is an in-process counter, so a claim it rejects under saturation costs nothing, whereas acquiring the cluster limiter first would spend a reserve/release round trip against Postgres on every rollback.
Default 16 via
SEC_FETCH_MAX_CONCURRENT(clamped 1–64), so the cap binds only once a fetch averages over two seconds — a healthy sweep runs at exactly today's speed, and a degraded EDGAR costs throughput instead of the whole process.The two limits are independent and both are needed. The rate limiter cannot be derived from the concurrency one (
λ = C/Wfloats with the peer's latency: at C=16 against a 50ms response that is ~320 req/s, well past EDGAR's ceiling), and their scopes differ — the EDGAR quota is per-IP and cluster-shared, descriptors are per-process.Behaviour is identical on both backends:
PostgresRateLimiterStoragecountsWHERE executed_at > now - windowMs, the same age-pruned window on starts as the in-memory path.CompositeLimiter.scopewas already"process"(EvenlySpacedRateLimiteris process-scoped), so no start-up warning changes.Testing
SecFetchConcurrency.test.tsdrives the real queue against a slow server and asserts the peak never exceeds the cap, with a lower bound so a silently-serialized queue cannot pass vacuously. Verified it fails without the fix:bunx tsc --noEmit— cleansrc/task/fetch,src/task/spac,src/config— 169 passed, 9 skippedNote
While measuring this I confirmed a separate, unrelated defect in libs: under Bun,
undici'sdispatcheroption is ignored, soSafeFetch.server.ts's DNS-rebinding connection pin is a silent no-op on the runtime this CLI ships on. Filed as workglow-dev/libs#789 — it is a security issue, not a resource one, and does not affect this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01SUkk4LNNdcYUD2v1FMvkvL
Generated by Claude Code