Repository navigation
feat: cap concurrent requests per FDSN provider at 3 per run - #24
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
A run now keeps at most 3 requests in flight against any single FDSN provider, however many boxes download at once or how many threads each uses. Requests over the limit wait for a slot; nothing fails because of it. Released as 0.6.0.
Why
max_workers(boxes in flight) andthreads_per_client(MassDownloader threads per provider) multiply when overlapping boxes share a provider. At the defaults that is up to 9 concurrent requests at one archive, and up to 30 atMAX_WORKERS_CAP. SCEDC publishes a limit of 3 concurrent sessions and states that exceeding it results in IP blacklisting (source), so any Southern California event could breach it. The source also claimed FDSN providers have "no published rate limit", which was wrong.Lowering a default is not a good fix. On the Shenzhou-15 case, both compliant single-knob settings (
max_workers=3, threads_per_client=1andmax_workers=1, threads_per_client=3) took roughly 200 s against 71 s at the old defaults.How
Clientper provider is already built per run and shared by every box and thread. Right after construction, its three request methods (get_stations,get_stations_bulk,get_waveforms_bulk) are wrapped to take a permit from a per-providerBoundedSemaphore(3). That covers every request a run makes: inventory, availability, waveforms and StationXML.groundtrack.download.Clientmonkeypatch working and passes MassDownloader'sisinstance(..., Client)check.get_waveforms_bulk(attach_response=True)callsself.get_stations, which would otherwise deadlock once every permit is held.PROVIDER_CONCURRENCY_LIMIT = 3lives inproviders.pyand applies uniformly to every provider. The limit is per run, and per server rather than per provider name: permits are keyed by the client'sbase_url, soIRISPH5(a distinct provider on EarthScope's server) shares EarthScope's allowance instead of doubling it.max_workersandthreads_per_clientkeep their defaults and meaning. What is downloaded is unchanged.provider_concurrency_limitfield.max_workerscap warning, README and docs are corrected (no more "connections per data centre" or "no published rate limit").Testing
download_boxes()run with 6 overlapping boxes atmax_workers=4, threads_per_client=3never exceeds 3 in flight and downloads the same stations as a sequential run. Two providers reporting the same server share one limit.mass_downloadersource and fails if it ever calls aClientmethod outside the gated three.Cost
One paired trial on Shenzhou-15:
MassDownloader, its own defaultsIdentical data, about 1.5x slower than the old non-compliant defaults, and roughly time-neutral with plain ObsPy at equal per-provider load while fetching 1.7x the data. Single trial, so treat the ratios as indicative.