Skip to content

fix(rate_limiter): enforce spacing between concurrent requests - #1030

Draft
u7-u7 wants to merge 1 commit into
modelscope:mainfrom
u7-u7:fix/rate-limiter-concurrent-spacing
Draft

u7-u7 wants to merge 1 commit into
modelscope:mainfrom
u7-u7:fix/rate-limiter-concurrent-spacing

Conversation

@u7-u7

@u7-u7 u7-u7 commented Oct 28, 2026

Copy link
Copy Markdown

Change Summary

Enforce min_request_interval between concurrent callers, not only sequential ones.

The minimum-interval check currently runs before acquiring the request-pacing lock. With max_concurrent > 1, several callers can read the same previous start time and sleep until the same deadline, then all enter almost simultaneously. Through the installed SDK, four requests configured for 100 ms spacing started at approximately 0 / 100 / 100 / 101 ms. The basic limiter used by FinancialDataFetcher exposes both concurrency and minimum-interval settings, so this can exceed a data provider's configured pacing.

Move the interval calculation and wait under the existing async lock, alongside the requests-per-second check and timestamp update. Request bodies remain outside the lock and can still overlap. No public API or default changes.

This does not change the synchronous bookkeeping in #923, interrupted context entry in #1016, or keyword argument forwarding in #1027.

Related issue number

No matching issue or PR was found in the all-state duplicate search.

Validation

CPython 3.12.14, normal package imports from an editable source install, no external service calls or rate-limiter mocks:

  • New tests on unchanged main: 2 failed, 2 passed. Concurrent requests had 13–19 microsecond gaps despite a configured 50 ms interval.
  • With the fix: 4 passed. Covers sequential/concurrent pacing, overlapping request bodies, and the existing requests-per-second cap.
  • Pacing, retry-classification and atomic-file focused subset: 32 passed.
  • python -m pytest tests/utils -q: 151 passed, 30 subtests passed, 1 unrelated baseline failure. The unchanged transcript test expects subagents/worker-1.jsonl, while the implementation writes .ms_agent/subagents/worker-1.jsonl. The same failure was reproduced before the source edit.
  • pre-commit install completed. All applicable changed-file pre-commit hooks passed, as did compilation and git diff --check.
  • Full-repository pre-commit was run on both baseline source and final source. Both report existing lint/formatting errors in unrelated files. All unrelated auto-formatting changes were discarded; no full-repository lint-pass claim.

The full repository test suite, real provider calls, other Python versions, and non-Linux platforms were not run.

Checklist

  • The title summarizes the change.
  • Regression tests cover the change.
  • Full-repository pre-commit passes (existing baseline failures described above).
  • Existing API documentation remains accurate; no new command or option.

AI assistance: prepared and tested with OpenAI Codex. No human-review attestation is claimed; submitted as a draft without requesting reviewers.

This branch has not been deployed

No deployments
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.

1 participant