Hi all, I agree the persistence layer needs a single, well-defined contract for multi-object consistency rather than a series of operation-specific patches.
I want to clarify where PR #5035 sits in that picture. It is not meant to be yet another incremental, operation-specific fix. It attempts to introduce the generic "change set" primitive that Dmitri described: BasePersistence#commitChangeSet(List<EntityMutation>, List<GrantMutation>) - one atomic call for mixed entity + grant mutations. EntityMutation / GrantMutation records that describe create, update, and delete operations. StoreChangeSet expanded to carry creates, CAS-updates, deletes, and grant-record changes. AtomicOperationMetaStoreManager grant/revoke paths already build a change set and use commitChangeSet when the backend supports it. So #5035 is intended as the SPI foundation for the holistic model, not a one-off workaround. That said, the current implementation is incomplete in exactly the ways the discussion has surfaced: 1. The change-set API does not yet carry the CAS baseline for updates cleanly (EntityWithPath only holds the new entity, not the original). 2. AbstractTransactionalPersistence/TreeMap does not implement commitChangeSet, so TransactionalMetaStoreManagerImpl cannot yet use the same primitive. 3. AtomicOperationMetaStoreManager#commitTransactionBatch ignores deletes and grant records in the change set. 4. TransactionalMetaStoreManagerImpl does not override commitTransactionBatch at all. 5. createCatalog, dropEntity, and renameEntity still compose individual SPI calls instead of building one change set. wanted to check if we shall keep #5035 focused: fix (1)-(4) above so that the change-set SPI is sound and usable, and land it. Then refactor createCatalog / dropEntity / renameEntity in follow-up PRs that also subsume #4939 (staged creates) and #5095 (atomic drop + cleanup task) into the same MetaStoreChangeSet. If this is ok I will also update the PolarisMetaStoreManager and BasePersistence Javadoc to make the contract explicit: a manager-level operation may be implemented as a change set, and backends that support commitChangeSet guarantee that the whole set is atomic; backends that do not support it must fall back to individual atomic operations with the documented-atomic behavior. WDYT? Thanks, Prithvi On Sat, Jul 25, 2026 at 5:05 AM Dmitri Bourlatchkov <[email protected]> wrote: > Another related PR: https://github.com/apache/polaris/pull/5095 > > On 2026/07/16 15:21:20 Dmitri Bourlatchkov wrote: > > Hi all, > > > > Ayush and Prithvi recently contributed a couple of interesting PRs: > > [4939], [5035]. > > > > It looks like people are starting to encounter consistency issues in > > JDBC persistence. > > > > The PRs provide valuable insight into the underlying issues. They offer > > incremental fixes that can work. However, I believe it is time for the > > Polaris community to review and improve this area of the codebase > > holistically. > > > > By this, I mean finding a solution that can be applied to all > > persistence backends (in-memory, JDBC, NoSQL) and addresses these > > aspects: > > > > * Supporting concurrent and consistent changes where the service reads > > and validates current catalog state, then commits a change (e.g. > > name clashes during renames). > > * Supporting consistent but independent changes to RBAC grants and > > MetaStore entities. This independence is needed to support > > external authorizers like OPA and Ranger. > > * Supporting atomic changes across multiple similar entities. > > * Supporting authorization-based filtering of list operations (cf. > > [4831]). > > * Supporting credential-vending decisions that are rooted in the > > exact state of the catalog. > > * Supporting server-side retries for transient persistence failures > > (e.g. RDBMS Tx serializability failures). > > > > Please share your comments and ideas. > > > > [4831] https://github.com/apache/polaris/pull/4831 > > > > [4939] https://github.com/apache/polaris/pull/4939 > > > > [5035] https://github.com/apache/polaris/pull/5035 > > > > Thanks, > > Dmitri > > >
