dimas-b commented on code in PR #1135:
URL: https://github.com/apache/polaris/pull/1135#discussion_r1985855340
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalPersistence.java:
##########
@@ -88,9 +100,214 @@ void runActionInReadTransaction(
* @return the list of entityActiveKeys for the specified lookup operation
*/
@Nonnull
- List<EntityNameLookupRecord> lookupEntityActiveBatch(
+ List<EntityNameLookupRecord> lookupEntityActiveBatchInCurrentTxn(
@Nonnull PolarisCallContext callCtx, List<PolarisEntitiesActiveKey>
entityActiveKeys);
/** Rollback the current transaction */
void rollback();
+
+ //
+ // Every method of BasePersistence will have a related *InCurrentTxn method
here; the semantics
+ // being that transactional implementations of a PolarisMetaStoreManager may
choose to
+ // self-manage outer transactions to perform all the persistence calls
within that provided
+ // transaction, while the basic implementation will only use the "durable in
a single-shot"
+ // methods from BasePersistence.
Review Comment:
the new methods seem to match methods in `BasePersistence` 1:1, which seems
like a redundant copy.
How about we use `BasePersistence` to define API methods, parameters and
version check expectations, but not attach any atomicity or durability
guarantees to them?
Then, `AtomicPersistence extends BasePersistence` <- here we define that
each method call is durable and atomic.
`TransactionPersistence extends BasePersistence` <- allows explicitly
managing Tx boundaries and defines that changes are durable on Tx commit. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
