Forgot to link - I'm referring to https://dev.mysql.com/doc/refman/9.7/en/commit.html as an example of the AUTOCOMMIT vs START TRANSACTION stuff in RDBMS.
But note that it's not only relevant in the RDBMS; it carries over into the data lakehouse world even in cases where the syntax borrows directly from the RDBMS idioms, such as in Snowflake: https://docs.snowflake.com/en/sql-reference/transactions#implicit-transactions On Thu, Jul 23, 2026 at 7:30 PM Dennis Huo <[email protected]> wrote: > Great to see this topic being raised, and I agree it's a good time to hash > out most of our bulk-mutation atomicity semantics. > > One main thing I want to make sure we've rolled into this discussion is > all the prior analysis from the "persistence refactor" work about 1.5 years > ago: > https://docs.google.com/document/d/1U9rprj8w8-Q0SnQvRMvoVlbX996z-89eOkVwWTQaZG0/edit?tab=t.0#heading=h.rg4mbxsmdvk4 > > In particular, a couple key areas: > > 1. Directly related to this thread's rpoposal - whether to incorporate > lower-level primitives of the "batch update" concept ( > https://docs.google.com/document/d/1U9rprj8w8-Q0SnQvRMvoVlbX996z-89eOkVwWTQaZG0/edit?tab=t.0#bookmark=id.5tgsfvekfuf8) > in the lower-level BasePersistence - I think the stub paragraph in there > about "reflect the commitTransaction use case without using > runInTransaction blocks" could relate well to what Dmitri says about > "introducing > explicit start/end boundaries for each REST API request at the > Persistence layer" - essentially if our SPI "collects" the batch of updates > and the boundary is the commit/flush, and persistence backends that *do* > support atomic-batch can map cleanly. > > 2. Whether the reasoning for keeping the best-effort/multi-phase decisions > for the "special low-evel requirements" that explicitly call out > createCatalog/createPrincipal/bootstrap/dropTableWithPurge still hold - > factors into the question earlier in this thread about whether the > complexity of the real batchUpdate is worth it for the cases that are "good > enough" as non-atomic + cleanup. > > One thing is that it seems like we've taken as a premise in this thread > that the pattern TransactionWorkspaceMetaStoreManager uses is broken, > with the reasoning being: > > So a caller of one method on >> TransactionWorkspaceMetaStoreManager, after the call succeeds, still >> cannot >> be sure that the change is effective because the "real" change is made >> only >> later, in a batched way, which includes other changes and the batch may >> fail as a whole > > > I can be convinced that this may be a problem in the Polaris control flow, > *but* I don't agree that it's inherently wrong. We'd have to ask ourselves > whether/why the LocalIcebergCatalog *depends* on that "promise" of whether > the change is effective, vs whether the final verdict of success occurs at > a higher-level API-handler layer. > > In particular, the idiom of the TransactionWorkspace maps to fairly > standard patterns, but I can believe that it diverges in key ways that > render the mental model broken. I just don't see those key divergences > identified clearly yet. > > The analogy to underlying RDBMS systems is basically: > > 1. AtomicMetaStoreManager -- "SET autocommit=1", *and* lacks atomic > composite mutation statements - MetaStoreManager methods implicitly convey > certain "composite" statements that for AtomicMetaStoreManager are silently > decomposed into multiple substatements that are non-atomic across each other > 2. TransactionalMetaStoreManager -- "SET autocommit=1" but the implied > composite mutation statements from MetaStoreManager interface are > effectively internally atomic, and may rely on low-level transactions > across actually decomposite multi-statement commits > 3. TransactionWorkspaceMetaStoreManager-- "SET autocommit=0/ START > TRANSACTION" - things happening between START TRANSACTION and COMMIT don't > *locally* know whether they were transactionally flushed or not, but the > whole series of high-level mutations *beyond* the implied composite > mutation statements now behave as a single atomic unit > > And it's a common pattern in code then if you have helper functions that > do low-level mutations, those helper functions don't need to know whether > the top-level driver is using a "SET autocommit=1" or a "START TRANSACTION" > -- the multi-statement transactional driver still reuses the low-level code > and gets to decide when to actually flush the whole thing. > > The benefit of this approach is that the core "helper function" is the > shared Iceberg-core logic doesn't need to know about multi-table > transactions - LocalIcebergCatalog, the BaseTableOperations stuff, etc. We > want to minimize having forks in the parts that inherit the Iceberg > nuts-and-bolts implementations (the doRefresh/doCommit dance, single-table > UpdateRequirements, etc). > > One example of how this asssumption might *break* is if Icebeg-core itself > wants the multi-table semantics to be first-class, *not* just in the > Catalog layer. So if a core nominally single-table update can include, for > example, an UpdateRequirement tuple that refers to the state of *another* > table in the same catalog, then it makes a lot more sense to force the > LocalIcebergCatalog layer to be transaction-queueing aware. > > So if I were to take the devil's advocate stance against my own previous > stance of defending the TransactionalWorkspaceMetaStoreManager, I'd say > that certain emerging features such as the split between Definer and > Invoker views are likely to mor tightly-couple entities that used to behave > independently in the shared Iceberg-core layer, supporting the idea that > our LocalIcebergCatalog *should* be what explicitly manages the queueing of > mutations into a transactional commit set, and a data point against trying > to model the shared layer under the same MetaStoreManager APIs as the > "autocommit=1" model. > > That said, if the cross-entity UpdateRequirements are read-only, I'm still > not sure it supplants the TransactionWorkspace/autocommit=0 model, because > the state-requirements could still be expressible in the per-mutation model > like updateEntityIf*Dependencies*NotChanged(entity, > crossEntityDependencyVersions) > > > On Wed, Jul 22, 2026 at 3:32 PM Yufei Gu <[email protected]> wrote: > >> Thanks for raising this, Prithvi! The examples you shared are valid cases >> where we want to ensure consistency. As I mentioned in another thread, a >> lot of them are known issues, and `TransactionalMetaStoreManagerImpl` >> already handles most of these cases well. >> >> > 1. Is extending `BasePersistence` the right place? >> >> I think so. A good example is the `TransactionalPersistence` interface, >> which extends `BasePersistence` by introducing methods for transactional >> operations. >> >> 2. I think starting with a narrow PR first makes sense, for example, >> focusing on grant/revoke. A smaller scope is usually easier to review and >> tends to result in a higher quality PR. >> >> 3. In terms of the API shape, I don't think there's any urgency to >> introduce an abstraction yet, but I'm open to introducing something like a >> `ChangeSet` to group related changes. It could be similar to the >> `TableCommitActions` class introduced in this PR: >> https://github.com/apache/polaris/pull/5088. >> >> 4. Same as 3. I don't think we need to decide this now. >> >> 5. I think it's worth wrapping this in a transaction. Please take a look >> at >> the existing implementation here: >> >> >> https://github.com/apache/polaris/blob/b81766a70575b3c1ba122c2b7f389f591a9c7c53/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java#L965 >> >> Yufei >> >> >> On Tue, Jul 14, 2026 at 7:14 PM Dmitri Bourlatchkov <[email protected]> >> wrote: >> >> > Hi Prithvi and All, >> > >> > This is a good discussion! >> > >> > I'd like to highlight a different aspect, though. >> > >> > With the atomic MetaStore approach (one persistence method = one atomic >> > change) approach, we cannot establish any expectations about reads that >> > happen between changes. >> > >> > In many cases, assuming a JDBC backend, the end-to-end request execution >> > logic appears to assume that the RDBMS operates at the Serializable >> > isolation level. Each individual entity has optimistic locking checks, >> > which may look like Serializable isolation level is not required, but >> if we >> > consider that Polaris reads and processes entity properties for some >> > unchanged entities, per-entity optimistic locking does not seem >> sufficient. >> > >> > I'd like to propose introducing explicit start/end boundaries for each >> REST >> > API request at the Persistence layer. In JDBC, these boundaries will >> > clearly map to transaction start/commit and allow reads and writes to be >> > reconciled by the RDBMS (i.e. enforce Serializable isolation). >> > >> > A side effect of this is that Polaris will probably have to support >> retries >> > for transient persistence errors like Serializability failures. >> > >> > Another side effect is that with this approach, idempotency data could >> be >> > stored in a separate table and still participate in the JDBC transaction >> > that changes entities. >> > >> > I do not have any concrete code changes in mind yet. This is for >> > discussion. WDYT? >> > >> > Thanks, >> > Dmitri. >> > >> > On Tue, Jul 14, 2026 at 6:20 PM Prithvi S <[email protected]> >> > wrote: >> > >> > > Hi all, >> > > >> > > Following feedback from @snazy and @dimas-b on >> > > https://github.com/apache/polaris/pull/5035, and building on >> > > the `MetaStoreChangeSet` / `commitTransactionBatch` direction >> introduced >> > in >> > > PR #4939, I'd like to propose a design for making batch/ChangeSet >> > > operations the PRIMARY mutation API in Polaris, with single-entity >> > > operations >> > > riding on top, not the other way around. >> > > >> > > issue: >> > > 1. BasePersistence contract says each method is atomic - yet real >> > > operations >> > > (grant, createCatalog, dropEntity) need multi-call sequences that >> can >> > > partially >> > > fail (e.g., grant record written but version bump lost). >> > > 2. flush() is a leaky abstraction - PR #5035 tried adding `flush()` to >> > > batch >> > > writes across SPI calls, but this violates the "no pending changes" >> > > contract. >> > > Backends that don't need it (NoSQL, EclipseLink) shouldn't see it. >> > > 3. TransactionWorkspaceMetaStoreManager is broken. It intercepts >> > individual >> > > calls, returns success before persistence, and batches later. As Dmitr >> > > noted >> > > in this >> https://lists.apache.org/thread/rf5orxs815zs4h64p4rwp03q3pbgxb5r >> > : >> > > "a caller of one method... after the call succeeds, >> > > still cannot be sure that the change is effective." >> > > 4. Two parallel MetaStoreManager implementations - >> > > `AtomicOperationMetaStoreManager` >> > > (for JDBC/NoSQL) and `TransactionalMetaStoreManagerImpl` (for >> > > EclipseLink) >> > > duplicate logic but with different transaction models. Maintenance >> > > burden. >> > > >> > > Proposed Design: "ChangeSet-First" API >> > > Core Principle >> > > All mutations flow through a single `commitChangeSet(ChangeSet)` path. >> > > Single-entity operations (`createEntityIfNotExists`, `grantPrivilege`, >> > > etc.) are >> > > thin wrappers that build a `ChangeSet` and delegate to >> `commitChangeSet`. >> > > 1. BasePersistence SPI (minimal addition) >> > > Add ONE new method to `BasePersistence`: >> > > >> > > ```java >> > > /* >> > > * Atomically commit a mixed set of entity mutations and grant-record >> > > changes. >> > > * Either every change is applied durably and becomes visible >> together, >> > or >> > > none >> > > * are applied. Implementations that cannot support mixed atomic >> commits >> > > MUST >> > > * throw {@link UnsupportedOperationException}. >> > > * >> > > * <p>The default implementation throws UnsupportedOperationException, >> > > preserving >> > > * backward compatibility for existing backends. >> > > */ >> > > default void commitChangeSet( >> > > @NonNull PolarisCallContext callCtx, >> > > @NonNull List<EntityMutation> entityMutations, >> > > @NonNull List<GrantMutation> grantMutations) { >> > > throw new UnsupportedOperationException( >> > > "Backend does not support atomic mixed commits; use individual >> > > operations"); >> > > } >> > > ``` >> > > Where: >> > > - `EntityMutation` = (entity, originalEntity, type: >> CREATE/UPDATE/DELETE) >> > > - `GrantMutation` = (grantRecord, type: CREATE/DELETE) >> > > >> > > choosing this because: >> > > - It keeps `BasePersistence` atomic per-call (no flush, no pending >> state) >> > > - It groups everything that needs to be atomic into ONE call >> > > - Backends that can do it (JDBC with transactions, EclipseLink) >> override >> > it >> > > - Backends that can't (simple KV) keep the default and callers use >> > fallback >> > > >> > > 2. PolarisMetaStoreManager (reformulate all mutations) >> > > `commitTransactionBatch(MetaStoreChangeSet)` becomes THE method. All >> > other >> > > mutation methods get default implementations in terms of it: >> > > >> > > ```java >> > > // ALREADY EXISTS in PR #4939 - we expand it >> > > default @NonNull EntitiesResult commitTransactionBatch( >> > > @NonNull PolarisCallContext callCtx, @NonNull MetaStoreChangeSet >> > > changeSet) { >> > > // fallback: call individual methods in sequence (NOT atomic) >> > > for (EntityWithPath create : changeSet.creates()) { ... } >> > > for (EntityWithPath update : changeSet.updates()) { ... } >> > > } >> > > >> > > // NEW: Single-entity operations ride on top of commitTransactionBatch >> > > default @NonNull EntityResult createEntityIfNotExists( >> > > @NonNull PolarisCallContext callCtx, >> > > @Nullable List<PolarisEntityCore> catalogPath, >> > > @NonNull PolarisBaseEntity entity) { >> > > return commitTransactionBatch(callCtx, >> > > MetaStoreChangeSet.ofCreate(catalogPath, entity)) >> > > .toSingleResult(); >> > > } >> > > >> > > default @NonNull EntitiesResult updateEntitiesPropertiesIfNotChanged( >> > > @NonNull PolarisCallContext callCtx, @NonNull List<EntityWithPath> >> > > entities) { >> > > return commitTransactionBatch(callCtx, >> > > MetaStoreChangeSet.ofUpdates(entities)); >> > > } >> > > ``` >> > > >> > > Then `AtomicOperationMetaStoreManager` ONLY overrides >> > > `commitTransactionBatch`: >> > > >> > > ```java >> > > @Override >> > > public @NonNull EntitiesResult commitTransactionBatch( >> > > @NonNull PolarisCallContext callCtx, @NonNull MetaStoreChangeSet >> > > changeSet) { >> > > BasePersistence ms = callCtx.getMetaStore(); >> > > >> > > // Build entity mutations (creates + updates) >> > > List<EntityMutation> entityMutations = >> buildEntityMutations(changeSet); >> > > >> > > // Build grant mutations from the ChangeSet context (if any) >> > > List<GrantMutation> grantMutations = buildGrantMutations(changeSet); >> > > >> > > if (ms.supportsAtomicMixedCommit()) { >> > > // JDBC path: one transaction, everything atomic >> > > ms.commitChangeSet(callCtx, entityMutations, grantMutations); >> > > } else { >> > > // Fallback: sequence of individual operations (eventual >> consistency) >> > > return super.commitTransactionBatch(callCtx, changeSet); >> > > } >> > > } >> > > ``` >> > > >> > > 3. AtomicOperationMetaStoreManager refactor >> > > Move ALL complex operations (grant, revoke, createCatalog, >> dropEntity) to >> > > build a `ChangeSet` and call `commitTransactionBatch`: >> > > >> > > Example: grantPrivilege >> > > ```java >> > > // OLD: 3 separate persistence calls, no atomicity >> > > ms.writeToGrantRecords(callCtx, grantRecord); >> > > ms.writeEntity(callCtx, updatedGrantee, false, granteeEntity); >> > > ms.writeEntity(callCtx, updatedSecurable, false, securableEntity); >> > > >> > > // NEW: build ChangeSet, commit once >> > > MetaStoreChangeSet changeSet = MetaStoreChangeSet.builder() >> > > .addUpdate(granteeEntity, updatedGrantee) >> > > .addUpdate(securableEntity, updatedSecurable) >> > > .addGrant(grantRecord) >> > > .build(); >> > > return commitTransactionBatch(callCtx, changeSet); >> > > ``` >> > > Example: dropEntity >> > > ```java >> > > // OLD: delete entity, then delete grants, then bump versions — 3+ >> calls >> > > // NEW: build ChangeSet with deletes + updates, commit once >> > > MetaStoreChangeSet changeSet = MetaStoreChangeSet.builder() >> > > .addDelete(entity) >> > > .addDeletes(grantRecords) >> > > .addUpdates(relatedEntitiesWithBumpedVersions) >> > > .build(); >> > > return commitTransactionBatch(callCtx, changeSet); >> > > ``` >> > > >> > > 4. JdbcBasePersistenceImpl >> > > Implement `commitChangeSet` using a single transaction: >> > > >> > > ```java >> > > @Override >> > > public void commitChangeSet( >> > > @NonNull PolarisCallContext callCtx, >> > > @NonNull List<EntityMutation> entityMutations, >> > > @NonNull List<GrantMutation> grantMutations) { >> > > datasourceOperations.runWithinTransaction(connection -> { >> > > for (EntityMutation em : entityMutations) { >> > > switch (em.type()) { >> > > case CREATE -> persistEntity(connection, em.entity(), null); >> > > case UPDATE -> persistEntity(connection, em.entity(), >> > > em.originalEntity()); >> > > case DELETE -> deleteEntity(connection, em.entity()); >> > > } >> > > } >> > > for (GrantMutation gm : grantMutations) { >> > > switch (gm.type()) { >> > > case CREATE -> persistGrantRecord(connection, >> gm.grantRecord()); >> > > case DELETE -> deleteGrantRecord(connection, >> gm.grantRecord()); >> > > } >> > > } >> > > return true; >> > > }); >> > > } >> > > ``` >> > > No ThreadLocal. No flush(). Just one transaction per `commitChangeSet` >> > > call. >> > > >> > > 5. TransactionWorkspaceMetaStoreManager >> > > We can DEPRECATE IT. The "accumulate in memory, commit once" pattern >> > moves >> > > to the >> > > CALLER (e.g., IcebergCatalogHandler for commitTransaction): >> > > >> > > ```java >> > > // In IcebergCatalogHandler >> > > MetaStoreChangeSet.Builder builder = MetaStoreChangeSet.builder(); >> > > for (TableCommit commit : commits) { >> > > builder.addUpdate(commit.oldEntity(), commit.newEntity()); >> > > } >> > > return metaStoreManager.commitTransactionBatch(callCtx, >> builder.build()); >> > > ``` >> > > >> > > No more "workspace that pretends to persist but doesn't." The caller >> > builds >> > > the ChangeSet explicitly. >> > > >> > > Implementation Plan if this is fine :) >> > > >> > > 1. Phase 1 (this PR): Add `commitChangeSet` to `BasePersistence`, >> expand >> > > `MetaStoreChangeSet` to include deletes and grants, refactor >> > > `AtomicOperationMetaStoreManager` to use `commitTransactionBatch` >> for >> > > grant operations + createCatalog + dropEntity. >> > > >> > > 2. Phase 2 (follow-up): Refactor remaining single-entity methods to >> > default >> > > to `commitTransactionBatch`. Deprecate >> > > `TransactionWorkspaceMetaStoreManager`. >> > > >> > > 3. Phase 3 (follow-up): Unify `AtomicOperationMetaStoreManager` and >> > > `TransactionalMetaStoreManagerImpl` - both use the same >> > > `commitTransactionBatch` >> > > path, just with different `BasePersistence` backends. >> > > >> > > I have a few queries as well: >> > > 1. Should `commitChangeSet` live on `BasePersistence` or a new >> > > sub-interface? >> > > (I lean toward `BasePersistence` with a default throw.. simplest >> > > migration.) >> > > >> > > 2. Should we keep `writeEntities` or deprecate it in favor of >> > > `commitChangeSet`? >> > > (I lean toward keep but document that `commitChangeSet` is >> preferred >> > for >> > > mixed operations.) >> > > >> > > 3. How do we handle the `PolarisMetaStoreManager` methods that do >> reads >> > > before >> > > writes (e.g., `grantPrivilege` loads grantee/securable before >> > updating)? >> > > (I lean toward: reads happen in `AtomicOperationMetaStoreManager`, >> > then >> > > the >> > > computed ChangeSet is committed. Reads are not part of the atomic >> > > boundary.) >> > > >> > > Looking forward to feedback! >> > > >> > > On Tue, Jul 14, 2026 at 3:21 AM Prithvi S < >> [email protected]> >> > > wrote: >> > > >> > > > 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 >> > > >> > > >> > > >> > >> > > >> >> > > > >> > > >> > >> >
