Hi Robert, Makes senseđź‘Ť, thanks for checking this. I've dropped writeEntitiesAndGrantRecords from BasePersistence entirely. Instead added a small flush() default no-op. JDBC now batches writes internally and the manager calls flush() after atomic sequences. NoSQL is unaffected.. keeps the SPI clean :)
Please take another look when you can https://github.com/apache/polaris/pull/5035 Regards, Prithvi On Mon, Jul 13, 2026 at 7:46 PM Robert Stupp <[email protected]> wrote: > 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 > > > > > >
