Skip to content

bug: commit-retry sleep has no timeout wrapper, unlike the write-retry path #7882

Description

@LuciferYang

Summary

The commit-retry loop in commit_transaction sleeps between attempts with a bare tokio::time::sleep(backoff.next_backoff()) (rust/lance/src/io/commit.rs:1116) that has no timeout wrapper. The write-retry path in execute_with_retry already wraps its sleep via maybe_timeout against retry_timeout (rust/lance/src/dataset/write/retry.rs:113-115), so the two retry paths behave differently.

SlotBackoff's unit is derived from the first attempt's latency (unit = first_attempt_ms * 11 / 10, commit.rs:1104), and the backoff is proportional to it. When the first commit attempt is slow (a large manifest, many indices, or a high-latency object store), a single sleep on the commit path can run for minutes, and there is no deadline to cut it short.

Impact

Under sustained write contention with a slow first attempt, the commit path can spend several minutes in a single sleep, or a long time across the full retry budget, with no way to bound the wall-clock. A caller that wants to cap how long a commit may block cannot, because the sleep ignores any deadline.

Suggested fix

Wrap the commit-path sleep in a timeout the same way execute_with_retry does, so the retry loop respects a wall-clock budget regardless of the observed first-attempt latency.

Note

The per-sleep duration is separately bounded by the SlotBackoff slot cap ((MAX_SLOTS - 1) * unit), but that bound is proportional to unit, not absolute. A timeout wrapper is what bounds wall-clock independently of unit.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions