Hi Prithvi, thanks for your contribution.
I agree that the current create-catalog path for JDBC persistence is not atomic. For NoSQL, create-catalog already already guarantees the important invariant: the catalog is only made visible after the catalog admin role and initial grants have been successfully created. So I think this deserves a narrower fix for the JDBC persistence path, rather than a broad BasePersistence SPI change. Orthogonally, Polaris is moving toward treating built-in RBAC as one authorization implementation among others. That makes me hesitant to add new generic persistence SPI methods that are specifically shaped around built-in RBAC grant records. Robert On Sat, Jul 11, 2026 at 1:30 AM Prithvi S <[email protected]> wrote: > Hi all, > > FYR, I had to change the branch, so I created a new PR and closed the > mentioned PR. Please check this > https://github.com/apache/polaris/pull/5035 > instead of the mentioned PR (https://github.com/apache/polaris/pull/5032) > in the discussion. > > Thanks! > Prithvi S > > On Sat, Jul 11, 2026 at 3:35 AM Prithvi S <[email protected]> > wrote: > > > Hi all, > > > > I’d like to open discussion on hardening partial-commit windows in the > > atomic metastore path (AtomicOperationMetaStoreManager + > BasePersistence). > > > > a little background, > > BasePersistence requires each SPI method to be atomic, but several > manager > > flows still compose multiple SPI calls: > > 1. Grant / revoke - write/delete a grant row, then separately CAS-bump > > grant_records_version on grantee and securable > > 2. createCatalog - create catalog + admin role + several grants as a > > sequence of writes > > 3. dropEntity - delete entity, delete grants, bump partner versions as > > separate steps > > > > If the server fails mid-sequence, we can leave partial state (grant > > without version bumps, catalog without admin role/grants, etc.). The code > > already documents some of this as acceptable eventual consistency / “drop > > and recreate,” with TODOs asking for bulk update of grants + entity > > versions. > > > > I opened a draft implementation to make the problem concrete: > > https://github.com/apache/polaris/pull/5032 > > > > It adds BasePersistence.writeEntitiesAndGrantRecords(...) (entity > > creates/updates with per-row CAS, entity deletes, grant inserts/deletes > in > > one all-or-nothing op) and migrates grant/revoke, createCatalog, and > > dropEntity in AtomicOperationMetaStoreManager to use it. > > > > before pushing this further (or reshaping it), I’d like the community’s > > view on the approach: > > > > 1. Is extending BasePersistence the right place? > > Is a first-class “entities + grants in one atomic op” method the > preferred > > contract for backends (JDBC today, others later), or should this stay > > backend-local / optional? > > > > 2. Scope of a first change > > Would you rather see > > • (A) Narrow first PR: only grant/revoke (highest concurrency / > > cache-invalidation impact, smallest SPI surface), or > > • (B) Broader SPI + migrate createCatalog / drop in the same change (what > > #5032 currently does), or > > • (C) SPI + tests only first, call-site migration in follow-ups? > > > > 3. API shape > > Six parallel lists (entitiesToWrite, originals, deletes, grants to > > write/delete) is simple but easy to misuse. Prefer a small structured > > batch/commit type instead? > > > > 4. What must be in-scope vs out-of-scope for “atomic” > > Even with this SPI, some windows remain intentionally outside (e.g. > > storage integration create, principal secrets delete, policy-mapping > > cleanup, cleanup task scheduling). Is that acceptable for v1, or should > the > > contract cover more? > > > > 5. createCatalog specifically > > The existing comments treat partial catalog init as recoverable via drop. > > Is full atomic create worth the complexity (pre-computed > > grant_records_version, mixed create+CAS on principal roles), or is > > grant/revoke enough for now? > > > > Regards, > > Prithvi S > > >
