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 >
