Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch: support rate limiting #133

Merged
merged 4 commits into from
Dec 30, 2021

Conversation

mhuang74
Copy link
Contributor

@mhuang74 mhuang74 commented Dec 30, 2021

Add rate limiting support to Fetch (Issue #77 )

  • use linkedin 30 qps upper limit as reference rate limit
  • believe 30 qps can be reached via single thread, so current implementation uses just one thread
  • sleeps 10ms when rate limited. haven't fully tested this out, but believe this should still allow 30 qps
  • to reach higher qps using more threads, may need to first figure out how to keep output rows in same order as input

Integration test uses Actix to simulate web api with 2 qps limit. So it takes 10 sec to request 20 URLs.

mhuang@twisted-linen:/usr/local/projects/mhuang/rust/qsv (fetch-rate-limiting)$ QSV_LOG_LEVEL=debug RUST_BACKTRACE=1 cargo test fetch::fetch_ratelimit
    Finished test [optimized + debuginfo] target(s) in 0.40s
     Running unittests (target/debug/deps/qsv-e24fec371759ce4e)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/tests.rs (target/debug/deps/tests-84caf56d8c8e9a11)

running 1 test
test test_fetch::fetch_ratelimit ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 616 filtered out; finished in 9.03s

@jqnatividad
Copy link
Collaborator

Thanks @mhuang74 !

  • use linkedin 30 qps upper limit as reference rate limit

Rate limit is definitely better than throttle for dealing with real-world web services.

  • believe 30 qps can be reached via single thread, so current implementation uses just one thread

And at this stage, multi-threading is somewhat mismatched with the reality of the same services, and we can leave it for a future implementation.

  • sleeps 10ms when rate limited. haven't fully tested this out, but believe this should still allow 30 qps

10 ms seems to be a reasonable default... we can always tweak it or maybe even make it a configurable parameter if required.

  • to reach higher qps using more threads, may need to first figure out how to keep output rows in same order as input

Agreed. You may want to check out if indexmap can help. It certainly did with the test-data-generation crate which enables the generate command. #90

@jqnatividad jqnatividad merged commit 6143e27 into dathere:master Dec 30, 2021
@mhuang74 mhuang74 deleted the fetch-rate-limiting branch January 5, 2022 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants