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 > > > > > > > > > > > > > > >
