Skip to content

fix: lance error TooMuchWriteContention should be converted to a retryable namespace error.#6177

Open
wojiaodoubao wants to merge 1 commit intolance-format:mainfrom
wojiaodoubao:too-much-write-contention-should-be-retryable
Open

fix: lance error TooMuchWriteContention should be converted to a retryable namespace error.#6177
wojiaodoubao wants to merge 1 commit intolance-format:mainfrom
wojiaodoubao:too-much-write-contention-should-be-retryable

Conversation

@wojiaodoubao
Copy link
Contributor

In semantics, I think TooMuchWriteContention should be converted to NamespaceError::Throttled. Because TooMuchWriteContention is caused by RetryableCommitConflict, which means too many concurrent commits and can be retried, just like CommitConflict.

Currently, TooMuchWriteContention and IncompatibleTransaction are both converted into NamespaceError::ConcurrentModification. Since RetryableCommitConflict should be retry and IncompatibleTransaction should not, the caller can't distinguish whether the ConcurrentModification error should be retried

@github-actions github-actions bot added the bug Something isn't working label Mar 12, 2026
@github-actions
Copy link
Contributor

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

@wojiaodoubao wojiaodoubao force-pushed the too-much-write-contention-should-be-retryable branch from 7e7e44e to 78f4636 Compare March 12, 2026 13:53
@github-actions
Copy link
Contributor

Review

The semantic fix is correct: TooMuchWriteContention originates from RetryableCommitConflict with exhausted retries, so mapping it to Throttled (retryable) instead of ConcurrentModification (non-retryable) is the right call. This lets callers distinguish between errors they can retry vs. errors they cannot.

One minor issue:

The doc comment on convert_lance_commit_error (line 270) still says:

/// - `TooMuchWriteContention`: RetryableCommitConflict (semantic conflict) retries exhausted -> ConcurrentModification

This should be updated to reflect the new mapping (-> Throttled), and ideally moved next to the CommitConflict bullet since they now share the same branch.

Otherwise LGTM.

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-namespace-impls/src/dir/manifest.rs 0.00% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@wojiaodoubao wojiaodoubao mentioned this pull request Mar 17, 2026
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant