This is an automated email from the ASF dual-hosted git repository. dhuo pushed a commit to branch persistence-poc in repository https://gitbox.apache.org/repos/asf/polaris.git
commit 13ba1616c0e4a6f434e7d8befc6bb6e3c89c50c9 Author: Dennis Huo <[email protected]> AuthorDate: Wed Feb 26 06:46:50 2025 +0000 Turn PolarisMetaStoreSession into an abstract class and make lookupEntityActive protected-visibility; remove all callsites where PolarisMetaStoreManagerImpl calls it. Technically, while in the same package this doesn't prevent it from leaking, but we could reposition PolarisMetaStoreSession into a separate transaction-specific package to help protect it from leaking the lower-level abstractions. --- .../PolarisEclipseLinkMetaStoreSessionImpl.java | 2 +- .../polaris/core/persistence/BasePersistence.java | 23 ++++++++++++ .../persistence/PolarisMetaStoreManagerImpl.java | 40 +++++++++----------- .../core/persistence/PolarisMetaStoreSession.java | 43 +++++++++++++++------- .../PolarisTreeMapMetaStoreSessionImpl.java | 2 +- 5 files changed, 71 insertions(+), 39 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 621993e7..7de38105 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -68,7 +68,7 @@ import org.slf4j.LoggerFactory; * EclipseLink implementation of a Polaris metadata store supporting persisting and retrieving all * Polaris metadata from/to the configured database systems. */ -public class PolarisEclipseLinkMetaStoreSessionImpl implements PolarisMetaStoreSession { +public class PolarisEclipseLinkMetaStoreSessionImpl extends PolarisMetaStoreSession { private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEclipseLinkMetaStoreSessionImpl.class); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index a4283bf5..9ac8268b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -145,6 +145,29 @@ public interface BasePersistence { int typeCode, @Nonnull String name); + /** + * Looks up just the entity's subType and id given it catalogId, parentId, typeCode, and name. + * + * @param callCtx call context + * @param catalogId catalog id or NULL_ID + * @param parentId id of the parent, either a namespace or a catalog + * @param typeCode the PolarisEntityType code of the entity to lookup + * @param name the name of the entity + * @return null if the specified entity does not exist + */ + default PolarisEntityActiveRecord lookupEntityIdAndSubTypeByName( + @Nonnull PolarisCallContext callCtx, + long catalogId, + long parentId, + int typeCode, + @Nonnull String name) { + PolarisBaseEntity baseEntity = lookupEntityByName(callCtx, catalogId, parentId, typeCode, name); + if (baseEntity == null) { + return null; + } + return new PolarisEntityActiveRecord(baseEntity); + } + /** * Lookup a set of entities given their catalog id/entity id unique identifier * diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java index 7a016854..9fa92514 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java @@ -39,7 +39,6 @@ import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.entity.AsyncTaskType; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; -import org.apache.polaris.core.entity.PolarisEntitiesActiveKey; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntityActiveRecord; import org.apache.polaris.core.entity.PolarisEntityConstants; @@ -499,16 +498,14 @@ public class PolarisMetaStoreManagerImpl extends BaseMetaStoreManager { } // check that a catalog with the same name does not exist already - PolarisEntitiesActiveKey catalogNameKey = - new PolarisEntitiesActiveKey( + // if it exists, this is an error, the client should retry + if (ms.lookupEntityIdAndSubTypeByName( + callCtx, PolarisEntityConstants.getNullId(), PolarisEntityConstants.getRootEntityId(), PolarisEntityType.CATALOG.getCode(), - catalog.getName()); - PolarisEntityActiveRecord otherCatalogRecord = ms.lookupEntityActive(callCtx, catalogNameKey); - - // if it exists, this is an error, the client should retry - if (otherCatalogRecord != null) { + catalog.getName()) + != null) { return new CreateCatalogResult(BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS, null); } @@ -886,17 +883,14 @@ public class PolarisMetaStoreManagerImpl extends BaseMetaStoreManager { } // check that a principal with the same name does not exist already - PolarisEntitiesActiveKey principalNameKey = - new PolarisEntitiesActiveKey( + // if it exists, this is an error, the client should retry + if (ms.lookupEntityIdAndSubTypeByName( + callCtx, PolarisEntityConstants.getNullId(), PolarisEntityConstants.getRootEntityId(), PolarisEntityType.PRINCIPAL.getCode(), - principal.getName()); - PolarisEntityActiveRecord otherPrincipalRecord = - ms.lookupEntityActive(callCtx, principalNameKey); - - // if it exists, this is an error, the client should retry - if (otherPrincipalRecord != null) { + principal.getName()) + != null) { return new CreatePrincipalResult(BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS, null); } @@ -1087,13 +1081,13 @@ public class PolarisMetaStoreManagerImpl extends BaseMetaStoreManager { } // check if an entity does not already exist with the same name. If true, this is an error - PolarisEntitiesActiveKey entityActiveKey = - new PolarisEntitiesActiveKey( + PolarisEntityActiveRecord entityActiveRecord = + ms.lookupEntityIdAndSubTypeByName( + callCtx, entity.getCatalogId(), entity.getParentId(), entity.getType().getCode(), entity.getName()); - PolarisEntityActiveRecord entityActiveRecord = ms.lookupEntityActive(callCtx, entityActiveKey); if (entityActiveRecord != null) { return new EntityResult( BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS, entityActiveRecord.getSubTypeCode()); @@ -1317,14 +1311,14 @@ public class PolarisMetaStoreManagerImpl extends BaseMetaStoreManager { } // ensure that nothing exists where we create that entity - PolarisEntitiesActiveKey entityActiveKey = - new PolarisEntitiesActiveKey( + // if this entity already exists, this is an error + PolarisEntityActiveRecord entityActiveRecord = + ms.lookupEntityIdAndSubTypeByName( + callCtx, resolver.getCatalogIdOrNull(), resolver.getParentId(), refreshEntityToRename.getTypeCode(), renamedEntity.getName()); - // if this entity already exists, this is an error - PolarisEntityActiveRecord entityActiveRecord = ms.lookupEntityActive(callCtx, entityActiveKey); if (entityActiveRecord != null) { return new EntityResult( BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS, entityActiveRecord.getSubTypeCode()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java index 3a871ffb..883ac0f2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java @@ -30,9 +30,10 @@ import org.apache.polaris.core.entity.PolarisEntityCore; /** * Extends BasePersistence to express a more "transaction-oriented" control flow for backing stores - * which can support a runInTransaction semantic. + * which can support a runInTransaction semantic, while providing default implementations of some of + * the BasePersistence methods in terms of lower-level methods that subclasses must implement. */ -public interface PolarisMetaStoreSession extends BasePersistence { +public abstract class PolarisMetaStoreSession implements BasePersistence { /** * Run the specified transaction code (a Supplier lambda type) in a database read/write @@ -44,7 +45,8 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param callCtx call context * @param transactionCode code of the transaction being executed, a supplier lambda */ - <T> T runInTransaction(@Nonnull PolarisCallContext callCtx, @Nonnull Supplier<T> transactionCode); + public abstract <T> T runInTransaction( + @Nonnull PolarisCallContext callCtx, @Nonnull Supplier<T> transactionCode); /** * Run the specified transaction code (a runnable lambda type) in a database read/write @@ -55,7 +57,7 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param callCtx call context * @param transactionCode code of the transaction being executed, a runnable lambda */ - void runActionInTransaction( + public abstract void runActionInTransaction( @Nonnull PolarisCallContext callCtx, @Nonnull Runnable transactionCode); /** @@ -67,7 +69,7 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param callCtx call context * @param transactionCode code of the transaction being executed, a supplier lambda */ - <T> T runInReadTransaction( + public abstract <T> T runInReadTransaction( @Nonnull PolarisCallContext callCtx, @Nonnull Supplier<T> transactionCode); /** @@ -78,12 +80,12 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param callCtx call context * @param transactionCode code of the transaction being executed, a runnable lambda */ - void runActionInReadTransaction( + public abstract void runActionInReadTransaction( @Nonnull PolarisCallContext callCtx, @Nonnull Runnable transactionCode); /** {@inheritDoc} */ @Override - default PolarisBaseEntity lookupEntityByName( + public PolarisBaseEntity lookupEntityByName( @Nonnull PolarisCallContext callCtx, long catalogId, long parentId, @@ -112,6 +114,19 @@ public interface PolarisMetaStoreSession extends BasePersistence { return entity; } + /** {@inheritDoc} */ + @Override + public PolarisEntityActiveRecord lookupEntityIdAndSubTypeByName( + @Nonnull PolarisCallContext callCtx, + long catalogId, + long parentId, + int typeCode, + @Nonnull String name) { + PolarisEntitiesActiveKey entityActiveKey = + new PolarisEntitiesActiveKey(catalogId, parentId, typeCode, name); + return lookupEntityActive(callCtx, entityActiveKey); + } + /** * Lookup an entity by entityActiveKey * @@ -120,7 +135,7 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @return null if the specified entity does not exist or has been dropped. */ @Nullable - PolarisEntityActiveRecord lookupEntityActive( + protected abstract PolarisEntityActiveRecord lookupEntityActive( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntitiesActiveKey entityActiveKey); /** @@ -130,7 +145,7 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @return the list of entityActiveKeys for the specified lookup operation */ @Nonnull - List<PolarisEntityActiveRecord> lookupEntityActiveBatch( + public abstract List<PolarisEntityActiveRecord> lookupEntityActiveBatch( @Nonnull PolarisCallContext callCtx, List<PolarisEntitiesActiveKey> entityActiveKeys); /** @@ -141,7 +156,7 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param entity entity record to write, potentially replacing an existing entity record with the * same key */ - void writeToEntitiesActive( + public abstract void writeToEntitiesActive( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisBaseEntity entity); /** @@ -152,7 +167,7 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param entity entity record to write, potentially replacing an existing entity record with the * same key */ - void writeToEntitiesChangeTracking( + public abstract void writeToEntitiesChangeTracking( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisBaseEntity entity); /** @@ -161,7 +176,7 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param callCtx call context * @param entity entity record to delete */ - void deleteFromEntitiesActive( + public abstract void deleteFromEntitiesActive( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore entity); /** @@ -170,9 +185,9 @@ public interface PolarisMetaStoreSession extends BasePersistence { * @param callCtx call context * @param entity entity record to delete */ - void deleteFromEntitiesChangeTracking( + public abstract void deleteFromEntitiesChangeTracking( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntityCore entity); /** Rollback the current transaction */ - void rollback(); + public abstract void rollback(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreSessionImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreSessionImpl.java index 536e81c7..8e29bc24 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreSessionImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreSessionImpl.java @@ -40,7 +40,7 @@ import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; -public class PolarisTreeMapMetaStoreSessionImpl implements PolarisMetaStoreSession { +public class PolarisTreeMapMetaStoreSessionImpl extends PolarisMetaStoreSession { // the TreeMap store to use private final PolarisTreeMapStore store;
