feat: move rate limiting to the object store

[westonpace]

Closes #6239

[westonpace]

Keeping in draft until #6266 merges

[github-actions]

PR Review

Well-designed change that moves rate limiting to the right layer. The AIMD algorithm, per-category throttles, and anti-thundering-herd token bucket are solid. A few concerns:

P0: Breaking change without deprecation

Removing LANCE_PROCESS_IO_THREADS_LIMIT is a silent breaking change for users who have configured it. Consider:

• Logging a warning at startup if LANCE_PROCESS_IO_THREADS_LIMIT is still set, telling users to migrate to the new LANCE_AIMD_* env vars
• Or keeping it as a no-op with a deprecation warning for one release cycle

P1: is_throttle_error string matching is fragile

The heuristic of matching "retries, max_retries" in error messages is acknowledged as crude, but deserves more attention:

• If object_store changes its error format, throttle detection silently breaks and AIMD never decreases rate — the worst failure mode
• Consider also logging/counting unrecognized Generic errors that don't match the pattern, so operators can notice if the heuristic stops matching
• Worth adding a code comment with the object_store version this was tested against so future maintainers know when to re-verify

P1: ThrottledMultipartUpload::put_part — token acquired after future is created

fn put_part(&mut self, data: PutPayload) -> UploadPart {
    let fut = self.target.put_part(data);  // called eagerly
    Box::pin(async move {
        write.acquire_token().await;       // throttle happens later
        let result = fut.await;
        ...
    })
}

self.target.put_part(data) is called before the token is acquired. If an underlying store implementation starts I/O eagerly inside put_part (rather than deferring to the returned future), the throttle is bypassed. This is likely fine for current object_store implementations but is subtle — a brief comment explaining the assumption would help.

Minor nits (non-blocking)

• AimdController uses std::sync::Mutex with .unwrap() — a poisoned mutex will panic. Fine in practice but parking_lot::Mutex avoids this class of issue.
• observe_outcome silently drops rate updates via try_lock when contended. The doc comment explains this, which is good — just noting that under sustained throttling with high contention, the rate decrease could be delayed.

Overall this is a solid improvement over the old semaphore-based approach. The per-store scoping, per-category isolation, and AIMD adaptiveness are all well-motivated by the linked issue.

[codecov]

Codecov Report

❌ Patch coverage is 61.56584% with 108 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance-io/src/object_store/throttle.rs	60.58%	91 Missing and 17 partials ⚠️

📢 Thoughts on this report? Let us know!

[Xuanwo]

client_max_retries is not useful anymore, do we need to remove it?

[westonpace]

It should still be used. We actually rely on object_store's retries to make the AIMD throttle work. The object_store errors have no "is_temporary" so we have no other way of knowing if an error is a temporary error except to see if object_store applied retries to it.

So by default we should do 3 object store retries now. Each time those fail we apply an AIMD retry (longer backoff, cut throttle). So in total we get 9 retries instead of 10.

[Xuanwo]

Seems multipart upload process is not covered. Is it expected?

[westonpace]

Good catch. I've added multipart to the retry handling. I still can't apply the outer retry loop to the delete stream or list stream methods. These return a stream of items and there is no way of mapping results to underlying object store requests so there is nothing I can retry there. Since these operations are hopefully rare I think it will be ok.

[Xuanwo]

I think we should consider overriding our own rename_if_not_exists handling.

Currently, the logic retries the entire copy and delete process. If the copy succeeds but the delete fails due to throttling, the entire operation is retried. After that, the copy will fail because the object already exists.

[westonpace]

I'll create a follow-up, I don't think we use rename_if_not_exists in most cases anymore.

[westonpace]

Test results:

Test Description

On Azure, 40 VMs (8 cores each) hammer the same blob with random takes, each take grabbing 1024 rows. Each VM has 8 processes so there are a total of 320 processes taking from the same blob. This blob is in a standard class storage account. This should aggressively trigger rate limiting errors. Everything runs for 5 minutes.

With AIMD disabled, previous settings (note, red line is success in this chart for some reason)

8,330 failed takes, 50,176 rows taken

With AIMD enabled, default settings

0 Errors, 9,951,232 rows taken

Aggressive AIMD (1 object_store retry, 5 AIMD retries)

0 Errors, 7,606,272 rows taken

(will be updated with additional results)

[Xuanwo]

Thank you!