From efbb6661e93cbea1f14c09f5b45e0841fab89c60 Mon Sep 17 00:00:00 2001 From: Dennis Huo Date: Mon, 24 Feb 2025 02:41:51 +0000 Subject: [PATCH 1/2] Remove all tightly coupled EntityCache dependencies in the main persistence stack Remove the EntityCacheEntry wrapper since the timestamp fields were never used anyways; instead the underlying Caffeine cache transparently handles access times, and the types of entries we cache are simply the existing ResolvedPolarisEntity. Remove interactions of business logic with explicit "cache entries", instead operating on ResolvedPolarisEntity. Fix the equals()/hashCode() behavior of PolarisEntity to be compatible with PolarisBaseEntity as intended. Improve code comments to explain the (current) relationship between PolarisEntity and PolarisBaseEntity, and clarify the behaviors in Resolver.java. Fully remove the PolarisRemoteCache interface and its methods. Add different methods that aren't cache-specific instead. --- .../polaris/core/entity/PolarisEntity.java | 52 ++-- .../persistence/PolarisMetaStoreManager.java | 203 ++++++++++++++- .../PolarisMetaStoreManagerImpl.java | 40 +-- .../persistence/ResolvedPolarisEntity.java | 67 ++++- .../TransactionWorkspaceMetaStoreManager.java | 12 +- .../core/persistence/cache/EntityCache.java | 79 +++--- .../persistence/cache/EntityCacheEntry.java | 129 ---------- .../cache/EntityCacheLookupResult.java | 7 +- .../persistence/cache/PolarisRemoteCache.java | 232 ------------------ .../resolver/PolarisResolutionManifest.java | 50 ++-- .../core/persistence/resolver/Resolver.java | 222 +++++++++-------- .../core/persistence/EntityCacheTest.java | 17 +- .../core/persistence/ResolverTest.java | 42 ++-- .../PolarisTestMetaStoreManager.java | 22 +- .../catalog/IcebergCatalogAdapter.java | 4 +- 15 files changed, 526 insertions(+), 652 deletions(-) delete mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheEntry.java delete mode 100644 polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/PolarisRemoteCache.java diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java index 5f0a7ad651..3ded968af2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java @@ -25,12 +25,23 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; import java.util.stream.Collectors; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +/** + * For legacy reasons, this class is only a thin facade over PolarisBaseEntity's members/methods. No + * direct members should be added to this class; rather, they should reside in the PolarisBaseEntity + * and this class should just contain the relevant builder methods, etc. The intention when using + * this class is to use "immutable" semantics as much as possible, for example constructing new + * copies with the Builder pattern when "mutating" fields rather than ever chaing fields in-place. + * Currently, code that intends to operate directly on a PolarisBaseEntity may not adhere to + * immutability semantics, and may modify the entity in-place. + * + *

TODO: Combine this fully into PolarisBaseEntity, refactor all callsites to use strict + * immutability semantics, and remove all mutator methods from PolarisBaseEntity. + */ public class PolarisEntity extends PolarisBaseEntity { public static class NameAndId { @@ -227,41 +238,18 @@ public String toString() { @Override public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof PolarisEntity)) return false; - PolarisEntity that = (PolarisEntity) o; - return catalogId == that.catalogId - && id == that.id - && parentId == that.parentId - && createTimestamp == that.createTimestamp - && dropTimestamp == that.dropTimestamp - && purgeTimestamp == that.purgeTimestamp - && lastUpdateTimestamp == that.lastUpdateTimestamp - && entityVersion == that.entityVersion - && grantRecordsVersion == that.grantRecordsVersion - && typeCode == that.typeCode - && subTypeCode == that.subTypeCode - && Objects.equals(name, that.name) - && Objects.equals(properties, that.properties) - && Objects.equals(internalProperties, that.internalProperties); + // Note: Keeping this here explicitly instead silently inheriting super.equals as a more + // prominent warning that the data members of this class *must not* diverge from those of + // PolarisBaseEntity. + return super.equals(o); } @Override public int hashCode() { - return Objects.hash( - typeCode, - subTypeCode, - catalogId, - id, - parentId, - name, - createTimestamp, - dropTimestamp, - purgeTimestamp, - lastUpdateTimestamp, - properties, - internalProperties, - entityVersion, - grantRecordsVersion); + // Note: Keeping this here explicitly instead silently inheriting super.hashCode as a more + // prominent warning that the data members of this class *must not* diverge from those of + // PolarisBaseEntity. + return super.hashCode(); } public static class Builder extends BaseBuilder { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java index 69652cb872..30c72a26a1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java @@ -29,13 +29,15 @@ import org.apache.polaris.core.auth.PolarisGrantManager; import org.apache.polaris.core.auth.PolarisSecretsManager; import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntityActiveRecord; import org.apache.polaris.core.entity.PolarisEntityCore; +import org.apache.polaris.core.entity.PolarisEntityId; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; -import org.apache.polaris.core.persistence.cache.PolarisRemoteCache; import org.apache.polaris.core.storage.PolarisCredentialVendor; /** @@ -43,10 +45,7 @@ * authorization. It uses the underlying persistent metastore to store and retrieve Polaris metadata */ public interface PolarisMetaStoreManager - extends PolarisSecretsManager, - PolarisGrantManager, - PolarisRemoteCache, - PolarisCredentialVendor { + extends PolarisSecretsManager, PolarisGrantManager, PolarisCredentialVendor { /** * Bootstrap the Polaris service, creating the root catalog, root principal, and associated @@ -688,4 +687,198 @@ DropEntityResult dropEntityIfExists( */ @Nonnull EntitiesResult loadTasks(@Nonnull PolarisCallContext callCtx, String executorId, int limit); + + /** Result of a loadEntitiesChangeTracking call */ + class ChangeTrackingResult extends BaseResult { + + // null if not success. Else, will be null if the grant to revoke was not found + private final List changeTrackingVersions; + + /** + * Constructor for an error + * + * @param errorCode error code, cannot be SUCCESS + * @param extraInformation extra information + */ + public ChangeTrackingResult( + @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { + super(errorCode, extraInformation); + this.changeTrackingVersions = null; + } + + /** + * Constructor for success + * + * @param changeTrackingVersions change tracking versions + */ + public ChangeTrackingResult( + @Nonnull List changeTrackingVersions) { + super(BaseResult.ReturnStatus.SUCCESS); + this.changeTrackingVersions = changeTrackingVersions; + } + + @JsonCreator + private ChangeTrackingResult( + @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, + @JsonProperty("extraInformation") String extraInformation, + @JsonProperty("changeTrackingVersions") + List changeTrackingVersions) { + super(returnStatus, extraInformation); + this.changeTrackingVersions = changeTrackingVersions; + } + + public List getChangeTrackingVersions() { + return changeTrackingVersions; + } + } + + /** + * Load change tracking information for a set of entities in one single shot and return for each + * the version for the entity itself and the version associated to its grant records. + * + * @param callCtx call context + * @param entityIds list of catalog/entity pair ids for which we need to efficiently load the + * version information, both entity version and grant records version. + * @return a list of version tracking information. Order in that returned list is the same as the + * input list. Some elements might be NULL if the entity has been purged. Not expected to fail + */ + @Nonnull + ChangeTrackingResult loadEntitiesChangeTracking( + @Nonnull PolarisCallContext callCtx, @Nonnull List entityIds); + + /** + * Represents an entity with its grants. If we "refresh" a previously fetched entity, we will only + * refresh the information which has changed, based on the version of the entity. + */ + class ResolvedEntityResult extends BaseResult { + + // the entity itself if it was loaded + private final @Nullable PolarisBaseEntity entity; + + // version for the grant records, in case the entity was not loaded + private final int grantRecordsVersion; + + private final @Nullable List entityGrantRecords; + + /** + * Constructor for an error + * + * @param errorCode error code, cannot be SUCCESS + * @param extraInformation extra information + */ + public ResolvedEntityResult( + @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { + super(errorCode, extraInformation); + this.entity = null; + this.entityGrantRecords = null; + this.grantRecordsVersion = 0; + } + + /** + * Constructor with success + * + * @param entity the main entity + * @param grantRecordsVersion the version of the grant records + * @param entityGrantRecords the list of grant records + */ + public ResolvedEntityResult( + @Nullable PolarisBaseEntity entity, + int grantRecordsVersion, + @Nullable List entityGrantRecords) { + super(BaseResult.ReturnStatus.SUCCESS); + this.entity = entity; + this.entityGrantRecords = entityGrantRecords; + this.grantRecordsVersion = grantRecordsVersion; + } + + @JsonCreator + public ResolvedEntityResult( + @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, + @JsonProperty("extraInformation") String extraInformation, + @Nullable @JsonProperty("entity") PolarisBaseEntity entity, + @JsonProperty("grantRecordsVersion") int grantRecordsVersion, + @Nullable @JsonProperty("entityGrantRecords") List entityGrantRecords) { + super(returnStatus, extraInformation); + this.entity = entity; + this.entityGrantRecords = entityGrantRecords; + this.grantRecordsVersion = grantRecordsVersion; + } + + public @Nullable PolarisBaseEntity getEntity() { + return entity; + } + + public int getGrantRecordsVersion() { + return grantRecordsVersion; + } + + public @Nullable List getEntityGrantRecords() { + return entityGrantRecords; + } + } + + /** + * Load a resolved entity, i.e. an entity definition and associated grant records, from the + * backend store. The entity is identified by its id (entity catalog id and id). + * + *

For entities that can be grantees, the associated grant records will include both the grant + * records for this entity as a grantee and for this entity as a securable. + * + * @param callCtx call context + * @param entityCatalogId id of the catalog for that entity + * @param entityId id of the entity + * @return result with entity and grants. Status will be ENTITY_NOT_FOUND if the entity was not + * found + */ + @Nonnull + ResolvedEntityResult loadResolvedEntityById( + @Nonnull PolarisCallContext callCtx, long entityCatalogId, long entityId); + + /** + * Load a resolved entity, i.e. an entity definition and associated grant records, from the + * backend store. The entity is identified by its name. Will return NULL if the entity does not + * exist, i.e. has been purged or dropped. + * + *

For entities that can be grantees, the associated grant records will include both the grant + * records for this entity as a grantee and for this entity as a securable. + * + * @param callCtx call context + * @param entityCatalogId id of the catalog for that entity + * @param parentId the id of the parent of that entity + * @param entityType the type of this entity + * @param entityName the name of this entity + * @return result with entity and grants. Status will be ENTITY_NOT_FOUND if the entity was not + * found + */ + @Nonnull + ResolvedEntityResult loadResolvedEntityByName( + @Nonnull PolarisCallContext callCtx, + long entityCatalogId, + long parentId, + @Nonnull PolarisEntityType entityType, + @Nonnull String entityName); + + /** + * Refresh a resolved entity from the backend store. Will return NULL if the entity does not + * exist, i.e. has been purged or dropped. Else, will determine what has changed based on the + * version information sent by the caller and will return only what has changed. + * + *

For entities that can be grantees, the associated grant records will include both the grant + * records for this entity as a grantee and for this entity as a securable. + * + * @param callCtx call context + * @param entityType type of the entity whose entity and grants we are refreshing + * @param entityCatalogId id of the catalog for that entity + * @param entityId the id of the entity to load + * @return result with entity and grants. Status will be ENTITY_NOT_FOUND if the entity was not + * found + */ + @Nonnull + ResolvedEntityResult refreshResolvedEntity( + @Nonnull PolarisCallContext callCtx, + int entityVersion, + int entityGrantRecordsVersion, + @Nonnull PolarisEntityType entityType, + long entityCatalogId, + long entityId); } 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 b90c7e51cf..98fd7789ca 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 @@ -2211,8 +2211,8 @@ public Map getInternalPropertyMap( return deserializeProperties(callCtx, internalPropStr); } - /** {@link #loadCachedEntryById(PolarisCallContext, long, long)} */ - private @Nonnull CachedEntryResult loadCachedEntryById( + /** {@link #loadResolvedEntityById(PolarisCallContext, long, long)} */ + private @Nonnull ResolvedEntityResult loadResolvedEntityById( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisMetaStoreSession ms, long entityCatalogId, @@ -2223,7 +2223,7 @@ public Map getInternalPropertyMap( // if entity not found, return null if (entity == null) { - return new CachedEntryResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + return new ResolvedEntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } // load the grant records @@ -2237,23 +2237,23 @@ public Map getInternalPropertyMap( } // return the result - return new CachedEntryResult(entity, entity.getGrantRecordsVersion(), grantRecords); + return new ResolvedEntityResult(entity, entity.getGrantRecordsVersion(), grantRecords); } /** {@inheritDoc} */ @Override - public @Nonnull CachedEntryResult loadCachedEntryById( + public @Nonnull ResolvedEntityResult loadResolvedEntityById( @Nonnull PolarisCallContext callCtx, long entityCatalogId, long entityId) { // get metastore we should be using PolarisMetaStoreSession ms = callCtx.getMetaStore(); // need to run inside a read transaction return ms.runInReadTransaction( - callCtx, () -> this.loadCachedEntryById(callCtx, ms, entityCatalogId, entityId)); + callCtx, () -> this.loadResolvedEntityById(callCtx, ms, entityCatalogId, entityId)); } - /** {@link #loadCachedEntryById(PolarisCallContext, long, long)} */ - private @Nonnull CachedEntryResult loadCachedEntryByName( + /** {@link #loadResolvedEntityById(PolarisCallContext, long, long)} */ + private @Nonnull ResolvedEntityResult loadResolvedEntityByName( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisMetaStoreSession ms, long entityCatalogId, @@ -2268,7 +2268,7 @@ public Map getInternalPropertyMap( // null if entity not found if (entity == null) { - return new CachedEntryResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + return new ResolvedEntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } // load the grant records @@ -2284,12 +2284,12 @@ public Map getInternalPropertyMap( } // return the result - return new CachedEntryResult(entity, entity.getGrantRecordsVersion(), grantRecords); + return new ResolvedEntityResult(entity, entity.getGrantRecordsVersion(), grantRecords); } /** {@inheritDoc} */ @Override - public @Nonnull CachedEntryResult loadCachedEntryByName( + public @Nonnull ResolvedEntityResult loadResolvedEntityByName( @Nonnull PolarisCallContext callCtx, long entityCatalogId, long parentId, @@ -2299,11 +2299,11 @@ public Map getInternalPropertyMap( PolarisMetaStoreSession ms = callCtx.getMetaStore(); // need to run inside a read transaction - CachedEntryResult result = + ResolvedEntityResult result = ms.runInReadTransaction( callCtx, () -> - this.loadCachedEntryByName( + this.loadResolvedEntityByName( callCtx, ms, entityCatalogId, parentId, entityType, entityName)); if (PolarisEntityConstants.getRootContainerName().equals(entityName) && entityType == PolarisEntityType.ROOT @@ -2347,14 +2347,14 @@ public Map getInternalPropertyMap( ms.runInReadTransaction( callCtx, () -> - this.loadCachedEntryByName( + this.loadResolvedEntityByName( callCtx, ms, entityCatalogId, parentId, entityType, entityName)); } return result; } /** {@inheritDoc} */ - private @Nonnull CachedEntryResult refreshCachedEntity( + private @Nonnull ResolvedEntityResult refreshResolvedEntity( @Nonnull PolarisCallContext callCtx, @Nonnull PolarisMetaStoreSession ms, int entityVersion, @@ -2370,7 +2370,7 @@ public Map getInternalPropertyMap( // if null, the entity has been purged if (entityVersions == null) { - return new CachedEntryResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + return new ResolvedEntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } // load the entity if something changed @@ -2380,7 +2380,7 @@ public Map getInternalPropertyMap( // if not found, return null if (entity == null) { - return new CachedEntryResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + return new ResolvedEntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } } else { // entity has not changed, no need to reload it @@ -2402,12 +2402,12 @@ public Map getInternalPropertyMap( } // return the result - return new CachedEntryResult(entity, entityVersions.getGrantRecordsVersion(), grantRecords); + return new ResolvedEntityResult(entity, entityVersions.getGrantRecordsVersion(), grantRecords); } /** {@inheritDoc} */ @Override - public @Nonnull CachedEntryResult refreshCachedEntity( + public @Nonnull ResolvedEntityResult refreshResolvedEntity( @Nonnull PolarisCallContext callCtx, int entityVersion, int entityGrantRecordsVersion, @@ -2421,7 +2421,7 @@ public Map getInternalPropertyMap( return ms.runInReadTransaction( callCtx, () -> - this.refreshCachedEntity( + this.refreshResolvedEntity( callCtx, ms, entityVersion, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/ResolvedPolarisEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/ResolvedPolarisEntity.java index a091362ac2..7449b97395 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/ResolvedPolarisEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/ResolvedPolarisEntity.java @@ -18,12 +18,14 @@ */ package org.apache.polaris.core.persistence; -import com.google.common.collect.ImmutableList; import jakarta.annotation.Nonnull; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisGrantRecord; -import org.apache.polaris.core.persistence.cache.EntityCacheEntry; public class ResolvedPolarisEntity { private final PolarisEntity entity; @@ -37,6 +39,56 @@ public class ResolvedPolarisEntity { // these are the grants like TABLE_READ_PROPERTIES, NAMESPACE_LIST, etc. private final List grantRecordsAsSecurable; + /** + * Constructor used when an entry is initially created after loading the entity and its grants + * from the backend. + * + * @param diagnostics diagnostic services + * @param entity the entity which has just been loaded + * @param grantRecords associated grant records, including grants for this entity as a securable + * as well as grants for this entity as a grantee if applicable + * @param grantsVersion version of the grants when they were loaded + */ + public ResolvedPolarisEntity( + @Nonnull PolarisDiagnostics diagnostics, + @Nonnull PolarisBaseEntity entity, + @Nonnull List grantRecords, + int grantsVersion) { + // validate not null + diagnostics.checkNotNull(entity, "entity_null"); + diagnostics.checkNotNull(grantRecords, "grant_records_null"); + + // we copy all attributes of the entity to avoid any contamination + this.entity = PolarisEntity.of(entity); + + // if only the grant records have been reloaded because they were changed, the entity will + // have an old version for those. Patch the entity if this is the case, as if we had reloaded it + if (this.entity.getGrantRecordsVersion() != grantsVersion) { + // remember the grants versions. For now grants should be loaded after the entity, so expect + // grants version to be same or higher + diagnostics.check( + this.entity.getGrantRecordsVersion() <= grantsVersion, + "grants_version_going_backward", + "entity={} grantsVersion={}", + entity, + grantsVersion); + + // patch grant records version + this.entity.setGrantRecordsVersion(grantsVersion); + } + + // Split the combined list of grant records into grantee vs securable grants since the main + // usage pattern is to get the two lists separately. + this.grantRecordsAsGrantee = + grantRecords.stream() + .filter(record -> record.getGranteeId() == entity.getId()) + .collect(Collectors.toList()); + this.grantRecordsAsSecurable = + grantRecords.stream() + .filter(record -> record.getSecurableId() == entity.getId()) + .collect(Collectors.toList()); + } + public ResolvedPolarisEntity( PolarisEntity entity, List grantRecordsAsGrantee, @@ -48,16 +100,15 @@ public ResolvedPolarisEntity( this.grantRecordsAsSecurable = grantRecordsAsSecurable; } - public ResolvedPolarisEntity(@Nonnull EntityCacheEntry cacheEntry) { - this.entity = PolarisEntity.of(cacheEntry.getEntity()); - this.grantRecordsAsGrantee = ImmutableList.copyOf(cacheEntry.getGrantRecordsAsGrantee()); - this.grantRecordsAsSecurable = ImmutableList.copyOf(cacheEntry.getGrantRecordsAsSecurable()); - } - public PolarisEntity getEntity() { return entity; } + public @Nonnull List getAllGrantRecords() { + return Stream.concat(grantRecordsAsGrantee.stream(), grantRecordsAsSecurable.stream()) + .collect(Collectors.toList()); + } + /** The grant records associated with this entity being the grantee of the record. */ public List getGrantRecordsAsGrantee() { return grantRecordsAsGrantee; diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 44134751bc..0d7c72d298 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -335,16 +335,16 @@ public ValidateAccessResult validateAccessToLocations( } @Override - public CachedEntryResult loadCachedEntryById( + public ResolvedEntityResult loadResolvedEntityById( @Nonnull PolarisCallContext callCtx, long entityCatalogId, long entityId) { callCtx .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "loadCachedEntryById"); + .fail("illegal_method_in_transaction_workspace", "loadResolvedEntityById"); return null; } @Override - public CachedEntryResult loadCachedEntryByName( + public ResolvedEntityResult loadResolvedEntityByName( @Nonnull PolarisCallContext callCtx, long entityCatalogId, long parentId, @@ -352,12 +352,12 @@ public CachedEntryResult loadCachedEntryByName( @Nonnull String entityName) { callCtx .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "loadCachedEntryByName"); + .fail("illegal_method_in_transaction_workspace", "loadResolvedEntityByName"); return null; } @Override - public CachedEntryResult refreshCachedEntity( + public ResolvedEntityResult refreshResolvedEntity( @Nonnull PolarisCallContext callCtx, int entityVersion, int entityGrantRecordsVersion, @@ -366,7 +366,7 @@ public CachedEntryResult refreshCachedEntity( long entityId) { callCtx .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "refreshCachedEntity"); + .fail("illegal_method_in_transaction_workspace", "refreshResolvedEntity"); return null; } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCache.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCache.java index 05efba7fb7..791c43b2c0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCache.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCache.java @@ -31,7 +31,9 @@ import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; -import org.apache.polaris.core.persistence.cache.PolarisRemoteCache.CachedEntryResult; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager.ResolvedEntityResult; +import org.apache.polaris.core.persistence.ResolvedPolarisEntity; /** The entity cache, can be private or shared */ public class EntityCache { @@ -40,26 +42,26 @@ public class EntityCache { private EntityCacheMode cacheMode; // the meta store manager - private final PolarisRemoteCache polarisRemoteCache; + private final PolarisMetaStoreManager polarisMetaStoreManager; // Caffeine cache to keep entries by id - private final Cache byId; + private final Cache byId; // index by name - private final AbstractMap byName; + private final AbstractMap byName; /** * Constructor. Cache can be private or shared * - * @param polarisRemoteCache the meta store manager implementation + * @param polarisMetaStoreManager the meta store manager implementation */ - public EntityCache(@Nonnull PolarisRemoteCache polarisRemoteCache) { + public EntityCache(@Nonnull PolarisMetaStoreManager polarisMetaStoreManager) { // by name cache this.byName = new ConcurrentHashMap<>(); // When an entry is removed, we simply remove it from the byName map - RemovalListener removalListener = + RemovalListener removalListener = (key, value, cause) -> { if (value != null) { // compute name key @@ -80,7 +82,7 @@ public EntityCache(@Nonnull PolarisRemoteCache polarisRemoteCache) { .build(); // remember the meta store manager - this.polarisRemoteCache = polarisRemoteCache; + this.polarisMetaStoreManager = polarisMetaStoreManager; // enabled by default this.cacheMode = EntityCacheMode.ENABLE; @@ -91,7 +93,7 @@ public EntityCache(@Nonnull PolarisRemoteCache polarisRemoteCache) { * * @param cacheEntry cache entry to remove */ - public void removeCacheEntry(@Nonnull EntityCacheEntry cacheEntry) { + public void removeCacheEntry(@Nonnull ResolvedPolarisEntity cacheEntry) { // compute name key EntityCacheByNameKey nameKey = new EntityCacheByNameKey(cacheEntry.getEntity()); @@ -107,13 +109,13 @@ public void removeCacheEntry(@Nonnull EntityCacheEntry cacheEntry) { * * @param cacheEntry new cache entry */ - private void cacheNewEntry(@Nonnull EntityCacheEntry cacheEntry) { + private void cacheNewEntry(@Nonnull ResolvedPolarisEntity cacheEntry) { // compute name key EntityCacheByNameKey nameKey = new EntityCacheByNameKey(cacheEntry.getEntity()); // get old value if one exist - EntityCacheEntry oldCacheEntry = this.byId.getIfPresent(cacheEntry.getEntity().getId()); + ResolvedPolarisEntity oldCacheEntry = this.byId.getIfPresent(cacheEntry.getEntity().getId()); // put new entry, only if really newer one this.byId @@ -147,7 +149,7 @@ private void cacheNewEntry(@Nonnull EntityCacheEntry cacheEntry) { * @param oldValue old cache entry * @return true if the newer cache entry */ - private boolean isNewer(EntityCacheEntry newValue, EntityCacheEntry oldValue) { + private boolean isNewer(ResolvedPolarisEntity newValue, ResolvedPolarisEntity oldValue) { return (newValue.getEntity().getEntityVersion() > oldValue.getEntity().getEntityVersion() || newValue.getEntity().getGrantRecordsVersion() > oldValue.getEntity().getGrantRecordsVersion()); @@ -160,7 +162,7 @@ private boolean isNewer(EntityCacheEntry newValue, EntityCacheEntry oldValue) { * @param newCacheEntry new entry */ private void replaceCacheEntry( - @Nullable EntityCacheEntry oldCacheEntry, @Nonnull EntityCacheEntry newCacheEntry) { + @Nullable ResolvedPolarisEntity oldCacheEntry, @Nonnull ResolvedPolarisEntity newCacheEntry) { // need to remove old? if (oldCacheEntry != null) { @@ -175,8 +177,6 @@ private void replaceCacheEntry( // delete the old one assuming it has not been replaced by the above new entry this.removeCacheEntry(oldCacheEntry); - } else { - oldCacheEntry.updateLastAccess(); } } else { // write new one @@ -223,7 +223,7 @@ public void setCacheMode(EntityCacheMode cacheMode) { * @param entityId entity id * @return the cache entry or null if not found */ - public @Nullable EntityCacheEntry getEntityById(long entityId) { + public @Nullable ResolvedPolarisEntity getEntityById(long entityId) { return byId.getIfPresent(entityId); } @@ -233,7 +233,8 @@ public void setCacheMode(EntityCacheMode cacheMode) { * @param entityNameKey entity name key * @return the cache entry or null if not found */ - public @Nullable EntityCacheEntry getEntityByName(@Nonnull EntityCacheByNameKey entityNameKey) { + public @Nullable ResolvedPolarisEntity getEntityByName( + @Nonnull EntityCacheByNameKey entityNameKey) { return byName.get(entityNameKey); } @@ -249,7 +250,7 @@ public void setCacheMode(EntityCacheMode cacheMode) { * records should be reloaded if needed * @return the cache entry for the entity or null if the specified entity does not exist */ - public @Nullable EntityCacheEntry getAndRefreshIfNeeded( + public @Nullable ResolvedPolarisEntity getAndRefreshIfNeeded( @Nonnull PolarisCallContext callContext, @Nonnull PolarisBaseEntity entityToValidate, int entityMinVersion, @@ -259,13 +260,13 @@ public void setCacheMode(EntityCacheMode cacheMode) { PolarisEntityType entityType = entityToValidate.getType(); // first lookup the cache to find the existing cache entry - EntityCacheEntry existingCacheEntry = this.getEntityById(entityId); + ResolvedPolarisEntity existingCacheEntry = this.getEntityById(entityId); // the caller's fetched entity may have come from a stale lookup byName; we should consider // the existingCacheEntry to be the older of the two for purposes of invalidation to make // sure when we replaceCacheEntry we're also removing the old name if it's no longer valid EntityCacheByNameKey nameKey = new EntityCacheByNameKey(entityToValidate); - EntityCacheEntry existingCacheEntryByName = this.getEntityByName(nameKey); + ResolvedPolarisEntity existingCacheEntryByName = this.getEntityByName(nameKey); if (existingCacheEntryByName != null && existingCacheEntry != null && isNewer(existingCacheEntry, existingCacheEntryByName)) { @@ -273,7 +274,7 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { } // the new one to be returned - final EntityCacheEntry newCacheEntry; + final ResolvedPolarisEntity newCacheEntry; // see if we need to load or refresh that entity if (existingCacheEntry == null @@ -281,7 +282,7 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { || existingCacheEntry.getEntity().getGrantRecordsVersion() < entityGrantRecordsMinVersion) { // the refreshed entity - final CachedEntryResult refreshedCacheEntry; + final ResolvedEntityResult refreshedCacheEntry; // was not found in the cache? final PolarisBaseEntity entity; @@ -290,7 +291,8 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { if (existingCacheEntry == null) { // try to load it refreshedCacheEntry = - this.polarisRemoteCache.loadCachedEntryById(callContext, entityCatalogId, entityId); + this.polarisMetaStoreManager.loadResolvedEntityById( + callContext, entityCatalogId, entityId); if (refreshedCacheEntry.isSuccess()) { entity = refreshedCacheEntry.getEntity(); grantRecords = refreshedCacheEntry.getEntityGrantRecords(); @@ -301,7 +303,7 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { } else { // refresh it refreshedCacheEntry = - this.polarisRemoteCache.refreshCachedEntity( + this.polarisMetaStoreManager.refreshResolvedEntity( callContext, existingCacheEntry.getEntity().getEntityVersion(), existingCacheEntry.getEntity().getGrantRecordsVersion(), @@ -336,20 +338,13 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { // create new cache entry newCacheEntry = - new EntityCacheEntry( - callContext.getDiagServices(), - existingCacheEntry == null - ? System.nanoTime() - : existingCacheEntry.getCreatedOnNanoTimestamp(), - entity, - grantRecords, - grantRecordsVersion); + new ResolvedPolarisEntity( + callContext.getDiagServices(), entity, grantRecords, grantRecordsVersion); // insert cache entry this.replaceCacheEntry(existingCacheEntry, newCacheEntry); } else { // found it in the cache and it is up-to-date, simply return it - existingCacheEntry.updateLastAccess(); newCacheEntry = existingCacheEntry; } @@ -369,7 +364,7 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { @Nonnull PolarisCallContext callContext, long entityCatalogId, long entityId) { // if it exists, we are set - EntityCacheEntry entry = this.getEntityById(entityId); + ResolvedPolarisEntity entry = this.getEntityById(entityId); final boolean cacheHit; // we need to load it if it does not exist @@ -378,8 +373,8 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { cacheHit = false; // load it - CachedEntryResult result = - polarisRemoteCache.loadCachedEntryById(callContext, entityCatalogId, entityId); + ResolvedEntityResult result = + polarisMetaStoreManager.loadResolvedEntityById(callContext, entityCatalogId, entityId); // not found, exit if (!result.isSuccess()) { @@ -392,9 +387,8 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { .getDiagServices() .checkNotNull(result.getEntityGrantRecords(), "entity_grant_records_should_loaded"); entry = - new EntityCacheEntry( + new ResolvedPolarisEntity( callContext.getDiagServices(), - System.nanoTime(), result.getEntity(), result.getEntityGrantRecords(), result.getGrantRecordsVersion()); @@ -421,7 +415,7 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { @Nonnull PolarisCallContext callContext, @Nonnull EntityCacheByNameKey entityNameKey) { // if it exists, we are set - EntityCacheEntry entry = this.getEntityByName(entityNameKey); + ResolvedPolarisEntity entry = this.getEntityByName(entityNameKey); final boolean cacheHit; // we need to load it if it does not exist @@ -430,8 +424,8 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { cacheHit = false; // load it - CachedEntryResult result = - polarisRemoteCache.loadCachedEntryByName( + ResolvedEntityResult result = + polarisMetaStoreManager.loadResolvedEntityByName( callContext, entityNameKey.getCatalogId(), entityNameKey.getParentId(), @@ -451,9 +445,8 @@ && isNewer(existingCacheEntry, existingCacheEntryByName)) { // if found, setup entry entry = - new EntityCacheEntry( + new ResolvedPolarisEntity( callContext.getDiagServices(), - System.nanoTime(), result.getEntity(), result.getEntityGrantRecords(), result.getGrantRecordsVersion()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheEntry.java deleted file mode 100644 index ca1a7e4e33..0000000000 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheEntry.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.polaris.core.persistence.cache; - -import com.google.common.collect.ImmutableList; -import jakarta.annotation.Nonnull; -import java.util.List; -import java.util.stream.Collectors; -import org.apache.polaris.core.PolarisDiagnostics; -import org.apache.polaris.core.entity.PolarisBaseEntity; -import org.apache.polaris.core.entity.PolarisGrantRecord; - -/** An entry in our entity cache. Note, this is fully immutable */ -public class EntityCacheEntry { - - // epoch time (ns) when the cache entry was added to the cache - private final long createdOnNanoTimestamp; - - // epoch time (ns) when the cache entry was added to the cache - private long lastAccessedNanoTimestamp; - - // the entity which have been cached. - private final PolarisBaseEntity entity; - - // grants associated to this entity, for a principal, a principal role, or a catalog role these - // are role usage - // grants on that entity. For a catalog securable (i.e. a catalog, namespace, or table_like - // securable), these are - // the grants on this securable. - private final List grantRecords; - - /** - * Constructor used when an entry is initially created after loading the entity and its grants - * from the backend. - * - * @param diagnostics diagnostic services - * @param createdOnNanoTimestamp when the entity was created - * @param entity the entity which has just been loaded - * @param grantRecords associated grant records, including grants for this entity as a securable - * as well as grants for this entity as a grantee if applicable - * @param grantsVersion version of the grants when they were loaded - */ - EntityCacheEntry( - @Nonnull PolarisDiagnostics diagnostics, - long createdOnNanoTimestamp, - @Nonnull PolarisBaseEntity entity, - @Nonnull List grantRecords, - int grantsVersion) { - // validate not null - diagnostics.checkNotNull(entity, "entity_null"); - diagnostics.checkNotNull(grantRecords, "grant_records_null"); - - // when this entry has been created - this.createdOnNanoTimestamp = createdOnNanoTimestamp; - - // last accessed time is now - this.lastAccessedNanoTimestamp = System.nanoTime(); - - // we copy all attributes of the entity to avoid any contamination - this.entity = new PolarisBaseEntity(entity); - - // if only the grant records have been reloaded because they were changed, the entity will - // have an old version for those. Patch the entity if this is the case, as if we had reloaded it - if (this.entity.getGrantRecordsVersion() != grantsVersion) { - // remember the grants versions. For now grants should be loaded after the entity, so expect - // grants version to be same or higher - diagnostics.check( - this.entity.getGrantRecordsVersion() <= grantsVersion, - "grants_version_going_backward", - "entity={} grantsVersion={}", - entity, - grantsVersion); - - // patch grant records version - this.entity.setGrantRecordsVersion(grantsVersion); - } - - // the grants - this.grantRecords = ImmutableList.copyOf(grantRecords); - } - - public long getCreatedOnNanoTimestamp() { - return createdOnNanoTimestamp; - } - - public long getLastAccessedNanoTimestamp() { - return lastAccessedNanoTimestamp; - } - - public @Nonnull PolarisBaseEntity getEntity() { - return entity; - } - - public @Nonnull List getAllGrantRecords() { - return grantRecords; - } - - public @Nonnull List getGrantRecordsAsGrantee() { - return grantRecords.stream() - .filter(record -> record.getGranteeId() == entity.getId()) - .collect(Collectors.toList()); - } - - public @Nonnull List getGrantRecordsAsSecurable() { - return grantRecords.stream() - .filter(record -> record.getSecurableId() == entity.getId()) - .collect(Collectors.toList()); - } - - public void updateLastAccess() { - this.lastAccessedNanoTimestamp = System.nanoTime(); - } -} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheLookupResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheLookupResult.java index a4677baa92..415fdd29b0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheLookupResult.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCacheLookupResult.java @@ -19,23 +19,24 @@ package org.apache.polaris.core.persistence.cache; import jakarta.annotation.Nullable; +import org.apache.polaris.core.persistence.ResolvedPolarisEntity; /** Result of a lookup operation */ public class EntityCacheLookupResult { // if not null, we found the entity and this is the entry. If not found, the entity was dropped or // does not exist - private final @Nullable EntityCacheEntry cacheEntry; + private final @Nullable ResolvedPolarisEntity cacheEntry; // true if the entity was found in the cache private final boolean cacheHit; - public EntityCacheLookupResult(@Nullable EntityCacheEntry cacheEntry, boolean cacheHit) { + public EntityCacheLookupResult(@Nullable ResolvedPolarisEntity cacheEntry, boolean cacheHit) { this.cacheEntry = cacheEntry; this.cacheHit = cacheHit; } - public @Nullable EntityCacheEntry getCacheEntry() { + public @Nullable ResolvedPolarisEntity getCacheEntry() { return cacheEntry; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/PolarisRemoteCache.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/PolarisRemoteCache.java deleted file mode 100644 index e45cc8b1df..0000000000 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/PolarisRemoteCache.java +++ /dev/null @@ -1,232 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.polaris.core.persistence.cache; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; -import jakarta.annotation.Nonnull; -import jakarta.annotation.Nullable; -import java.util.List; -import org.apache.polaris.core.PolarisCallContext; -import org.apache.polaris.core.entity.PolarisBaseEntity; -import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; -import org.apache.polaris.core.entity.PolarisEntityId; -import org.apache.polaris.core.entity.PolarisEntityType; -import org.apache.polaris.core.entity.PolarisGrantRecord; -import org.apache.polaris.core.persistence.BaseResult; - -/** - * Interface to the remote entity cache. This allows the local cache to detect remote entity changes - * and refresh the local copies where necessary. - */ -public interface PolarisRemoteCache { - /** - * Load change tracking information for a set of entities in one single shot and return for each - * the version for the entity itself and the version associated to its grant records. - * - * @param callCtx call context - * @param entityIds list of catalog/entity pair ids for which we need to efficiently load the - * version information, both entity version and grant records version. - * @return a list of version tracking information. Order in that returned list is the same as the - * input list. Some elements might be NULL if the entity has been purged. Not expected to fail - */ - @Nonnull - ChangeTrackingResult loadEntitiesChangeTracking( - @Nonnull PolarisCallContext callCtx, @Nonnull List entityIds); - - /** - * Load a cached entry, i.e. an entity definition and associated grant records, from the backend - * store. The entity is identified by its id (entity catalog id and id). - * - *

For entities that can be grantees, the associated grant records will include both the grant - * records for this entity as a grantee and for this entity as a securable. - * - * @param callCtx call context - * @param entityCatalogId id of the catalog for that entity - * @param entityId id of the entity - * @return cached entry for this entity. Status will be ENTITY_NOT_FOUND if the entity was not - * found - */ - @Nonnull - CachedEntryResult loadCachedEntryById( - @Nonnull PolarisCallContext callCtx, long entityCatalogId, long entityId); - - /** - * Load a cached entry, i.e. an entity definition and associated grant records, from the backend - * store. The entity is identified by its name. Will return NULL if the entity does not exist, - * i.e. has been purged or dropped. - * - *

For entities that can be grantees, the associated grant records will include both the grant - * records for this entity as a grantee and for this entity as a securable. - * - * @param callCtx call context - * @param entityCatalogId id of the catalog for that entity - * @param parentId the id of the parent of that entity - * @param entityType the type of this entity - * @param entityName the name of this entity - * @return cached entry for this entity. Status will be ENTITY_NOT_FOUND if the entity was not - * found - */ - @Nonnull - CachedEntryResult loadCachedEntryByName( - @Nonnull PolarisCallContext callCtx, - long entityCatalogId, - long parentId, - @Nonnull PolarisEntityType entityType, - @Nonnull String entityName); - - /** - * Refresh a cached entity from the backend store. Will return NULL if the entity does not exist, - * i.e. has been purged or dropped. Else, will determine what has changed based on the version - * information sent by the caller and will return only what has changed. - * - *

For entities that can be grantees, the associated grant records will include both the grant - * records for this entity as a grantee and for this entity as a securable. - * - * @param callCtx call context - * @param entityType type of the entity whose cached entry we are refreshing - * @param entityCatalogId id of the catalog for that entity - * @param entityId the id of the entity to load - * @return cached entry for this entity. Status will be ENTITY_NOT_FOUND if the entity was not * - * found - */ - @Nonnull - CachedEntryResult refreshCachedEntity( - @Nonnull PolarisCallContext callCtx, - int entityVersion, - int entityGrantRecordsVersion, - @Nonnull PolarisEntityType entityType, - long entityCatalogId, - long entityId); - - /** Result of a loadEntitiesChangeTracking call */ - class ChangeTrackingResult extends BaseResult { - - // null if not success. Else, will be null if the grant to revoke was not found - private final List changeTrackingVersions; - - /** - * Constructor for an error - * - * @param errorCode error code, cannot be SUCCESS - * @param extraInformation extra information - */ - public ChangeTrackingResult( - @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { - super(errorCode, extraInformation); - this.changeTrackingVersions = null; - } - - /** - * Constructor for success - * - * @param changeTrackingVersions change tracking versions - */ - public ChangeTrackingResult( - @Nonnull List changeTrackingVersions) { - super(BaseResult.ReturnStatus.SUCCESS); - this.changeTrackingVersions = changeTrackingVersions; - } - - @JsonCreator - private ChangeTrackingResult( - @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, - @JsonProperty("extraInformation") String extraInformation, - @JsonProperty("changeTrackingVersions") - List changeTrackingVersions) { - super(returnStatus, extraInformation); - this.changeTrackingVersions = changeTrackingVersions; - } - - public List getChangeTrackingVersions() { - return changeTrackingVersions; - } - } - - /** - * Represents an entry in the cache. If we refresh a cached entry, we will only refresh the - * information which have changed, based on the version of the entity - */ - class CachedEntryResult extends BaseResult { - - // the entity itself if it was loaded - private final @Nullable PolarisBaseEntity entity; - - // version for the grant records, in case the entity was not loaded - private final int grantRecordsVersion; - - private final @Nullable List entityGrantRecords; - - /** - * Constructor for an error - * - * @param errorCode error code, cannot be SUCCESS - * @param extraInformation extra information - */ - public CachedEntryResult( - @Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) { - super(errorCode, extraInformation); - this.entity = null; - this.entityGrantRecords = null; - this.grantRecordsVersion = 0; - } - - /** - * Constructor with success - * - * @param entity the entity for that cached entry - * @param grantRecordsVersion the version of the grant records - * @param entityGrantRecords the list of grant records - */ - public CachedEntryResult( - @Nullable PolarisBaseEntity entity, - int grantRecordsVersion, - @Nullable List entityGrantRecords) { - super(BaseResult.ReturnStatus.SUCCESS); - this.entity = entity; - this.entityGrantRecords = entityGrantRecords; - this.grantRecordsVersion = grantRecordsVersion; - } - - @JsonCreator - public CachedEntryResult( - @JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus, - @JsonProperty("extraInformation") String extraInformation, - @Nullable @JsonProperty("entity") PolarisBaseEntity entity, - @JsonProperty("grantRecordsVersion") int grantRecordsVersion, - @Nullable @JsonProperty("entityGrantRecords") List entityGrantRecords) { - super(returnStatus, extraInformation); - this.entity = entity; - this.entityGrantRecords = entityGrantRecords; - this.grantRecordsVersion = grantRecordsVersion; - } - - public @Nullable PolarisBaseEntity getEntity() { - return entity; - } - - public int getGrantRecordsVersion() { - return grantRecordsVersion; - } - - public @Nullable List getEntityGrantRecords() { - return entityGrantRecords; - } - } -} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java index 2ab0140a07..d37e2ce2ff 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java @@ -39,7 +39,6 @@ import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; import org.apache.polaris.core.persistence.ResolvedPolarisEntity; -import org.apache.polaris.core.persistence.cache.EntityCacheEntry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -211,23 +210,22 @@ public PolarisResolvedPathWrapper getPassthroughResolvedPath(Object key) { return null; } - List resolvedPath = passthroughResolver.getResolvedPath(); + List resolvedPath = passthroughResolver.getResolvedPath(); if (requestedPath.isOptional()) { if (resolvedPath.size() != requestedPath.getEntityNames().size()) { LOGGER.debug( "Returning null for key {} due to size mismatch from getPassthroughResolvedPath " + "resolvedPath: {}, requestedPath.getEntityNames(): {}", key, - resolvedPath.stream().map(ResolvedPolarisEntity::new).collect(Collectors.toList()), + resolvedPath, requestedPath.getEntityNames()); return null; } } List resolvedEntities = new ArrayList<>(); - resolvedEntities.add( - new ResolvedPolarisEntity(passthroughResolver.getResolvedReferenceCatalog())); - resolvedPath.forEach(cacheEntry -> resolvedEntities.add(new ResolvedPolarisEntity(cacheEntry))); + resolvedEntities.add(passthroughResolver.getResolvedReferenceCatalog()); + resolvedPath.forEach(resolvedEntity -> resolvedEntities.add(resolvedEntity)); LOGGER.debug( "Returning resolvedEntities from getPassthroughResolvedPath: {}", resolvedEntities); return new PolarisResolvedPathWrapper(resolvedEntities); @@ -255,11 +253,11 @@ public PolarisResolvedPathWrapper getPassthroughResolvedPath( public Set getAllActivatedCatalogRoleAndPrincipalRoles() { Set activatedRoles = new HashSet<>(); primaryResolver.getResolvedCallerPrincipalRoles().stream() - .map(EntityCacheEntry::getEntity) + .map(ResolvedPolarisEntity::getEntity) .forEach(activatedRoles::add); if (primaryResolver.getResolvedCatalogRoles() != null) { primaryResolver.getResolvedCatalogRoles().values().stream() - .map(EntityCacheEntry::getEntity) + .map(ResolvedPolarisEntity::getEntity) .forEach(activatedRoles::add); } return activatedRoles; @@ -268,7 +266,7 @@ public Set getAllActivatedCatalogRoleAndPrincipalRoles() { public Set getAllActivatedPrincipalRoleEntities() { Set activatedEntities = new HashSet<>(); primaryResolver.getResolvedCallerPrincipalRoles().stream() - .map(EntityCacheEntry::getEntity) + .map(ResolvedPolarisEntity::getEntity) .forEach(activatedEntities::add); return activatedEntities; } @@ -282,14 +280,14 @@ private ResolvedPolarisEntity getResolvedRootContainerEntity() { if (primaryResolverStatus.getStatus() != ResolverStatus.StatusEnum.SUCCESS) { return null; } - EntityCacheEntry resolvedCacheEntry = + ResolvedPolarisEntity resolvedEntity = primaryResolver.getResolvedEntity( PolarisEntityType.ROOT, PolarisEntityConstants.getRootContainerName()); - if (resolvedCacheEntry == null) { + if (resolvedEntity == null) { LOGGER.debug("Failed to find rootContainer, so using simulated rootContainer instead."); return simulatedResolvedRootContainerEntity; } - return new ResolvedPolarisEntity(resolvedCacheEntry); + return resolvedEntity; } public PolarisResolvedPathWrapper getResolvedRootContainerEntityAsPath() { @@ -302,8 +300,8 @@ public PolarisResolvedPathWrapper getResolvedReferenceCatalogEntity( // a callsite failed to incorporate a reference catalog into its authorization flow but is // still trying to perform operations on the (nonexistence) reference catalog. diagnostics.checkNotNull(catalogName, "null_catalog_name_for_resolved_reference_catalog"); - EntityCacheEntry resolvedCachedCatalog = primaryResolver.getResolvedReferenceCatalog(); - if (resolvedCachedCatalog == null) { + ResolvedPolarisEntity resolvedReferenceCatalog = primaryResolver.getResolvedReferenceCatalog(); + if (resolvedReferenceCatalog == null) { return null; } if (prependRootContainer) { @@ -312,11 +310,9 @@ public PolarisResolvedPathWrapper getResolvedReferenceCatalogEntity( // TODO: Throw appropriate Catalog NOT_FOUND exception before any call to // getResolvedReferenceCatalogEntity(). return new PolarisResolvedPathWrapper( - List.of( - getResolvedRootContainerEntity(), new ResolvedPolarisEntity(resolvedCachedCatalog))); + List.of(getResolvedRootContainerEntity(), resolvedReferenceCatalog)); } else { - return new PolarisResolvedPathWrapper( - List.of(new ResolvedPolarisEntity(resolvedCachedCatalog))); + return new PolarisResolvedPathWrapper(List.of(resolvedReferenceCatalog)); } } @@ -328,7 +324,7 @@ public PolarisEntitySubType getLeafSubType(Object key) { key, pathLookup); int index = pathLookup.get(key); - List resolved = primaryResolver.getResolvedPaths().get(index); + List resolved = primaryResolver.getResolvedPaths().get(index); if (resolved.isEmpty()) { return PolarisEntitySubType.NULL_SUBTYPE; } @@ -357,7 +353,7 @@ public PolarisResolvedPathWrapper getResolvedPath(Object key, boolean prependRoo // Return null for a partially-resolved "optional" path. ResolverPath requestedPath = addedPaths.get(index); - List resolvedPath = primaryResolver.getResolvedPaths().get(index); + List resolvedPath = primaryResolver.getResolvedPaths().get(index); if (requestedPath.isOptional()) { if (resolvedPath.size() != requestedPath.getEntityNames().size()) { return null; @@ -368,8 +364,8 @@ public PolarisResolvedPathWrapper getResolvedPath(Object key, boolean prependRoo if (prependRootContainer) { resolvedEntities.add(getResolvedRootContainerEntity()); } - resolvedEntities.add(new ResolvedPolarisEntity(primaryResolver.getResolvedReferenceCatalog())); - resolvedPath.forEach(cacheEntry -> resolvedEntities.add(new ResolvedPolarisEntity(cacheEntry))); + resolvedEntities.add(primaryResolver.getResolvedReferenceCatalog()); + resolvedPath.forEach(resolvedEntity -> resolvedEntities.add(resolvedEntity)); return new PolarisResolvedPathWrapper(resolvedEntities); } @@ -407,15 +403,15 @@ public PolarisResolvedPathWrapper getResolvedTopLevelEntity( return null; } - EntityCacheEntry resolvedCacheEntry = primaryResolver.getResolvedEntity(entityType, entityName); - if (resolvedCacheEntry == null) { + ResolvedPolarisEntity resolvedEntity = + primaryResolver.getResolvedEntity(entityType, entityName); + if (resolvedEntity == null) { return null; } ResolvedPolarisEntity resolvedRootContainerEntity = getResolvedRootContainerEntity(); return resolvedRootContainerEntity == null - ? new PolarisResolvedPathWrapper(List.of(new ResolvedPolarisEntity(resolvedCacheEntry))) - : new PolarisResolvedPathWrapper( - List.of(resolvedRootContainerEntity, new ResolvedPolarisEntity(resolvedCacheEntry))); + ? new PolarisResolvedPathWrapper(List.of(resolvedEntity)) + : new PolarisResolvedPathWrapper(List.of(resolvedRootContainerEntity, resolvedEntity)); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java index 8141248239..2b85690ba2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java @@ -40,12 +40,12 @@ import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrivilege; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager.ChangeTrackingResult; +import org.apache.polaris.core.persistence.ResolvedPolarisEntity; import org.apache.polaris.core.persistence.cache.EntityCache; import org.apache.polaris.core.persistence.cache.EntityCacheByNameKey; -import org.apache.polaris.core.persistence.cache.EntityCacheEntry; import org.apache.polaris.core.persistence.cache.EntityCacheLookupResult; -import org.apache.polaris.core.persistence.cache.PolarisRemoteCache; -import org.apache.polaris.core.persistence.cache.PolarisRemoteCache.ChangeTrackingResult; /** * REST request resolver, allows to resolve all entities referenced directly or indirectly by in @@ -60,7 +60,7 @@ public class Resolver { private final @Nonnull PolarisDiagnostics diagnostics; // the polaris metastore manager - private final @Nonnull PolarisRemoteCache polarisRemoteCache; + private final @Nonnull PolarisMetaStoreManager polarisMetaStoreManager; // the cache of entities private final @Nonnull EntityCache cache; @@ -81,25 +81,27 @@ public class Resolver { private final List pathsToResolve; // caller principal - private EntityCacheEntry resolvedCallerPrincipal; + private ResolvedPolarisEntity resolvedCallerPrincipal; // all principal roles which have been resolved - private List resolvedCallerPrincipalRoles; + private List resolvedCallerPrincipalRoles; // catalog to use as the reference catalog for role activation - private EntityCacheEntry resolvedReferenceCatalog; + private ResolvedPolarisEntity resolvedReferenceCatalog; // all catalog roles which have been activated - private final Map resolvedCatalogRoles; + private final Map resolvedCatalogRoles; // all resolved paths - private List> resolvedPaths; + private List> resolvedPaths; - // all entities which have been successfully resolved, by name - private final Map resolvedEntriesByName; + // all entities which have been successfully resolved, by name. The entries may or may not + // have come from a cache, but we use the EntityCacheByNameKey anyways as a convenient + // canonical by-name key. + private final Map resolvedEntriesByName; // all entities which have been fully resolved, by id - private final Map resolvedEntriesById; + private final Map resolvedEntriesById; private ResolverStatus resolverStatus; @@ -107,7 +109,7 @@ public class Resolver { * Constructor, effectively starts an entity resolver session * * @param polarisCallContext the polaris call context - * @param polarisRemoteCache meta store manager + * @param polarisMetaStoreManager meta store manager * @param securityContext The {@link AuthenticatedPolarisPrincipal} for the current request * @param cache shared entity cache * @param referenceCatalogName if not null, specifies the name of the reference catalog. The @@ -120,20 +122,21 @@ public class Resolver { */ public Resolver( @Nonnull PolarisCallContext polarisCallContext, - @Nonnull PolarisRemoteCache polarisRemoteCache, + @Nonnull PolarisMetaStoreManager polarisMetaStoreManager, @Nonnull SecurityContext securityContext, @Nonnull EntityCache cache, @Nullable String referenceCatalogName) { this.polarisCallContext = polarisCallContext; this.diagnostics = polarisCallContext.getDiagServices(); - this.polarisRemoteCache = polarisRemoteCache; + this.polarisMetaStoreManager = polarisMetaStoreManager; this.cache = cache; this.securityContext = securityContext; this.referenceCatalogName = referenceCatalogName; // validate inputs this.diagnostics.checkNotNull(polarisCallContext, "unexpected_null_polarisCallContext"); - this.diagnostics.checkNotNull(polarisRemoteCache, "unexpected_null_polarisRemoteCache"); + this.diagnostics.checkNotNull( + polarisMetaStoreManager, "unexpected_null_polarisMetaStoreManager"); this.diagnostics.checkNotNull(cache, "unexpected_null_cache"); this.diagnostics.checkNotNull(securityContext, "security_context_must_be_specified"); this.diagnostics.checkNotNull( @@ -264,7 +267,7 @@ public ResolverStatus resolveAll() { /** * @return the principal we resolved */ - public @Nonnull EntityCacheEntry getResolvedCallerPrincipal() { + public @Nonnull ResolvedPolarisEntity getResolvedCallerPrincipal() { // can only be called if the resolver has been called and was success this.diagnostics.checkNotNull(resolverStatus, "resolver_must_be_called_first"); this.diagnostics.check( @@ -277,7 +280,7 @@ public ResolverStatus resolveAll() { /** * @return all principal roles which were activated. The list can be empty */ - public @Nonnull List getResolvedCallerPrincipalRoles() { + public @Nonnull List getResolvedCallerPrincipalRoles() { // can only be called if the resolver has been called and was success this.diagnostics.checkNotNull(resolverStatus, "resolver_must_be_called_first"); this.diagnostics.check( @@ -291,7 +294,7 @@ public ResolverStatus resolveAll() { * @return the reference catalog which has been resolved. Will be null if null was passed in for * the parameter referenceCatalogName when the Resolver was constructed. */ - public @Nullable EntityCacheEntry getResolvedReferenceCatalog() { + public @Nullable ResolvedPolarisEntity getResolvedReferenceCatalog() { // can only be called if the resolver has been called and was success this.diagnostics.checkNotNull(resolverStatus, "resolver_must_be_called_first"); this.diagnostics.check( @@ -307,7 +310,7 @@ public ResolverStatus resolveAll() { * * @return map of activated catalog roles or null if no referenceCatalogName was specified */ - public @Nullable Map getResolvedCatalogRoles() { + public @Nullable Map getResolvedCatalogRoles() { // can only be called if the resolver has been called and was success this.diagnostics.checkNotNull(resolverStatus, "resolver_must_be_called_first"); this.diagnostics.check( @@ -324,7 +327,7 @@ public ResolverStatus resolveAll() { * * @return single resolved path */ - public @Nonnull List getResolvedPath() { + public @Nonnull List getResolvedPath() { // can only be called if the resolver has been called and was success this.diagnostics.checkNotNull(resolverStatus, "resolver_must_be_called_first"); this.diagnostics.check( @@ -340,7 +343,7 @@ public ResolverStatus resolveAll() { * * @return list of resolved path */ - public @Nonnull List> getResolvedPaths() { + public @Nonnull List> getResolvedPaths() { // can only be called if the resolver has been called and was success this.diagnostics.checkNotNull(resolverStatus, "resolver_must_be_called_first"); this.diagnostics.check( @@ -360,7 +363,7 @@ public ResolverStatus resolveAll() { * @param entityName name of the entity. * @return the entity which has been resolved or null if that entity does not exist */ - public @Nullable EntityCacheEntry getResolvedEntity( + public @Nullable ResolvedPolarisEntity getResolvedEntity( @Nonnull PolarisEntityType entityType, @Nonnull String entityName) { // can only be called if the resolver has been called and was success this.diagnostics.checkNotNull(resolverStatus, "resolver_must_be_called_first"); @@ -400,8 +403,9 @@ private ResolverStatus runResolvePass() { this.resolvedCallerPrincipalRoles.clear(); this.resolvedPaths.clear(); - // all entries we found in the cache but that we need to validate since they might be stale - List toValidate = new ArrayList<>(); + // all entries we found in the cache or resolved hierarchically but that we need to validate + // since they might be stale + List toValidate = new ArrayList<>(); // first resolve the principal and determine the set of activated principal roles ResolverStatus status = this.resolveCallerPrincipalAndPrincipalRoles(toValidate); @@ -428,9 +432,10 @@ private ResolverStatus runResolvePass() { } // all the above resolution was optimistic i.e. when we probe the cache and find an entity, we - // don't validate if this entity has been changed in the backend. So validate now all these - // entities in one single - // go, + // don't validate if this entity has been changed in the backend. Also, hierarchical entities + // were resolved incrementally and may have changed in ways that impact the behavior of + // resolved child entities. So validate now all these entities in one single go, which ensures + // happens-before semantics. boolean validationSuccess = this.bulkValidate(toValidate); if (validationSuccess) { @@ -448,36 +453,37 @@ private void updateResolved() { // if success, we need to get the validated entries // we will resolve those again - this.resolvedCallerPrincipal = this.getResolved(this.resolvedCallerPrincipal); + this.resolvedCallerPrincipal = this.getFreshlyResolved(this.resolvedCallerPrincipal); // update all principal roles with latest if (!this.resolvedCallerPrincipalRoles.isEmpty()) { - List refreshedResolvedCallerPrincipalRoles = + List refreshedResolvedCallerPrincipalRoles = new ArrayList<>(this.resolvedCallerPrincipalRoles.size()); this.resolvedCallerPrincipalRoles.forEach( - ce -> refreshedResolvedCallerPrincipalRoles.add(this.getResolved(ce))); + ce -> refreshedResolvedCallerPrincipalRoles.add(this.getFreshlyResolved(ce))); this.resolvedCallerPrincipalRoles = refreshedResolvedCallerPrincipalRoles; } // update referenced catalog - this.resolvedReferenceCatalog = this.getResolved(this.resolvedReferenceCatalog); + this.resolvedReferenceCatalog = this.getFreshlyResolved(this.resolvedReferenceCatalog); // update all resolved catalog roles if (this.resolvedCatalogRoles != null) { - for (EntityCacheEntry catalogCacheEntry : this.resolvedCatalogRoles.values()) { + for (ResolvedPolarisEntity catalogResolvedEntity : this.resolvedCatalogRoles.values()) { this.resolvedCatalogRoles.put( - catalogCacheEntry.getEntity().getId(), this.getResolved(catalogCacheEntry)); + catalogResolvedEntity.getEntity().getId(), + this.getFreshlyResolved(catalogResolvedEntity)); } } // update all resolved paths if (!this.resolvedPaths.isEmpty()) { - List> refreshedResolvedPaths = + List> refreshedResolvedPaths = new ArrayList<>(this.resolvedPaths.size()); this.resolvedPaths.forEach( rp -> { - List refreshedRp = new ArrayList<>(rp.size()); - rp.forEach(ce -> refreshedRp.add(this.getResolved(ce))); + List refreshedRp = new ArrayList<>(rp.size()); + rp.forEach(ce -> refreshedRp.add(this.getFreshlyResolved(ce))); refreshedResolvedPaths.add(refreshedRp); }); this.resolvedPaths = refreshedResolvedPaths; @@ -485,76 +491,80 @@ private void updateResolved() { } /** - * Get the fully resolved cache entry for the specified cache entry + * Exchange a possibly-stale entity for the latest resolved version of that entity * - * @param cacheEntry input cache entry - * @return the fully resolved cached entry which will often be the same + * @param originalEntity original resolved entity for which to get the latest resolved version + * @return the fully resolved entry which will often be the same */ - private EntityCacheEntry getResolved(EntityCacheEntry cacheEntry) { - final EntityCacheEntry refreshedEntry; - if (cacheEntry == null) { + private ResolvedPolarisEntity getFreshlyResolved(ResolvedPolarisEntity originalEntity) { + final ResolvedPolarisEntity refreshedEntry; + if (originalEntity == null) { refreshedEntry = null; } else { // the latest refreshed entry - refreshedEntry = this.resolvedEntriesById.get(cacheEntry.getEntity().getId()); + refreshedEntry = this.resolvedEntriesById.get(originalEntity.getEntity().getId()); this.diagnostics.checkNotNull( - refreshedEntry, "cache_entry_should_be_resolved", "entity={}", cacheEntry.getEntity()); + refreshedEntry, "_entry_should_be_resolved", "entity={}", originalEntity.getEntity()); } return refreshedEntry; } /** * Bulk validate now the set of entities we didn't validate when we were accessing the entity - * cache + * cache or incrementally resolving * * @param toValidate entities to validate - * @return true if none of the entities in the cache has changed + * @return true if none of the entities has changed */ - private boolean bulkValidate(List toValidate) { + private boolean bulkValidate(List toValidate) { // assume everything is good boolean validationStatus = true; // bulk validate if (!toValidate.isEmpty()) { + // TODO: Provide configurable option to enforce bulk validation of *all* entities in a + // resolution pass, instead of only validating ones on "cache hit"; this would allow the same + // semantics as the transactional validation performed for methods like readEntityByName + // when PolarisMetaStoreManagerImpl uses PolarisEntityResolver in a read transaction. List entityIds = toValidate.stream() .map( - cacheEntry -> + resolvedEntity -> new PolarisEntityId( - cacheEntry.getEntity().getCatalogId(), cacheEntry.getEntity().getId())) + resolvedEntity.getEntity().getCatalogId(), + resolvedEntity.getEntity().getId())) .collect(Collectors.toList()); // now get the current backend versions of all these entities ChangeTrackingResult changeTrackingResult = - this.polarisRemoteCache.loadEntitiesChangeTracking(this.polarisCallContext, entityIds); + this.polarisMetaStoreManager.loadEntitiesChangeTracking( + this.polarisCallContext, entityIds); // refresh any entity which is not fresh. If an entity is missing, reload it - Iterator entityIterator = toValidate.iterator(); + Iterator entityIterator = toValidate.iterator(); Iterator versionIterator = changeTrackingResult.getChangeTrackingVersions().iterator(); // determine the ones we need to reload or refresh and the ones which are up-to-date while (entityIterator.hasNext()) { - // get cache entry and associated versions - EntityCacheEntry cacheEntry = entityIterator.next(); + // get resolved entity and associated versions + ResolvedPolarisEntity resolvedEntity = entityIterator.next(); PolarisChangeTrackingVersions versions = versionIterator.next(); + PolarisBaseEntity entity = resolvedEntity.getEntity(); - // entity we found in the cache - PolarisBaseEntity entity = cacheEntry.getEntity(); - - // refresh cache entry if the entity or grant records version is different - final EntityCacheEntry refreshedCacheEntry; + // refresh the resolved entity if the entity or grant records version is different + final ResolvedPolarisEntity refreshedResolvedEntity; if (versions == null || entity.getEntityVersion() != versions.getEntityVersion() || entity.getGrantRecordsVersion() != versions.getGrantRecordsVersion()) { // if null version we need to invalidate the cached entry since it has probably been // dropped if (versions == null) { - this.cache.removeCacheEntry(cacheEntry); - refreshedCacheEntry = null; + this.cache.removeCacheEntry(resolvedEntity); + refreshedResolvedEntity = null; } else { // refresh that entity. If versions is null, it has been dropped - refreshedCacheEntry = + refreshedResolvedEntity = this.cache.getAndRefreshIfNeeded( this.polarisCallContext, entity, @@ -564,7 +574,7 @@ private boolean bulkValidate(List toValidate) { // get the refreshed entity PolarisBaseEntity refreshedEntity = - (refreshedCacheEntry == null) ? null : refreshedCacheEntry.getEntity(); + (refreshedResolvedEntity == null) ? null : refreshedResolvedEntity.getEntity(); // if the entity has been removed, or its name has changed, or it was re-parented, or it // was dropped, we will have to perform another pass @@ -584,13 +594,13 @@ private boolean bulkValidate(List toValidate) { } } else { // no need to refresh, it is up-to-date - refreshedCacheEntry = cacheEntry; + refreshedResolvedEntity = resolvedEntity; } // if it was found, it has been resolved, so if there is another pass, we will not have to // resolve it again - if (refreshedCacheEntry != null) { - this.addToResolved(refreshedCacheEntry); + if (refreshedResolvedEntity != null) { + this.addToResolved(refreshedResolvedEntity); } } } @@ -602,17 +612,18 @@ private boolean bulkValidate(List toValidate) { /** * Resolve a set of top-level service or catalog entities * - * @param toValidate all entities we have resolved from the cache, hence we will have to verify - * that these entities have not changed in the backend + * @param toValidate all entities we have resolved incrementally, possibly with some entries + * coming from cache, hence we will have to verify that these entities have not changed in the + * backend * @param entitiesToResolve the set of entities to resolve * @return the status of resolution */ private ResolverStatus resolveEntities( - List toValidate, AbstractSet entitiesToResolve) { + List toValidate, AbstractSet entitiesToResolve) { // resolve each for (ResolverEntityName entityName : entitiesToResolve) { // resolve that entity - EntityCacheEntry resolvedEntity = + ResolvedPolarisEntity resolvedEntity = this.resolveByName(toValidate, entityName.getEntityType(), entityName.getEntityName()); // if not found, we can exit unless the entity is optional @@ -629,13 +640,14 @@ private ResolverStatus resolveEntities( /** * Resolve a set of path inside the referenced catalog * - * @param toValidate all entities we have resolved from the cache, hence we will have to verify - * that these entities have not changed in the backend + * @param toValidate all entities we have resolved incrementally, possibly with some entries + * coming from cache, hence we will have to verify that these entities have not changed in the + * backend * @param pathsToResolve the set of paths to resolve * @return the status of resolution */ private ResolverStatus resolvePaths( - List toValidate, List pathsToResolve) { + List toValidate, List pathsToResolve) { // id of the catalog for all these paths final long catalogId = this.resolvedReferenceCatalog.getEntity().getId(); @@ -644,7 +656,7 @@ private ResolverStatus resolvePaths( for (ResolverPath path : pathsToResolve) { // path we are resolving - List resolvedPath = new ArrayList<>(); + List resolvedPath = new ArrayList<>(); // initial parent id is the catalog itself long parentId = catalogId; @@ -660,7 +672,7 @@ private ResolverStatus resolvePaths( pathIt.hasNext() ? PolarisEntityType.NAMESPACE : path.getLastEntityType(); // resolve that entity - EntityCacheEntry segment = + ResolvedPolarisEntity segment = this.resolveByName(toValidate, catalogId, segmentType, parentId, segmentName); // if not found, abort @@ -691,12 +703,13 @@ private ResolverStatus resolvePaths( /** * Resolve the principal and determine which principal roles are activated. Resolved those. * - * @param toValidate all entities we have resolved from the cache, hence we will have to verify - * that these entities have not changed in the backend + * @param toValidate all entities we have resolved incrementally, possibly with some entries + * coming from cache, hence we will have to verify that these entities have not changed in the + * backend * @return the status of resolution */ private ResolverStatus resolveCallerPrincipalAndPrincipalRoles( - List toValidate) { + List toValidate) { // resolve the principal, by name or id this.resolvedCallerPrincipal = @@ -730,8 +743,8 @@ private ResolverStatus resolveCallerPrincipalAndPrincipalRoles( * @param resolvedCallerPrincipal1 * @return the list of resolved principal roles the principal has grants for */ - private List resolveAllPrincipalRoles( - List toValidate, EntityCacheEntry resolvedCallerPrincipal1) { + private List resolveAllPrincipalRoles( + List toValidate, ResolvedPolarisEntity resolvedCallerPrincipal1) { return resolvedCallerPrincipal1.getGrantRecordsAsGrantee().stream() .filter(gr -> gr.getPrivilegeCode() == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode()) .map( @@ -752,8 +765,8 @@ private List resolveAllPrincipalRoles( * @param roleNames * @return the filtered list of resolved principal roles */ - private List resolvePrincipalRolesByName( - List toValidate, Set roleNames) { + private List resolvePrincipalRolesByName( + List toValidate, Set roleNames) { return roleNames.stream() .filter(securityContext::isUserInRole) .map(roleName -> resolveByName(toValidate, PolarisEntityType.PRINCIPAL_ROLE, roleName)) @@ -764,14 +777,15 @@ private List resolvePrincipalRolesByName( * Resolve the reference catalog and determine all activated role. The principal and principal * roles should have already been resolved * - * @param toValidate all entities we have resolved from the cache, hence we will have to verify - * that these entities have not changed in the backend + * @param toValidate all entities we have resolved incrementally, possibly with some entries + * coming from cache, hence we will have to verify that these entities have not changed in the + * backend * @param referenceCatalogName name of the reference catalog to resolve, along with all catalog * roles which are activated * @return the status of resolution */ private ResolverStatus resolveReferenceCatalog( - @Nonnull List toValidate, @Nonnull String referenceCatalogName) { + @Nonnull List toValidate, @Nonnull String referenceCatalogName) { // resolve the catalog this.resolvedReferenceCatalog = this.resolveByName(toValidate, PolarisEntityType.CATALOG, referenceCatalogName); @@ -784,7 +798,7 @@ private ResolverStatus resolveReferenceCatalog( // determine the set of catalog roles which have been activated long catalogId = this.resolvedReferenceCatalog.getEntity().getId(); - for (EntityCacheEntry principalRole : resolvedCallerPrincipalRoles) { + for (ResolvedPolarisEntity principalRole : resolvedCallerPrincipalRoles) { for (PolarisGrantRecord grantRecord : principalRole.getGrantRecordsAsGrantee()) { // the securable is a catalog role belonging to if (grantRecord.getPrivilegeCode() == PolarisPrivilege.CATALOG_ROLE_USAGE.getCode() @@ -795,7 +809,7 @@ private ResolverStatus resolveReferenceCatalog( // skip if it has already been added if (!this.resolvedCatalogRoles.containsKey(catalogRoleId)) { // see if this catalog can be resolved - EntityCacheEntry catalogRole = + ResolvedPolarisEntity catalogRole = this.resolveById( toValidate, PolarisEntityType.CATALOG_ROLE, catalogId, catalogRoleId); @@ -813,23 +827,23 @@ private ResolverStatus resolveReferenceCatalog( } /** - * Add a cache entry to the set of resolved entities + * Add a resolved entity to the current resolution collection's set of resolved entities * - * @param refreshedCacheEntry refreshed cache entry + * @param refreshedResolvedEntity refreshed resolved entity */ - private void addToResolved(EntityCacheEntry refreshedCacheEntry) { + private void addToResolved(ResolvedPolarisEntity refreshedResolvedEntity) { // underlying entity - PolarisBaseEntity entity = refreshedCacheEntry.getEntity(); + PolarisBaseEntity entity = refreshedResolvedEntity.getEntity(); // add it by ID - this.resolvedEntriesById.put(entity.getId(), refreshedCacheEntry); + this.resolvedEntriesById.put(entity.getId(), refreshedResolvedEntity); // in the by name map, only add it if it has not been dropped if (!entity.isDropped()) { this.resolvedEntriesByName.put( new EntityCacheByNameKey( entity.getCatalogId(), entity.getParentId(), entity.getType(), entity.getName()), - refreshedCacheEntry); + refreshedResolvedEntity); } } @@ -869,10 +883,10 @@ private void addEntityByName( * @param toValidate set of entries we will have to validate * @param entityType entity type * @param entityName name of the entity to resolve - * @return cache entry created for that entity + * @return resolved entity */ - private EntityCacheEntry resolveByName( - List toValidate, PolarisEntityType entityType, String entityName) { + private ResolvedPolarisEntity resolveByName( + List toValidate, PolarisEntityType entityType, String entityName) { if (entityType.isTopLevel()) { return this.resolveByName( toValidate, @@ -897,8 +911,8 @@ private EntityCacheEntry resolveByName( * @return the resolve entity. Potentially update the toValidate list if we will have to validate * that this entity is up-to-date */ - private EntityCacheEntry resolveByName( - @Nonnull List toValidate, + private ResolvedPolarisEntity resolveByName( + @Nonnull List toValidate, long catalogId, @Nonnull PolarisEntityType entityType, long parentId, @@ -909,14 +923,14 @@ private EntityCacheEntry resolveByName( new EntityCacheByNameKey(catalogId, parentId, entityType, entityName); // first check if this entity has not yet been resolved - EntityCacheEntry cacheEntry = this.resolvedEntriesByName.get(nameKey); - if (cacheEntry != null) { - return cacheEntry; + ResolvedPolarisEntity resolvedEntity = this.resolvedEntriesByName.get(nameKey); + if (resolvedEntity != null) { + return resolvedEntity; } // then check if it does not exist in the toValidate list. The same entity might be resolved // several times with multi-path resolution - for (EntityCacheEntry ce : toValidate) { + for (ResolvedPolarisEntity ce : toValidate) { PolarisBaseEntity entity = ce.getEntity(); if (entity.getCatalogId() == catalogId && entity.getParentId() == parentId @@ -960,8 +974,8 @@ private EntityCacheEntry resolveByName( * @return the resolve entity. Potentially update the toValidate list if we will have to validate * that this entity is up-to-date */ - private EntityCacheEntry resolveById( - @Nonnull List toValidate, + private ResolvedPolarisEntity resolveById( + @Nonnull List toValidate, @Nonnull PolarisEntityType entityType, long catalogId, long entityId) { diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/EntityCacheTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/EntityCacheTest.java index c47367769e..8dc3c84aba 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/EntityCacheTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/EntityCacheTest.java @@ -32,7 +32,6 @@ import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.core.persistence.cache.EntityCache; import org.apache.polaris.core.persistence.cache.EntityCacheByNameKey; -import org.apache.polaris.core.persistence.cache.EntityCacheEntry; import org.apache.polaris.core.persistence.cache.EntityCacheLookupResult; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @@ -141,7 +140,7 @@ void testGetOrLoadEntityByName() { EntityCacheByNameKey N1_name = new EntityCacheByNameKey( catalog.getId(), catalog.getId(), PolarisEntityType.NAMESPACE, "N1"); - EntityCacheEntry cacheEntry = cache.getEntityByName(N1_name); + ResolvedPolarisEntity cacheEntry = cache.getEntityByName(N1_name); Assertions.assertThat(cacheEntry).isNull(); // try to find it in the cache by id. Should not be there, i.e. no cache hit @@ -162,7 +161,7 @@ void testGetOrLoadEntityByName() { Assertions.assertThat(cacheEntry.getGrantRecordsAsSecurable()).isNotNull(); // lookup N1 - EntityCacheEntry N1_entry = cache.getEntityById(N1.getId()); + ResolvedPolarisEntity N1_entry = cache.getEntityById(N1.getId()); Assertions.assertThat(N1_entry).isNotNull(); Assertions.assertThat(N1_entry.getEntity()).isNotNull(); Assertions.assertThat(N1_entry.getGrantRecordsAsSecurable()).isNotNull(); @@ -181,7 +180,7 @@ void testGetOrLoadEntityByName() { new EntityCacheByNameKey(catalog.getId(), N1.getId(), PolarisEntityType.NAMESPACE, "N2"); lookup = cache.getOrLoadEntityByName(callCtx, N2_name); Assertions.assertThat(lookup).isNotNull(); - EntityCacheEntry cacheEntry_N1 = lookup.getCacheEntry(); + ResolvedPolarisEntity cacheEntry_N1 = lookup.getCacheEntry(); Assertions.assertThat(cacheEntry_N1).isNotNull(); Assertions.assertThat(cacheEntry_N1.getEntity()).isNotNull(); Assertions.assertThat(cacheEntry_N1.getGrantRecordsAsSecurable()).isNotNull(); @@ -192,7 +191,7 @@ void testGetOrLoadEntityByName() { catalog.getId(), catalog.getId(), PolarisEntityType.CATALOG_ROLE, "R1"); lookup = cache.getOrLoadEntityByName(callCtx, R1_name); Assertions.assertThat(lookup).isNotNull(); - EntityCacheEntry cacheEntry_R1 = lookup.getCacheEntry(); + ResolvedPolarisEntity cacheEntry_R1 = lookup.getCacheEntry(); Assertions.assertThat(cacheEntry_R1).isNotNull(); Assertions.assertThat(cacheEntry_R1.getEntity()).isNotNull(); Assertions.assertThat(cacheEntry_R1.getGrantRecordsAsSecurable()).isNotNull(); @@ -236,7 +235,7 @@ void testGetOrLoadEntityByName() { new EntityCacheByNameKey(PolarisEntityType.PRINCIPAL_ROLE, "PR1"); lookup = cache.getOrLoadEntityByName(callCtx, PR1_name); Assertions.assertThat(lookup).isNotNull(); - EntityCacheEntry cacheEntry_PR1 = lookup.getCacheEntry(); + ResolvedPolarisEntity cacheEntry_PR1 = lookup.getCacheEntry(); Assertions.assertThat(cacheEntry_PR1).isNotNull(); Assertions.assertThat(cacheEntry_PR1.getEntity()).isNotNull(); Assertions.assertThat(cacheEntry_PR1.getGrantRecordsAsSecurable()).isNotNull(); @@ -299,7 +298,7 @@ void testRefresh() { Assertions.assertThat(T6v1).isNotNull(); // that table is not in the cache - EntityCacheEntry cacheEntry = cache.getEntityById(T6v1.getId()); + ResolvedPolarisEntity cacheEntry = cache.getEntityById(T6v1.getId()); Assertions.assertThat(cacheEntry).isNull(); // now load that table in the cache @@ -433,7 +432,7 @@ void testRenameAndCacheDestinationBeforeLoadingSource() { new EntityCacheByNameKey(N1.getCatalogId(), N1.getId(), PolarisEntityType.TABLE_LIKE, "T4"); lookup = cache.getOrLoadEntityByName(callCtx, T4_name); Assertions.assertThat(lookup).isNotNull(); - EntityCacheEntry cacheEntry_T4 = lookup.getCacheEntry(); + ResolvedPolarisEntity cacheEntry_T4 = lookup.getCacheEntry(); Assertions.assertThat(cacheEntry_T4).isNotNull(); Assertions.assertThat(cacheEntry_T4.getEntity()).isNotNull(); Assertions.assertThat(cacheEntry_T4.getGrantRecordsAsSecurable()).isNotNull(); @@ -448,7 +447,7 @@ void testRenameAndCacheDestinationBeforeLoadingSource() { N1.getCatalogId(), N1.getId(), PolarisEntityType.TABLE_LIKE, "T4_renamed"); lookup = cache.getOrLoadEntityByName(callCtx, T4_renamed); Assertions.assertThat(lookup).isNotNull(); - EntityCacheEntry cacheEntry_T4_renamed = lookup.getCacheEntry(); + ResolvedPolarisEntity cacheEntry_T4_renamed = lookup.getCacheEntry(); Assertions.assertThat(cacheEntry_T4_renamed).isNotNull(); PolarisBaseEntity T4_renamed_entity = cacheEntry_T4_renamed.getEntity(); diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java index 9bc6cabfee..5376a9ab77 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java @@ -44,9 +44,8 @@ import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.entity.PrincipalRoleEntity; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager.ResolvedEntityResult; import org.apache.polaris.core.persistence.cache.EntityCache; -import org.apache.polaris.core.persistence.cache.EntityCacheEntry; -import org.apache.polaris.core.persistence.cache.PolarisRemoteCache.CachedEntryResult; import org.apache.polaris.core.persistence.resolver.Resolver; import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; @@ -245,7 +244,7 @@ void testResolvePath() { Resolver resolver = this.resolveDriver(this.cache, "test", null, List.of(N1, N5_N6_T8, N5_N6_T5, N1_N2), null); // get all the resolved paths - List> resolvedPath = resolver.getResolvedPaths(); + List> resolvedPath = resolver.getResolvedPaths(); Assertions.assertThat(resolvedPath.get(0)).hasSize(1); Assertions.assertThat(resolvedPath.get(1)).hasSize(2); Assertions.assertThat(resolvedPath.get(2)).hasSize(3); @@ -541,7 +540,7 @@ private void resolvePrincipalAndPrincipalRole( this.ensureResolved(resolver.getResolvedCallerPrincipal(), PolarisEntityType.PRINCIPAL, "P1"); // validate that the two principal roles have been activated - List principalRolesResolved = resolver.getResolvedCallerPrincipalRoles(); + List principalRolesResolved = resolver.getResolvedCallerPrincipalRoles(); // expect two principal roles Assertions.assertThat(principalRolesResolved).hasSize(2); @@ -774,7 +773,8 @@ private Resolver resolveDriver( } // validate that the correct set if principal roles have been activated - List principalRolesResolved = resolver.getResolvedCallerPrincipalRoles(); + List principalRolesResolved = + resolver.getResolvedCallerPrincipalRoles(); principalRolesResolved.sort(Comparator.comparing(p -> p.getEntity().getName())); // expect two principal roles if not scoped @@ -795,7 +795,7 @@ private Resolver resolveDriver( Assertions.assertThat(principalRolesResolved).hasSize(expectedSize); // expect either PR1 and PR2 - for (EntityCacheEntry principalRoleResolved : principalRolesResolved) { + for (ResolvedPolarisEntity principalRoleResolved : principalRolesResolved) { Assertions.assertThat(principalRoleResolved).isNotNull(); Assertions.assertThat(principalRoleResolved.getEntity()).isNotNull(); String roleName = principalRoleResolved.getEntity().getName(); @@ -817,14 +817,14 @@ private Resolver resolveDriver( // if a catalog was passed-in, ensure it exists if (catalogName != null) { - EntityCacheEntry catalogEntry = + ResolvedPolarisEntity catalogEntry = resolver.getResolvedEntity(PolarisEntityType.CATALOG, catalogName); Assertions.assertThat(catalogEntry).isNotNull(); this.ensureResolved(catalogEntry, PolarisEntityType.CATALOG, catalogName); // if a catalog role was passed-in, ensure that it was properly resolved if (catalogRoleName != null) { - EntityCacheEntry catalogRoleEntry = + ResolvedPolarisEntity catalogRoleEntry = resolver.getResolvedEntity(PolarisEntityType.CATALOG_ROLE, catalogRoleName); this.ensureResolved( catalogRoleEntry, @@ -834,7 +834,7 @@ private Resolver resolveDriver( } // validate activated catalog roles - Map activatedCatalogs = resolver.getResolvedCatalogRoles(); + Map activatedCatalogs = resolver.getResolvedCatalogRoles(); // if there is an expected set, ensure we have the same set if (expectedActivatedCatalogRoles != null) { @@ -842,7 +842,7 @@ private Resolver resolveDriver( } // process each of those - for (EntityCacheEntry resolvedActivatedCatalogEntry : activatedCatalogs.values()) { + for (ResolvedPolarisEntity resolvedActivatedCatalogEntry : activatedCatalogs.values()) { // must be in the expected list Assertions.assertThat(resolvedActivatedCatalogEntry).isNotNull(); PolarisBaseEntity activatedCatalogRole = resolvedActivatedCatalogEntry.getEntity(); @@ -867,7 +867,7 @@ private Resolver resolveDriver( List allPathsToCheck = (paths == null) ? List.of(path) : paths; // all resolved path - List> allResolvedPaths = resolver.getResolvedPaths(); + List> allResolvedPaths = resolver.getResolvedPaths(); // same size Assertions.assertThat(allResolvedPaths).hasSameSizeAs(allPathsToCheck); @@ -875,7 +875,7 @@ private Resolver resolveDriver( // check that each path was properly resolved int pathCount = 0; Iterator allPathsToCheckIt = allPathsToCheck.iterator(); - for (List resolvedPath : allResolvedPaths) { + for (List resolvedPath : allResolvedPaths) { this.ensurePathResolved( pathCount++, catalogEntry.getEntity(), allPathsToCheckIt.next(), resolvedPath); } @@ -897,7 +897,7 @@ private void ensurePathResolved( int pathCount, PolarisBaseEntity catalog, ResolverPath pathToResolve, - List resolvedPath) { + List resolvedPath) { // ensure same cardinality if (!pathToResolve.isOptional()) { @@ -910,7 +910,7 @@ private void ensurePathResolved( // loop and validate each element for (int index = 0; index < resolvedPath.size(); index++) { - EntityCacheEntry cacheEntry = resolvedPath.get(index); + ResolvedPolarisEntity cacheEntry = resolvedPath.get(index); String entityName = pathToResolve.getEntityNames().get(index); PolarisEntityType entityType = (index == pathToResolve.getEntityNames().size() - 1) @@ -934,7 +934,7 @@ private void ensurePathResolved( * @param entityName entity name */ private void ensureResolved( - EntityCacheEntry cacheEntry, + ResolvedPolarisEntity cacheEntry, List catalogPath, PolarisEntityType entityType, String entityName) { @@ -952,16 +952,16 @@ private void ensureResolved( Assertions.assertThat(refEntity).isNotNull(); // reload the cached entry from the backend - CachedEntryResult refCachedEntry = - this.metaStoreManager.loadCachedEntryById( + ResolvedEntityResult refResolvedEntity = + this.metaStoreManager.loadResolvedEntityById( this.callCtx, refEntity.getCatalogId(), refEntity.getId()); // should exist - Assertions.assertThat(refCachedEntry).isNotNull(); + Assertions.assertThat(refResolvedEntity).isNotNull(); // ensure same entity - refEntity = refCachedEntry.getEntity(); - List refGrantRecords = refCachedEntry.getEntityGrantRecords(); + refEntity = refResolvedEntity.getEntity(); + List refGrantRecords = refResolvedEntity.getEntityGrantRecords(); Assertions.assertThat(refEntity).isNotNull(); Assertions.assertThat(refGrantRecords).isNotNull(); Assertions.assertThat(entity).isEqualTo(refEntity); @@ -989,7 +989,7 @@ private void ensureResolved( * @param entityName entity name */ private void ensureResolved( - EntityCacheEntry cacheEntry, PolarisEntityType entityType, String entityName) { + ResolvedPolarisEntity cacheEntry, PolarisEntityType entityType, String entityName) { this.ensureResolved(cacheEntry, null, entityType, entityName); } } diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 11ee4a87f1..0c7c4131da 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -43,7 +43,7 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.core.entity.PolarisTaskConstants; -import org.apache.polaris.core.persistence.cache.PolarisRemoteCache.CachedEntryResult; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager.ResolvedEntityResult; import org.assertj.core.api.Assertions; /** Test the Polaris persistence layer */ @@ -1226,7 +1226,7 @@ private void validateListReturn( * * @param cacheEntry the cached entity to validate */ - private void validateCacheEntryLoad(CachedEntryResult cacheEntry) { + private void validateCacheEntryLoad(ResolvedEntityResult cacheEntry) { // cannot be null Assertions.assertThat(cacheEntry).isNotNull(); @@ -1287,7 +1287,7 @@ private void validateCacheEntryLoad(CachedEntryResult cacheEntry) { * @param cacheEntry the cached entity to validate */ private void validateCacheEntryRefresh( - CachedEntryResult cacheEntry, + ResolvedEntityResult cacheEntry, long catalogId, long entityId, int entityVersion, @@ -1362,8 +1362,8 @@ private PolarisBaseEntity loadCacheEntryByName( @Nonnull String entityName, boolean expectExists) { // load cached entry - CachedEntryResult cacheEntry = - this.polarisMetaStoreManager.loadCachedEntryByName( + ResolvedEntityResult cacheEntry = + this.polarisMetaStoreManager.loadResolvedEntityByName( this.polarisCallContext, entityCatalogId, parentId, entityType, entityName); // if null, validate that indeed the entry does not exist @@ -1408,8 +1408,8 @@ private PolarisBaseEntity loadCacheEntryByName( private PolarisBaseEntity loadCacheEntryById( long entityCatalogId, long entityId, boolean expectExists) { // load cached entry - CachedEntryResult cacheEntry = - this.polarisMetaStoreManager.loadCachedEntryById( + ResolvedEntityResult cacheEntry = + this.polarisMetaStoreManager.loadResolvedEntityById( this.polarisCallContext, entityCatalogId, entityId); // if null, validate that indeed the entry does not exist @@ -1455,8 +1455,8 @@ private void refreshCacheEntry( long entityId, boolean expectExists) { // load cached entry - CachedEntryResult cacheEntry = - this.polarisMetaStoreManager.refreshCachedEntity( + ResolvedEntityResult cacheEntry = + this.polarisMetaStoreManager.refreshResolvedEntity( this.polarisCallContext, entityVersion, entityGrantRecordsVersion, @@ -2087,8 +2087,8 @@ void renameEntity( // the renamed entity PolarisEntity renamedEntityInput = new PolarisEntity(entity); renamedEntityInput.setName(newName); - String updatedInternalPropertiesString = "updatedDataForInternalProperties1234"; - String updatedPropertiesString = "updatedDataForProperties9876"; + String updatedInternalPropertiesString = "{\"key1\": \"updatedDataForInternalProperties1234\"}"; + String updatedPropertiesString = "{\"key1\": \"updatedDataForProperties9876\"}"; // this is to test that properties are also updated during the rename operation renamedEntityInput.setInternalProperties(updatedInternalPropertiesString); diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java b/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java index 65f6665862..5ec318693c 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java @@ -63,7 +63,7 @@ import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisMetaStoreSession; -import org.apache.polaris.core.persistence.cache.EntityCacheEntry; +import org.apache.polaris.core.persistence.ResolvedPolarisEntity; import org.apache.polaris.core.persistence.resolver.Resolver; import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; @@ -590,7 +590,7 @@ public Response getConfig( if (!resolverStatus.getStatus().equals(ResolverStatus.StatusEnum.SUCCESS)) { throw new NotFoundException("Unable to find warehouse %s", warehouse); } - EntityCacheEntry resolvedReferenceCatalog = resolver.getResolvedReferenceCatalog(); + ResolvedPolarisEntity resolvedReferenceCatalog = resolver.getResolvedReferenceCatalog(); Map properties = PolarisEntity.of(resolvedReferenceCatalog.getEntity()).getPropertiesAsMap(); From 04a636e062671522891c72386f8558d6fe9ddc36 Mon Sep 17 00:00:00 2001 From: Dennis Huo Date: Mon, 24 Feb 2025 04:19:02 +0000 Subject: [PATCH 2/2] Add support for actually giving null EntityCache to Resolver; parameterize ResolverTest --- .../core/persistence/resolver/Resolver.java | 153 +++++++++++++----- .../core/persistence/ResolverTest.java | 55 +++++-- 2 files changed, 148 insertions(+), 60 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java index 2b85690ba2..52e5d9efc3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/Resolver.java @@ -42,6 +42,7 @@ import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager.ChangeTrackingResult; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager.ResolvedEntityResult; import org.apache.polaris.core.persistence.ResolvedPolarisEntity; import org.apache.polaris.core.persistence.cache.EntityCache; import org.apache.polaris.core.persistence.cache.EntityCacheByNameKey; @@ -137,7 +138,6 @@ public Resolver( this.diagnostics.checkNotNull(polarisCallContext, "unexpected_null_polarisCallContext"); this.diagnostics.checkNotNull( polarisMetaStoreManager, "unexpected_null_polarisMetaStoreManager"); - this.diagnostics.checkNotNull(cache, "unexpected_null_cache"); this.diagnostics.checkNotNull(securityContext, "security_context_must_be_specified"); this.diagnostics.checkNotNull( securityContext.getUserPrincipal(), "principal_must_be_specified"); @@ -560,16 +560,41 @@ private boolean bulkValidate(List toValidate) { // if null version we need to invalidate the cached entry since it has probably been // dropped if (versions == null) { - this.cache.removeCacheEntry(resolvedEntity); + if (this.cache != null) { + this.cache.removeCacheEntry(resolvedEntity); + } refreshedResolvedEntity = null; } else { // refresh that entity. If versions is null, it has been dropped - refreshedResolvedEntity = - this.cache.getAndRefreshIfNeeded( - this.polarisCallContext, - entity, - versions.getEntityVersion(), - versions.getGrantRecordsVersion()); + if (this.cache != null) { + refreshedResolvedEntity = + this.cache.getAndRefreshIfNeeded( + this.polarisCallContext, + entity, + versions.getEntityVersion(), + versions.getGrantRecordsVersion()); + } else { + ResolvedEntityResult result = + this.polarisMetaStoreManager.refreshResolvedEntity( + this.polarisCallContext, + entity.getEntityVersion(), + entity.getGrantRecordsVersion(), + entity.getType(), + entity.getCatalogId(), + entity.getId()); + refreshedResolvedEntity = + result.isSuccess() + ? new ResolvedPolarisEntity( + this.polarisCallContext.getDiagServices(), + result.getEntity() != null ? result.getEntity() : entity, + result.getEntityGrantRecords() != null + ? result.getEntityGrantRecords() + : resolvedEntity.getAllGrantRecords(), + result.getEntityGrantRecords() != null + ? result.getGrantRecordsVersion() + : entity.getGrantRecordsVersion()) + : null; + } } // get the refreshed entity @@ -941,27 +966,47 @@ private ResolvedPolarisEntity resolveByName( } // get or load by name - EntityCacheLookupResult lookupResult = - this.cache.getOrLoadEntityByName( - this.polarisCallContext, - new EntityCacheByNameKey(catalogId, parentId, entityType, entityName)); - - // if not found - if (lookupResult == null) { - // not found - return null; - } else if (lookupResult.isCacheHit()) { - // found in the cache, we will have to validate this entity - toValidate.add(lookupResult.getCacheEntry()); + if (this.cache != null) { + EntityCacheLookupResult lookupResult = + this.cache.getOrLoadEntityByName( + this.polarisCallContext, + new EntityCacheByNameKey(catalogId, parentId, entityType, entityName)); + + // if not found + if (lookupResult == null) { + // not found + return null; + } else if (lookupResult.isCacheHit()) { + // found in the cache, we will have to validate this entity + toValidate.add(lookupResult.getCacheEntry()); + } else { + // entry cannot be null + this.diagnostics.checkNotNull(lookupResult.getCacheEntry(), "cache_entry_is_null"); + // if not found in cache, it was loaded from backend, hence it has been resolved + this.addToResolved(lookupResult.getCacheEntry()); + } + + // return the cache entry + return lookupResult.getCacheEntry(); } else { - // entry cannot be null - this.diagnostics.checkNotNull(lookupResult.getCacheEntry(), "cache_entry_is_null"); - // if not found in cache, it was loaded from backend, hence it has been resolved - this.addToResolved(lookupResult.getCacheEntry()); - } + // If no cache, load directly from metastore manager. + ResolvedEntityResult result = + this.polarisMetaStoreManager.loadResolvedEntityByName( + this.polarisCallContext, catalogId, parentId, entityType, entityName); + if (!result.isSuccess()) { + // not found + return null; + } - // return the cache entry - return lookupResult.getCacheEntry(); + resolvedEntity = + new ResolvedPolarisEntity( + this.polarisCallContext.getDiagServices(), + result.getEntity(), + result.getEntityGrantRecords(), + result.getGrantRecordsVersion()); + this.addToResolved(resolvedEntity); + return resolvedEntity; + } } /** @@ -979,25 +1024,45 @@ private ResolvedPolarisEntity resolveById( @Nonnull PolarisEntityType entityType, long catalogId, long entityId) { - // get or load by name - EntityCacheLookupResult lookupResult = - this.cache.getOrLoadEntityById(this.polarisCallContext, catalogId, entityId); - - // if not found, return null - if (lookupResult == null) { - return null; - } else if (lookupResult.isCacheHit()) { - // found in the cache, we will have to validate this entity - toValidate.add(lookupResult.getCacheEntry()); + if (this.cache != null) { + // get or load by name + EntityCacheLookupResult lookupResult = + this.cache.getOrLoadEntityById(this.polarisCallContext, catalogId, entityId); + + // if not found, return null + if (lookupResult == null) { + return null; + } else if (lookupResult.isCacheHit()) { + // found in the cache, we will have to validate this entity + toValidate.add(lookupResult.getCacheEntry()); + } else { + // entry cannot be null + this.diagnostics.checkNotNull(lookupResult.getCacheEntry(), "cache_entry_is_null"); + + // if not found in cache, it was loaded from backend, hence it has been resolved + this.addToResolved(lookupResult.getCacheEntry()); + } + + // return the cache entry + return lookupResult.getCacheEntry(); } else { - // entry cannot be null - this.diagnostics.checkNotNull(lookupResult.getCacheEntry(), "cache_entry_is_null"); + // If no cache, load directly from metastore manager. + ResolvedEntityResult result = + polarisMetaStoreManager.loadResolvedEntityById( + this.polarisCallContext, catalogId, entityId); + if (!result.isSuccess()) { + // not found + return null; + } - // if not found in cache, it was loaded from backend, hence it has been resolved - this.addToResolved(lookupResult.getCacheEntry()); + ResolvedPolarisEntity resolvedEntity = + new ResolvedPolarisEntity( + this.polarisCallContext.getDiagServices(), + result.getEntity(), + result.getEntityGrantRecords(), + result.getGrantRecordsVersion()); + this.addToResolved(resolvedEntity); + return resolvedEntity; } - - // return the cache entry - return lookupResult.getCacheEntry(); } } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java index 5376a9ab77..5270a90e22 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/ResolverTest.java @@ -50,7 +50,8 @@ import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mockito; public class ResolverTest { @@ -79,6 +80,12 @@ public class ResolverTest { // cache we are using private EntityCache cache; + // whenever constructing a new Resolver instance, if false, disable cache for that Resolver + // instance by giving it a null cache regardless of the current state of the test-level + // cache instance; use a boolean for this instead of just modifying the test member 'cache' + // so that we can potentially alternate between using cache and not using cache + private boolean shouldUseCache; + /** * Initialize and create the test metadata * @@ -117,8 +124,10 @@ public ResolverTest() { } /** This test resolver for a create-principal scenario */ - @Test - void testResolvePrincipal() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testResolvePrincipal(boolean useCache) { + this.shouldUseCache = useCache; // resolve a principal which does not exist, but make it optional so will succeed this.resolveDriver(null, null, "P3", true, null, null); @@ -144,8 +153,10 @@ void testResolvePrincipal() { } /** Test that we can specify a subset of principal role names */ - @Test - void testScopedPrincipalRole() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testScopedPrincipalRole(boolean useCache) { + this.shouldUseCache = useCache; // start without a scope this.resolveDriver(null, null, "P2", false, "PR1", null); @@ -163,8 +174,10 @@ void testScopedPrincipalRole() { * Test that the set of catalog roles being activated is correctly inferred, based of a set of * principal roles */ - @Test - void testCatalogRolesActivation() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testCatalogRolesActivation(boolean useCache) { + this.shouldUseCache = useCache; // start simple, with both PR1 and PR2, you get R1 and R2 this.resolveDriver(null, Set.of("PR1", "PR2"), "test", Set.of("R1", "R2")); @@ -180,8 +193,11 @@ void testCatalogRolesActivation() { } /** Test that paths, one or more, are properly resolved */ - @Test - void testResolvePath() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testResolvePath(boolean useCache) { + this.shouldUseCache = useCache; + // N1 which exists ResolverPath N1 = new ResolverPath(List.of("N1"), PolarisEntityType.NAMESPACE); this.resolveDriver(null, "test", N1, null, null); @@ -255,8 +271,10 @@ void testResolvePath() { * Ensure that if data changes while entities are cached, we will always resolve to the latest * version */ - @Test - void testConsistency() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testConsistency(boolean useCache) { + this.shouldUseCache = useCache; // resolve principal "P2" this.resolveDriver(null, null, "P2", false, null, null); @@ -315,8 +333,11 @@ void testConsistency() { } /** Check resolve paths when cache is inconsistent */ - @Test - void testPathConsistency() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testPathConsistency(boolean useCache) { + this.shouldUseCache = useCache; + // resolve few paths path ResolverPath N1_PATH = new ResolverPath(List.of("N1"), PolarisEntityType.NAMESPACE); this.resolveDriver(null, "test", N1_PATH, null, null); @@ -362,8 +383,10 @@ void testPathConsistency() { } /** Resolve catalog roles */ - @Test - void testResolveCatalogRole() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testResolveCatalogRole(boolean useCache) { + this.shouldUseCache = useCache; // resolve catalog role this.resolveDriver(null, "test", "R1", null); @@ -492,7 +515,7 @@ public String getAuthenticationScheme() { return ""; } }, - this.cache, + this.shouldUseCache ? this.cache : null, referenceCatalogName); }