helium iceberg handle retry on commit conflict during the wap workflow

[bbalser]

• When the mobile-packet-verifier daemon and backfill run concurrently, they race on the same Iceberg table's
  sequence number and snapshot refs, causing "Cannot add snapshot with sequence number N older than last
  sequence number N" errors (returned as 400 from Polaris)

• Add CommitConflict error variant that detects conflict responses — both HTTP 409 and 400s with sequence
  number/requirement failure messages

• Add retry with exponential backoff (100ms base, 5s max, 4 retries) directly inside create_branch,
  commit_to_branch, and publish_branch in branch.rs — each reloads fresh table metadata before retrying

• iceberg_table.rs callers are now simple direct calls with no retry wrappers

  Files changed

  • error.rs — CommitConflict variant + is_commit_conflict() helper
  • rest_endpoint.rs — Return CommitConflict for 409 and conflict-indicating 400s; extracted is_conflict_body()
    for testability
  • branch.rs — Retry with metadata reload in create_branch, commit_to_branch, publish_branch; retry constants
    and backoff helper
  • iceberg_table.rs — Simplified to direct branch calls; reload_table made pub(crate)

[michaeldjeffrey]

Do we want to consider using backon here?

We can break out the actual branch creating to something like do_create_branch, then this function becomes about the retrying and error handling.

pub(crate) async fn create_branch(
    catalog: &Catalog,
    table: &RwLock<Table>,
    branch_name: &str,
) -> Result<()> {
    validate_branch_name(branch_name)?;

    use backon::{ExponentialBuilder, Retryable};

    (|| do_create_branch(catalog, table, branch_name))
        .retry(
            ExponentialBuilder::default()
                .with_min_delay(COMMIT_RETRY_BASE_DELAY)
                .with_max_delay(COMMIT_RETRY_MAX_DELAY)
                .with_max_times(COMMIT_MAX_RETRIES),
        )
        .when(|err| err.is_commit_conflict())
        .notify(|err, dur| {
            tracing::warn!(
                ?err,
                branch_name,
                delay_ms = dur.as_millis() as u64,
                "commit conflict, retrying create_branch"
            )
        })
        .await
}```

[bbalser]

updated 906a081

[michaeldjeffrey]

Should we be putting the body of the response in these errors? So we can start to differentiate between the different types of conflicts?

[bbalser]

good call, updated 4bfaef7

[michaeldjeffrey]

It is no where near necessary here, just thought it was interesting to point out. All the method on ExpontentialBuilder are const.

[michaeldjeffrey]

If we move the backon retry stuff into commit, we get retries for all commits on conflict, and our other functions don't need to be as aware of that error.

[bbalser]

I am open to refactor around this in the future, but not sure it is worth it at the moment.