Hi Dmitri, Thanks for flagging this, yes you're right that it overlaps with #5263 in scope. Both PRs touch the same root cause i.e an ambiguous JDBC write outcome during an Iceberg tablecommit.
The one thing #5458 adds beyond #5263 is a reconciliation step: instead of immediately surfacing the ambiguity to the caller, it reloads the persisted entity and checks whether the metadata location already matches what we tried to commit. If it does, the commit is treated as a genuine success with no error surfaced at all. Only when that reconciliation itself can't confirm the outcome does it fall back to CommitStateUnknownException. #5263 doesn't have this reconciliation step, it always surfaces the ambiguous case to the client. Given the overlap, I'm happy to defer to #5263 and not duplicate review effort on the same files. Once #5263 merges, I'll rebase and follow up with the reconciliation logic as an incremental PR on top of it, scoped specifically to that additional behavior. I already mentioned the same in the PR comment as well https://github.com/apache/polaris/pull/5263#issuecomment-5609541732 Thanks, Arun On Sat, Sep 12, 2026 at 4:47 AM Dmitri Bourlatchkov < [email protected]> wrote: > Hi Arun, > > How is this related to PR [5263]? > > I did not look into your PR in details, yet, but it matches [5263] in > scope, I'd prefer to continue on that earlier PR. > > If your work provides additional fixes, would it be possible to defer them > until after [5263] merges? > > Thanks, > Dmitri. > > [5263] https://github.com/apache/polaris/pull/5263 > > On Mon, Sep 7, 2026 at 10:23 AM Arun Suri via dev <[email protected]> > wrote: > >> Hi all, >> >> I’d like to get some feedback on how we should handle ambiguous >> persistence >> outcomes during Iceberg table metadata commits when Polaris uses the >> relational JDBC metastore. >> >> Today, an Iceberg metadata update can be executed as a JDBC auto-commit >> statement. If the database applies the update but the connection fails >> before the JDBC driver receives the result, Polaris can’t tell whether the >> catalog update actually committed. >> >> A client retry can then use stale table state and get a normal optimistic >> concurrency conflict. For Iceberg clients, that distinction is important: >> CommitFailedException represents a definite commit failure and can trigger >> cleanup of newly written manifest files, even though the catalog may >> already point to metadata referencing those files. >> >> The proposal is: >> >> 1. >> >> Add a persistence signal for an ambiguous write outcome. >> 2. >> >> Have the JDBC implementation raise it only for connection/I/O failures >> from PreparedStatement.executeUpdate(), where the statement may have >> already reached the database. >> 3. >> >> Keep definite failures retryable, including serialization failures and >> connection-acquisition failures where no statement was sent. >> 4. >> >> In LocalIcebergCatalog table-like updates, catch the ambiguous signal >> and reload the persisted entity. >> 5. >> >> If the persisted metadata location matches the requested metadata >> location, treat the operation as successful. >> 6. >> >> Otherwise, return Iceberg’s CommitStateUnknownException(500) for which >> prevents the client from deleting the staged metadata files instead of >> a >> definite concurrency conflict. >> >> There’s no additional work on the normal commit path; reconciliation only >> happens after an ambiguous JDBC failure. >> >> The draft implementation uses default methods on BasePersistence and >> PolarisMetaStoreManager, so only JDBC opts into ambiguity detection. >> Transactional and non-JDBC persistence implementations keep their current >> behavior. >> >> The main question I’d like feedback on is whether this persistence-SPI >> approach makes sense, or whether maintainers would prefer a different >> mechanism for propagating a backend-specific “write outcome unknown” >> signal >> up to the Iceberg catalog layer. >> >> I also have a draft PR with JDBC unit coverage, existing >> optimistic-concurrency coverage, and an end-to-end test that simulates an >> update being persisted while the response is lost. The test verifies that >> the committed manifest list and manifests remain available. >> >> Bug Ticket: https://github.com/apache/polaris/issues/5459 >> >> PR: https://github.com/apache/polaris/pull/5458 >> >> Thanks. >> >> -- >> Arun Suri >> >> Staff Software Engineer >> >> He/him/his >> >> Engineering | Fivetran >> [email protected] >> fivetran.com <//fivetran.com> >> <http://www.fivetran.com> >> [image: facebook] <https://www.facebook.com/Fivetran/> [image: twitter] >> < >> https://twitter.com/fivetran?ref_src=twsrc%5Egoogle%7Ctwcamp%5Eserp%7Ctwgr%5Eauthor >> > >> [image: >> linkedin] <https://www.linkedin.com/company/fivetran> [image: instagram] >> <https://www.instagram.com/fivetran_ig/> >> > > > -- > Dmitri Bourlatchkov > Senior Staff Software Engineer, Dremio > Dremio.com > <https://www.dremio.com/?utm_medium=email&utm_source=signature&utm_term=na&utm_content=email-signature&utm_campaign=email-signature> > / > Follow Us on LinkedIn <https://www.linkedin.com/company/dremio> / Get > Started <https://www.dremio.com/get-started/> > > > The Agentic Lakehouse > The only lakehouse built for agents, managed by agents > > > -- Arun Suri Senior Software Engineer He/him/his Engineering | Fivetran [email protected] fivetran.com <//fivetran.com> <http://www.fivetran.com> [image: facebook] <https://www.facebook.com/Fivetran/> [image: twitter] <https://twitter.com/fivetran?ref_src=twsrc%5Egoogle%7Ctwcamp%5Eserp%7Ctwgr%5Eauthor> [image: linkedin] <https://www.linkedin.com/company/fivetran> [image: instagram] <https://www.instagram.com/fivetran_ig/>
