dennishuo commented on code in PR #1135:
URL: https://github.com/apache/polaris/pull/1135#discussion_r1985957696
##########
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:
If we want to reduce the diffs on the existing `PolarisMetaStoreManagerImpl`
class we could instead invert the semantics, as in unqualified `writeEntity`
could already always mean "in current transaction" since the current
implementations all have that semantic, and instead, we introduce the new
methods in `AtomicPersistence` to have `writeEntityAtomically`,
`dropEntityAtomically`, etc.
The problem with doing it inverted like that though is that the
`BasePersistence` methods are automatically the "InCurrentTxn" variants, so a
subclass that only implements AtomicPersistence would I suppose just throw
`UnsupportedOperationException` for all the methods from `BasePersistence`
which kind of defeats the purpose.
Alternatively, the `doFooAtomically` methods could be the methods declared
in `BasePersistence`.
In general it makes sense to have the "Atomic" operations in the base
interface and "Transaction" ones in the subinterface, because as
[AbstractTransactionalPersistence.java](https://github.com/apache/polaris/pull/1135/files#diff-718b00bacadc702326318cb44c43f49659c96f1c6ae86b1267fa6300973e82ae)
shows, all Transactional implementations can also achieve the semantics of
"Atomic Persistence", but not all implementations of Atomic Persistence" can
support transactional styles.
--
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]
