Hi all I think Robert has a good point.
If the atomicity guarantee lives only in BasePersistence, then the manager contract can't tell a caller whether the state it read for validation/authorization/credential-vending is the same state that eventually commits. That's the actual bug class behind the JDBC symptoms (not any single operation being non-atomic, but the contract being silent about it. Every operation-specific fix (like #4939, #5035 or #5095) re-answers this question locally and it keeps recurring. So the deliverable should start with a written consistency contract at the manager level. I'm not sure migrating everyone to TransactionalMetaStoreManagerImpl is a good idea. It would tie the logical change set to a DB transaction spanning the REST request. It means: - it holds a durable transaction open across slow external work (credential vending, OPA, Ranger, ...) - it doesn't map to NoSQL - It wraps single-row updates in runWiithinTransaction, which is a overhead So, I think the transactional manager isn't a portable target. It's "only" one backend's strategy. I think Privthi's approach is right. A backend-agnostic change-set primitive with a documented fallback is the correct shape. There is one caveat: without carrying the original entitiy (not just the new one) the change set can't express optimistic concurrency. I propose the following multi-steps approach: 1. We write the manager-level consistency contract as Robert asked. It should include the explicit statement that a logical change set is not a request-scope DB transaction. 2. We make Compare And Swap (optimistic-concurrency pattern) baseline first-class in EntityMutation before merging the SPI 3. Then we refactor createCatalog/dropEntity/renameEntity. Thoughts? Regards JB On Fri, Jul 24, 2026 at 5:08 PM Robert Stupp <[email protected]> wrote: > > Hi all, > > Yufei's clarification about where the atomicity guarantee is defined seems > important. > If it is a property of the lower-level BasePersistence contract rather than > the general PolarisMetaStoreManager contract, the general contract does not > tell callers whether the state used for validation, authorization, or > credential vending is consistent with the change that eventually commits. > > The current PRs suggest that operation-specific multi-object methods can > fix individual cases, while leaving the same contract question to recur for > each new case. > > I would also avoid defining a logical change set as a database transaction > around an entire REST request. > That would tie the contract to the backend and database, and could keep the > durable attempt open across slow external work. > > So is the choice really between the two current manager implementations, or > do we first need to revisit the boundary and guarantees exposed to their > callers? > > Cheers, > Robert > > > On Fri, Jul 17, 2026 at 10:21 PM Dmitri Bourlatchkov <[email protected]> > wrote: > > > Hi Yufei, > > > > Thanks for the link. I stand corrected. The "one atomic change per method" > > contract as defined in javadoc does apply only to BasePersistence > > and AtomicOperationMetaStoreManager (which delegates to BasePersistence). > > > > Note that all non-test Persistence implementations in the Polaris codebase > > extend those classes (which is probably why I was confused about atomicity > > expectations). > > > > However, this creates a gap in the Persistence SPI specification. If > > other PolarisMetaStoreManager implementations do not have to comply with > > this principle, it will create a conceptual difficulty at call sites. How > > can PolarisMetaStoreManager callers reason about consistency and durability > > behaviours in general? > > > > I believe we need to address that as part of this discussion. > > > > Cheers, > > Dmitri. > > > > On Thu, Jul 16, 2026 at 9:29 PM Yufei Gu <[email protected]> wrote: > > > > > > The MetaStore SPI is currently defined with the idea that one method > > call > > > means one atomic change. > > > > > > If the "MetaStore SPI" refers to the interface PolarisMetaStoreManager, I > > > don't think we've ever state each method to be atomic. We did clarify > > > atomicity[1] in the interface BasePersistence though. > > > > > > > > > 1. > > > > > > > > https://github.com/apache/polaris/blob/e9039e12003a13e783b5130a3d30d30cfe78d93c/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java#L48 > > > > > > > > > > > > Yufei > > > > > > > > > On Thu, Jul 16, 2026 at 11:27 AM Dmitri Bourlatchkov <[email protected]> > > > wrote: > > > > > > > Hi Yufei, > > > > > > > > I agree that JDBC transactions must be handled more explicitly. > > However, > > > > I'm not sure that simply moving to TransactionalMetaStoreManagerImpl is > > > > sufficient. > > > > > > > > The MetaStore SPI is currently defined with the idea that one method > > call > > > > means one atomic change [1]. The "transactional" MetaStore impl. is > > but a > > > > sub-case of that. It cannot alter the high-level contract. > > > > > > > > We could add SPI methods having multiple object parameters to represent > > > > grouped changes, but I am not sure it will be a sound design. This will > > > > bloat the interface surfaces and require extra impl. effort for each > > > > backend type. More importantly, adding multi-arg change methods still > > > won't > > > > address the problem of reads being consistent with writes, because each > > > > method call will still be independent regarding the data stored in the > > > > database. > > > > > > > > I tend to think we need to introduce a "change set" or "atomic batch" > > > > concept to core Persistence and associate each REST API request with > > one > > > > such change set, which will be committed (or rolled back) at the end of > > > the > > > > request. I believe Ayush mentioned a similar concept in PR 4939 [2]. In > > > > JDBC each change set will naturally be associated with an RDBMS > > > > transaction. In NoSQL persistence, each atomic change set will be > > > > associated with one CAS operation on the underlying database. > > > > > > > > [1] https://lists.apache.org/thread/rf5orxs815zs4h64p4rwp03q3pbgxb5r > > > > > > > > [2] https://github.com/apache/polaris/pull/4939#discussion_r3575719158 > > > > > > > > Cheers, > > > > Dmitri. > > > > > > > > On Thu, Jul 16, 2026 at 1:12 PM Yufei Gu <[email protected]> wrote: > > > > > > > > > Thanks for raising this, Dmitri. > > > > > > > > > > These are valid concerns, and they were already recognized when we > > > > > introduced JDBC persistence to Polaris. At that time, we chose to use > > > > > AtomicOperationMetaStoreManager for the JDBC due to the simplicity. I > > > > think > > > > > most of the issues mentioned here can already be addressed by > > > > > TransactionalMetaStoreManagerImpl. > > > > > > > > > > For example, rename is already wrapped in a transaction in > > > > > TransactionalMetaStoreManagerImpl [1]. Similarly, catalog creation, > > > which > > > > > involves reading and creating multiple objects, is also executed > > > within a > > > > > transaction [2]. > > > > > > > > > > I see two possible directions: > > > > > > > > > > 1. > > > > > > > > > > Modify AtomicOperationMetaStoreManager together with the > > persistence > > > > > backends (such as JDBC) to provide the required consistency > > > guarantees > > > > > for > > > > > specific operations, similar to what > > > TransactionalMetaStoreManagerImpl > > > > > does. > > > > > 2. > > > > > > > > > > Migrate the persistence backends (such as JDBC) to use > > > > > TransactionalMetaStoreManagerImpl directly. We may have to deal > > with > > > > > transactional semantic mismatches across different persistence > > > > backends. > > > > > For example, we would likely avoid using JDBC's > > > `runWithinTransaction` > > > > > for > > > > > single row updates, which adds additional overhead and complexity > > > > > without > > > > > benefits. > > > > > > > > > > References: > > > > > > > > > > 1. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/polaris/blob/5731c5cbee02257d1f21f78ca3befcd639b100a3/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java#L1286 > > > > > 2. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/polaris/blob/5731c5cbee02257d1f21f78ca3befcd639b100a3/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java#L965 > > > > > > > > > > Yufei > > > > > > > > > > > > > > > On Thu, Jul 16, 2026 at 8:22 AM Dmitri Bourlatchkov < > > [email protected]> > > > > > 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 > > > > > > > > > > > > > > > > > > > >
