Fix batch rate-limit scheduling for partial batches

[hugosmoreira]

Summary

• calculate rate-limit spacing from the number of objects actually sent in the previous batch
• record the timestamp when a rate-limited batch is dispatched, after the scheduler finishes filling it
• preserve future timestamps set by concurrent rate-limit retries
• add focused coverage for full, partial, elapsed, retry-backoff, and empty batches

Root cause

Rate-limited batching used a fixed interval derived from the configured concurrency. A partial batch therefore incurred the same delay as a full batch, which unnecessarily reduced throughput. The request timestamp was also recorded before the scheduler's batch-filling wait.

The new interval is proportional to the previous batch's actual object count:

base_time * objects_sent / requests_per_minute

Full batches retain their existing spacing, while partial batches can proceed sooner without exceeding the configured object rate.

Validation

• pytest test/collection/test_batch.py -q — 16 passed
• pytest test --ignore=test/test_timeout.py -q — 414 passed, 1 skipped
• pytest mock_tests -q — 56 passed
• ruff check weaviate test mock_tests integration — passed
• ruff format --check weaviate test mock_tests integration — passed
• flake8 weaviate test mock_tests integration — passed
• pyright weaviate/collections/batch/base.py — passed
• package build and twine check — passed
• pytest integration/test_batch_v4.py::test_add_objects_in_multiple_batches -vv against Weaviate 1.39.0 — passed
• live two-partial-batch probe against Weaviate 1.39.0 — 2 objects imported; second batch completed in 1.187 seconds instead of the previous approximately 62-second delay
• remaining default-server batch integration coverage — 63 passed, 3 skipped

Two async-indexing integration cases could not run locally because Windows reserves the repository's configured host port 50061. Two unrelated subprocess timeout-harness tests were excluded locally because Windows xdist termination does not forward their expected stderr text; the remaining unit suite passed.

Fixes #1100

[orca-security-eu]

Orca Security Scan Summary

Status	Check	Issues by priority
Passed	Infrastructure as Code	0   0   0   0	View in Orca
Passed	SAST	0   0   0   0	View in Orca
Passed	Secrets	0   0   0   0	View in Orca
Passed	Vulnerabilities	0   0   0   0	View in Orca

[weaviate-git-bot]

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

[hugosmoreira]

I have read and agree to the Weaviate Contributor License Agreement.

[shashvat-singham]

Ran the branch — test/collection/test_batch.py is 16 passed, and the new parametrisation matches what get_sleep_time actually computes.

I simulated the scheduler to check the resulting throughput rather than just the arithmetic, feeding successive batches through get_sleep_time and accumulating the sleeps:

rpm= 1000 batch= 500 -> achieved   972.6 objects/min
rpm= 1000 batch= 100 -> achieved   972.6 objects/min
rpm=  600 batch= 300 -> achieved   583.6 objects/min
rpm= 2000 batch=1000 -> achieved  1945.2 objects/min

which is the intended behaviour: it lands just under the limit (the 62s base rather than 60s buying the buffer the comment mentions), and the achieved rate is now independent of batch size. That last property is the real win here — under the old base_time // concurrent_requests check the effective rate moved with how the batches happened to be partitioned, which is exactly the partial-batch problem in the title. Nice fix.

Two notes:

Docstring disagrees with the code, and this PR makes that concrete. The public parameter is documented as

requests_per_minute: The number of requests that the vectorizer can process per minute.

in both batch/client.py and batch/collection.py, but the implementation has always treated it as objects per minute — the sizing block says so explicitly ("should never send more than the given amount of objects per minute … 3000 objects, 1000/min -> 3 batches of 1000 objects, send every 20 seconds"), and get_sleep_time's base_time * number_objects / requests_per_minute is squarely object-based, as the simulation above confirms. A user who sets rate_limit(requests_per_minute=1000) expecting 1000 HTTP requests gets 1000 objects. Since this PR moves the rate maths into a named, tested method, it seems like a good moment to correct the two docstrings.

First batch has no delay. __num_objects_in_previous_batch starts at 0, so the first get_sleep_time call returns 0 and the first batch dispatches immediately. That looks deliberate (you can't pace on a previous batch that doesn't exist) and matches the "uses_previous_batch_size" framing, but it does mean a burst of concurrent_requests batches can go out before any pacing applies. Worth a one-line comment so it doesn't read as an uninitialised-state bug later.

Minor: __num_objects_in_previous_batch and the max(...) timestamp update are written outside __active_requests_lock, so with concurrent_requests > 1 the "previous batch" is whichever dispatch happened to land last. Probably benign given the pacing is approximate, but flagging it since the max() comment shows concurrent writers are already in mind.

[hugosmoreira]

Thanks @shashvat-singham for running the branch and for the careful review. I addressed the two documentation points in b15db79:

• Both public rate_limit docstrings now define the value as objects sent to Weaviate per minute, matching the implementation and public docs while preserving the existing parameter name.
• The zero previous-batch count now explains that the first dispatch is intentionally immediate because no prior batch exists.

Local verification: test/collection/test_batch.py is 16 passed; Ruff check/format, Flake8, compileall, source-contract checks, and diff hygiene pass.

On the concurrency note, the dispatch loop runs in one BgBatchScheduler thread, so the previous-batch count has a single writer. After the first dispatch it records that batch's size and timestamp, and the next scheduler iteration applies pacing; concurrent_requests allows already-paced requests to overlap rather than releasing an unpaced startup burst. Retry workers can separately update the future timestamp, but I left synchronization behavior unchanged here because that deserves a deterministic regression test and a focused concurrency change rather than an untested lock adjustment in this documentation follow-up.