Skip to content

Commit 9fc0632

Browse files
authored
Remove all tightly coupled EntityCache dependencies in the main persistence stack (#1055)
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.
1 parent f5b4d6f commit 9fc0632

File tree

15 files changed

+672
-710
lines changed

15 files changed

+672
-710
lines changed

polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,23 @@
2525
import java.util.HashMap;
2626
import java.util.List;
2727
import java.util.Map;
28-
import java.util.Objects;
2928
import java.util.Optional;
3029
import java.util.function.Predicate;
3130
import java.util.stream.Collectors;
3231
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
3332

33+
/**
34+
* For legacy reasons, this class is only a thin facade over PolarisBaseEntity's members/methods. No
35+
* direct members should be added to this class; rather, they should reside in the PolarisBaseEntity
36+
* and this class should just contain the relevant builder methods, etc. The intention when using
37+
* this class is to use "immutable" semantics as much as possible, for example constructing new
38+
* copies with the Builder pattern when "mutating" fields rather than ever chaing fields in-place.
39+
* Currently, code that intends to operate directly on a PolarisBaseEntity may not adhere to
40+
* immutability semantics, and may modify the entity in-place.
41+
*
42+
* <p>TODO: Combine this fully into PolarisBaseEntity, refactor all callsites to use strict
43+
* immutability semantics, and remove all mutator methods from PolarisBaseEntity.
44+
*/
3445
public class PolarisEntity extends PolarisBaseEntity {
3546

3647
public static class NameAndId {
@@ -227,41 +238,18 @@ public String toString() {
227238
@Override
228239
public boolean equals(Object o) {
229240
if (this == o) return true;
230-
if (!(o instanceof PolarisEntity)) return false;
231-
PolarisEntity that = (PolarisEntity) o;
232-
return catalogId == that.catalogId
233-
&& id == that.id
234-
&& parentId == that.parentId
235-
&& createTimestamp == that.createTimestamp
236-
&& dropTimestamp == that.dropTimestamp
237-
&& purgeTimestamp == that.purgeTimestamp
238-
&& lastUpdateTimestamp == that.lastUpdateTimestamp
239-
&& entityVersion == that.entityVersion
240-
&& grantRecordsVersion == that.grantRecordsVersion
241-
&& typeCode == that.typeCode
242-
&& subTypeCode == that.subTypeCode
243-
&& Objects.equals(name, that.name)
244-
&& Objects.equals(properties, that.properties)
245-
&& Objects.equals(internalProperties, that.internalProperties);
241+
// Note: Keeping this here explicitly instead silently inheriting super.equals as a more
242+
// prominent warning that the data members of this class *must not* diverge from those of
243+
// PolarisBaseEntity.
244+
return super.equals(o);
246245
}
247246

248247
@Override
249248
public int hashCode() {
250-
return Objects.hash(
251-
typeCode,
252-
subTypeCode,
253-
catalogId,
254-
id,
255-
parentId,
256-
name,
257-
createTimestamp,
258-
dropTimestamp,
259-
purgeTimestamp,
260-
lastUpdateTimestamp,
261-
properties,
262-
internalProperties,
263-
entityVersion,
264-
grantRecordsVersion);
249+
// Note: Keeping this here explicitly instead silently inheriting super.hashCode as a more
250+
// prominent warning that the data members of this class *must not* diverge from those of
251+
// PolarisBaseEntity.
252+
return super.hashCode();
265253
}
266254

267255
public static class Builder extends BaseBuilder<PolarisEntity, Builder> {

polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java

Lines changed: 198 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,23 @@
2929
import org.apache.polaris.core.auth.PolarisGrantManager;
3030
import org.apache.polaris.core.auth.PolarisSecretsManager;
3131
import org.apache.polaris.core.entity.PolarisBaseEntity;
32+
import org.apache.polaris.core.entity.PolarisChangeTrackingVersions;
3233
import org.apache.polaris.core.entity.PolarisEntity;
3334
import org.apache.polaris.core.entity.PolarisEntityActiveRecord;
3435
import org.apache.polaris.core.entity.PolarisEntityCore;
36+
import org.apache.polaris.core.entity.PolarisEntityId;
3537
import org.apache.polaris.core.entity.PolarisEntitySubType;
3638
import org.apache.polaris.core.entity.PolarisEntityType;
39+
import org.apache.polaris.core.entity.PolarisGrantRecord;
3740
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
38-
import org.apache.polaris.core.persistence.cache.PolarisRemoteCache;
3941
import org.apache.polaris.core.storage.PolarisCredentialVendor;
4042

4143
/**
4244
* Polaris Metastore Manager manages all Polaris entities and associated grant records metadata for
4345
* authorization. It uses the underlying persistent metastore to store and retrieve Polaris metadata
4446
*/
4547
public interface PolarisMetaStoreManager
46-
extends PolarisSecretsManager,
47-
PolarisGrantManager,
48-
PolarisRemoteCache,
49-
PolarisCredentialVendor {
48+
extends PolarisSecretsManager, PolarisGrantManager, PolarisCredentialVendor {
5049

5150
/**
5251
* Bootstrap the Polaris service, creating the root catalog, root principal, and associated
@@ -688,4 +687,198 @@ DropEntityResult dropEntityIfExists(
688687
*/
689688
@Nonnull
690689
EntitiesResult loadTasks(@Nonnull PolarisCallContext callCtx, String executorId, int limit);
690+
691+
/** Result of a loadEntitiesChangeTracking call */
692+
class ChangeTrackingResult extends BaseResult {
693+
694+
// null if not success. Else, will be null if the grant to revoke was not found
695+
private final List<PolarisChangeTrackingVersions> changeTrackingVersions;
696+
697+
/**
698+
* Constructor for an error
699+
*
700+
* @param errorCode error code, cannot be SUCCESS
701+
* @param extraInformation extra information
702+
*/
703+
public ChangeTrackingResult(
704+
@Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) {
705+
super(errorCode, extraInformation);
706+
this.changeTrackingVersions = null;
707+
}
708+
709+
/**
710+
* Constructor for success
711+
*
712+
* @param changeTrackingVersions change tracking versions
713+
*/
714+
public ChangeTrackingResult(
715+
@Nonnull List<PolarisChangeTrackingVersions> changeTrackingVersions) {
716+
super(BaseResult.ReturnStatus.SUCCESS);
717+
this.changeTrackingVersions = changeTrackingVersions;
718+
}
719+
720+
@JsonCreator
721+
private ChangeTrackingResult(
722+
@JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus,
723+
@JsonProperty("extraInformation") String extraInformation,
724+
@JsonProperty("changeTrackingVersions")
725+
List<PolarisChangeTrackingVersions> changeTrackingVersions) {
726+
super(returnStatus, extraInformation);
727+
this.changeTrackingVersions = changeTrackingVersions;
728+
}
729+
730+
public List<PolarisChangeTrackingVersions> getChangeTrackingVersions() {
731+
return changeTrackingVersions;
732+
}
733+
}
734+
735+
/**
736+
* Load change tracking information for a set of entities in one single shot and return for each
737+
* the version for the entity itself and the version associated to its grant records.
738+
*
739+
* @param callCtx call context
740+
* @param entityIds list of catalog/entity pair ids for which we need to efficiently load the
741+
* version information, both entity version and grant records version.
742+
* @return a list of version tracking information. Order in that returned list is the same as the
743+
* input list. Some elements might be NULL if the entity has been purged. Not expected to fail
744+
*/
745+
@Nonnull
746+
ChangeTrackingResult loadEntitiesChangeTracking(
747+
@Nonnull PolarisCallContext callCtx, @Nonnull List<PolarisEntityId> entityIds);
748+
749+
/**
750+
* Represents an entity with its grants. If we "refresh" a previously fetched entity, we will only
751+
* refresh the information which has changed, based on the version of the entity.
752+
*/
753+
class ResolvedEntityResult extends BaseResult {
754+
755+
// the entity itself if it was loaded
756+
private final @Nullable PolarisBaseEntity entity;
757+
758+
// version for the grant records, in case the entity was not loaded
759+
private final int grantRecordsVersion;
760+
761+
private final @Nullable List<PolarisGrantRecord> entityGrantRecords;
762+
763+
/**
764+
* Constructor for an error
765+
*
766+
* @param errorCode error code, cannot be SUCCESS
767+
* @param extraInformation extra information
768+
*/
769+
public ResolvedEntityResult(
770+
@Nonnull BaseResult.ReturnStatus errorCode, @Nullable String extraInformation) {
771+
super(errorCode, extraInformation);
772+
this.entity = null;
773+
this.entityGrantRecords = null;
774+
this.grantRecordsVersion = 0;
775+
}
776+
777+
/**
778+
* Constructor with success
779+
*
780+
* @param entity the main entity
781+
* @param grantRecordsVersion the version of the grant records
782+
* @param entityGrantRecords the list of grant records
783+
*/
784+
public ResolvedEntityResult(
785+
@Nullable PolarisBaseEntity entity,
786+
int grantRecordsVersion,
787+
@Nullable List<PolarisGrantRecord> entityGrantRecords) {
788+
super(BaseResult.ReturnStatus.SUCCESS);
789+
this.entity = entity;
790+
this.entityGrantRecords = entityGrantRecords;
791+
this.grantRecordsVersion = grantRecordsVersion;
792+
}
793+
794+
@JsonCreator
795+
public ResolvedEntityResult(
796+
@JsonProperty("returnStatus") @Nonnull BaseResult.ReturnStatus returnStatus,
797+
@JsonProperty("extraInformation") String extraInformation,
798+
@Nullable @JsonProperty("entity") PolarisBaseEntity entity,
799+
@JsonProperty("grantRecordsVersion") int grantRecordsVersion,
800+
@Nullable @JsonProperty("entityGrantRecords") List<PolarisGrantRecord> entityGrantRecords) {
801+
super(returnStatus, extraInformation);
802+
this.entity = entity;
803+
this.entityGrantRecords = entityGrantRecords;
804+
this.grantRecordsVersion = grantRecordsVersion;
805+
}
806+
807+
public @Nullable PolarisBaseEntity getEntity() {
808+
return entity;
809+
}
810+
811+
public int getGrantRecordsVersion() {
812+
return grantRecordsVersion;
813+
}
814+
815+
public @Nullable List<PolarisGrantRecord> getEntityGrantRecords() {
816+
return entityGrantRecords;
817+
}
818+
}
819+
820+
/**
821+
* Load a resolved entity, i.e. an entity definition and associated grant records, from the
822+
* backend store. The entity is identified by its id (entity catalog id and id).
823+
*
824+
* <p>For entities that can be grantees, the associated grant records will include both the grant
825+
* records for this entity as a grantee and for this entity as a securable.
826+
*
827+
* @param callCtx call context
828+
* @param entityCatalogId id of the catalog for that entity
829+
* @param entityId id of the entity
830+
* @return result with entity and grants. Status will be ENTITY_NOT_FOUND if the entity was not
831+
* found
832+
*/
833+
@Nonnull
834+
ResolvedEntityResult loadResolvedEntityById(
835+
@Nonnull PolarisCallContext callCtx, long entityCatalogId, long entityId);
836+
837+
/**
838+
* Load a resolved entity, i.e. an entity definition and associated grant records, from the
839+
* backend store. The entity is identified by its name. Will return NULL if the entity does not
840+
* exist, i.e. has been purged or dropped.
841+
*
842+
* <p>For entities that can be grantees, the associated grant records will include both the grant
843+
* records for this entity as a grantee and for this entity as a securable.
844+
*
845+
* @param callCtx call context
846+
* @param entityCatalogId id of the catalog for that entity
847+
* @param parentId the id of the parent of that entity
848+
* @param entityType the type of this entity
849+
* @param entityName the name of this entity
850+
* @return result with entity and grants. Status will be ENTITY_NOT_FOUND if the entity was not
851+
* found
852+
*/
853+
@Nonnull
854+
ResolvedEntityResult loadResolvedEntityByName(
855+
@Nonnull PolarisCallContext callCtx,
856+
long entityCatalogId,
857+
long parentId,
858+
@Nonnull PolarisEntityType entityType,
859+
@Nonnull String entityName);
860+
861+
/**
862+
* Refresh a resolved entity from the backend store. Will return NULL if the entity does not
863+
* exist, i.e. has been purged or dropped. Else, will determine what has changed based on the
864+
* version information sent by the caller and will return only what has changed.
865+
*
866+
* <p>For entities that can be grantees, the associated grant records will include both the grant
867+
* records for this entity as a grantee and for this entity as a securable.
868+
*
869+
* @param callCtx call context
870+
* @param entityType type of the entity whose entity and grants we are refreshing
871+
* @param entityCatalogId id of the catalog for that entity
872+
* @param entityId the id of the entity to load
873+
* @return result with entity and grants. Status will be ENTITY_NOT_FOUND if the entity was not
874+
* found
875+
*/
876+
@Nonnull
877+
ResolvedEntityResult refreshResolvedEntity(
878+
@Nonnull PolarisCallContext callCtx,
879+
int entityVersion,
880+
int entityGrantRecordsVersion,
881+
@Nonnull PolarisEntityType entityType,
882+
long entityCatalogId,
883+
long entityId);
691884
}

0 commit comments

Comments
 (0)