fix: common mechanism for retries in insert, upsert and delete

[konstantinoscs]

What this does

This PR makes table writes more resilient when two writers try to commit at the same time.

Insert, upsert, and delete now share one retry mechanism. After a genuine Iceberg commit conflict, the SDK waits for a small randomized delay, refreshes the table metadata, and tries again. The delay grows between attempts and is capped at 30 seconds.

Safety

Only a known CommitFailedException is retried. Authentication, permission, network, validation, not-found, and unknown-commit-state errors still fail immediately, because replaying those operations could duplicate or hide a write.

This does not change catalog routing or credential vending, so S3 Tables and BYO behavior is preserved.

Testing

Tests cover all three write operations, retry timing and ordering, exhausted retries, refresh failures, and errors that must not be retried.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3ec0bee0-e60c-4d8e-b195-43fd88d536f3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[jo-sm]

I would suggest to give an upper bound (maybe 300 seconds / 5 minutes?), rather than only checking if the delay is finite.

[konstantinoscs]

It's a bit confusing because the upper bound is anyway decided in retry_ceiling = min(retry_delay_seconds, _MAX_COMMIT_RETRY_DELAY_SECONDS) inside _commit_with-retry_().
I'll add some clarification in the documentation for this

[jo-sm]

If it's handled somewhere else, I would just be consistent with the above line then - retry_delay_seconds < 0 is probably enough here.

[jo-sm]

I would suggest to use an int here, rather than float, unless there's a strong need for sub second delays.

[konstantinoscs]

So the default value is itself a float (0.5). Also the randomized backoff produces fractional values.
I think float is the correct thing here

[jo-sm]

I would raise a new exception, like CommitRetryExhaustionException.

[konstantinoscs]

This should remain CommitFailedException because it's actually the PyIceberg exception.
Preserving that exception:

1. retains the original failure details;
2. keeps existing except CommitFailedException handlers working;
3. avoids introducing Tower-specific behavior for S3 Tables and BYO callers.

A new exception would be a breaking change unless it subclassed CommitFailedException, and even then it adds little value.

[jo-sm]

Hmm. The way that this is used, compared to what the naming of the variable is, could be confusing (I was not sure why we have this jitter when we're using retry_delay_seconds). I'd suggest to rename retry_delay_seconds to max_retry_delay to make it clear it's the maximum, not the actual delay.

[konstantinoscs]

Actually, max_retry_delay is also inaccurate: the supplied value is only the maximum for the first retry; later ceilings grow until the separate 30-second cap we discussed above.

The biggest problem though is that It is also an existing public keyword across insert, upsert, and delete, so renaming it breaks callers on every catalog type.
I’d keep retry_delay_seconds, improve its documentation, and use initial_retry_ceiling_seconds internally

[socksy]

ah nice