Skip to content

Commit 4a62e67

Browse files
committed
Rework PolarisMetaStoreManager.listEntities
the central `BasePersistence.listEntities` method supports pagination, filtering loaded entities via a Predicate and transforming the loaded entities to a more specific type. `PolarisMetaStoreManager.listEntities` only exposed a limited subset of that functionality i.e. the retrieval of a `ListEntitiesResult`. almost all callers then had to post-process the lookup records of `ListEntitiesResult` and called `PolarisMetaStoreManager.loadEntity` on the individual items to transform and filter them. this is bad for the following reasons: - suboptimal performance as we run N+1 queries to basically load every entity twice from the persistence backend - suffering from race-conditions when entities get dropped between the `listEntities` and `loadEntity` call - a lot of repeated code in all the callers (of which only some are dealing with the race-condition by filtering out `null` values) the fix is to simply let `PolarisMetaStoreManager.listEntities` offer all capabilities of the underlying `BasePersistence.listEntities` method. as it turns out `ListEntitiesResult` and `PolarisEntity.NameAndId` become completely obsolete. note that we remove `testCatalogNotReturnedWhenDeletedAfterListBeforeGet` from `ManagementServiceTest` because the simulated race-condition scenario can not happen any longer.
1 parent cfff798 commit 4a62e67

File tree

16 files changed

+238
-495
lines changed

16 files changed

+238
-495
lines changed

persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -451,14 +451,7 @@ public List<EntityNameLookupRecord> lookupEntityActiveBatchInCurrentTxn(
451451
parentId,
452452
entityType,
453453
entityFilter,
454-
entity ->
455-
new EntityNameLookupRecord(
456-
entity.getCatalogId(),
457-
entity.getId(),
458-
entity.getParentId(),
459-
entity.getName(),
460-
entity.getTypeCode(),
461-
entity.getSubTypeCode()),
454+
EntityNameLookupRecord::new,
462455
pageToken);
463456
}
464457

polaris-core/src/main/java/org/apache/polaris/core/catalog/PolarisCatalogHelpers.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.stream.Collectors;
2525
import org.apache.iceberg.catalog.Namespace;
2626
import org.apache.iceberg.catalog.TableIdentifier;
27+
import org.apache.polaris.core.entity.PolarisBaseEntity;
2728
import org.apache.polaris.core.entity.PolarisEntity;
2829

2930
/**
@@ -61,8 +62,8 @@ public static Namespace getParentNamespace(Namespace namespace) {
6162
return Namespace.of(parentLevels);
6263
}
6364

64-
public static Namespace nameAndIdToNamespace(
65-
List<PolarisEntity> catalogPath, PolarisEntity.NameAndId entity) {
65+
public static Namespace entityToNamespace(
66+
List<PolarisEntity> catalogPath, PolarisBaseEntity entity) {
6667
// Skip element 0 which is the catalog entity
6768
String[] fullName = new String[catalogPath.size()];
6869
for (int i = 0; i < fullName.length - 1; ++i) {
@@ -86,11 +87,11 @@ public static Namespace parentNamespace(List<PolarisEntity> catalogPath) {
8687
}
8788

8889
/**
89-
* Given the shortnames/ids of entities that all live under the given catalogPath, reconstructs
90-
* TableIdentifier objects for each that all hold the catalogPath excluding the catalog entity.
90+
* Given the entities that all live under the given catalogPath, reconstructs TableIdentifier
91+
* objects for each that all hold the catalogPath excluding the catalog entity.
9192
*/
92-
public static List<TableIdentifier> nameAndIdToTableIdentifiers(
93-
List<PolarisEntity> catalogPath, List<PolarisEntity.NameAndId> entities) {
93+
public static List<TableIdentifier> entitiesToTableIdentifiers(
94+
List<PolarisEntity> catalogPath, List<PolarisEntity> entities) {
9495
// Skip element 0 which is the catalog entity
9596
String[] parentNamespaces = new String[catalogPath.size() - 1];
9697
for (int i = 0; i < parentNamespaces.length; ++i) {

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

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -44,48 +44,6 @@
4444
*/
4545
public class PolarisEntity extends PolarisBaseEntity {
4646

47-
public static class NameAndId {
48-
private final String name;
49-
private final long id;
50-
51-
public NameAndId(String name, long id) {
52-
this.name = name;
53-
this.id = id;
54-
}
55-
56-
public String getName() {
57-
return name;
58-
}
59-
60-
public long getId() {
61-
return id;
62-
}
63-
}
64-
65-
public static class TypeSubTypeAndName {
66-
private final PolarisEntityType type;
67-
private final PolarisEntitySubType subType;
68-
private final String name;
69-
70-
public TypeSubTypeAndName(PolarisEntityType type, PolarisEntitySubType subType, String name) {
71-
this.type = type;
72-
this.subType = subType;
73-
this.name = name;
74-
}
75-
76-
public PolarisEntityType getType() {
77-
return type;
78-
}
79-
80-
public PolarisEntitySubType getSubType() {
81-
return subType;
82-
}
83-
84-
public String getName() {
85-
return name;
86-
}
87-
}
88-
8947
@JsonCreator
9048
private PolarisEntity(
9149
@JsonProperty("catalogId") long catalogId,
@@ -183,16 +141,6 @@ public static List<PolarisEntityCore> toCoreList(List<PolarisEntity> path) {
183141
.orElse(null);
184142
}
185143

186-
public static List<NameAndId> toNameAndIdList(List<EntityNameLookupRecord> entities) {
187-
return Optional.ofNullable(entities)
188-
.map(
189-
list ->
190-
list.stream()
191-
.map(record -> new NameAndId(record.getName(), record.getId()))
192-
.collect(Collectors.toList()))
193-
.orElse(null);
194-
}
195-
196144
public PolarisEntity(@Nonnull PolarisBaseEntity sourceEntity) {
197145
super(
198146
sourceEntity.getCatalogId(),
@@ -223,11 +171,6 @@ public PolarisEntitySubType getSubType() {
223171
return PolarisEntitySubType.fromCode(getSubTypeCode());
224172
}
225173

226-
@JsonIgnore
227-
public NameAndId nameAndId() {
228-
return new NameAndId(name, id);
229-
}
230-
231174
@Override
232175
public String toString() {
233176
return "name="

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@
5757
import org.apache.polaris.core.persistence.dao.entity.EntitiesResult;
5858
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
5959
import org.apache.polaris.core.persistence.dao.entity.EntityWithPath;
60-
import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult;
6160
import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult;
6261
import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult;
6362
import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult;
@@ -692,11 +691,13 @@ private void revokeGrantRecord(
692691

693692
/** {@inheritDoc} */
694693
@Override
695-
public @Nonnull ListEntitiesResult listEntities(
694+
public @Nonnull <T> Page<T> listEntities(
696695
@Nonnull PolarisCallContext callCtx,
697696
@Nullable List<PolarisEntityCore> catalogPath,
698697
@Nonnull PolarisEntityType entityType,
699698
@Nonnull PolarisEntitySubType entitySubType,
699+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
700+
@Nonnull Function<PolarisBaseEntity, T> transformer,
700701
@Nonnull PageToken pageToken) {
701702
// get meta store we should be using
702703
BasePersistence ms = callCtx.getMetaStore();
@@ -709,14 +710,12 @@ private void revokeGrantRecord(
709710
? 0L
710711
: catalogPath.get(catalogPath.size() - 1).getId();
711712

712-
// prune the returned list with only entities matching the entity subtype
713-
Predicate<PolarisBaseEntity> filter =
714-
entitySubType != PolarisEntitySubType.ANY_SUBTYPE
715-
? e -> e.getSubTypeCode() == entitySubType.getCode()
716-
: entity -> true;
717-
718-
Page<EntityNameLookupRecord> resultPage =
719-
ms.listEntities(callCtx, catalogId, parentId, entityType, filter, pageToken);
713+
Predicate<PolarisBaseEntity> filter;
714+
if (entitySubType == PolarisEntitySubType.ANY_SUBTYPE) {
715+
filter = entityFilter;
716+
} else {
717+
filter = e -> e.getSubTypeCode() == entitySubType.getCode() && entityFilter.test(e);
718+
}
720719

721720
// TODO: Use post-validation to enforce consistent view against catalogPath. In the
722721
// meantime, happens-before ordering semantics aren't guaranteed during high-concurrency
@@ -725,7 +724,8 @@ private void revokeGrantRecord(
725724
// in-flight request (the cache-based resolution follows a different path entirely).
726725

727726
// done
728-
return ListEntitiesResult.fromPage(resultPage);
727+
return ms.listEntities(
728+
callCtx, catalogId, parentId, entityType, filter, transformer, pageToken);
729729
}
730730

731731
/** {@inheritDoc} */

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

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import java.util.List;
2424
import java.util.Map;
2525
import java.util.Optional;
26+
import java.util.function.Function;
27+
import java.util.function.Predicate;
2628
import org.apache.polaris.core.PolarisCallContext;
2729
import org.apache.polaris.core.auth.PolarisGrantManager;
2830
import org.apache.polaris.core.auth.PolarisSecretsManager;
@@ -45,8 +47,8 @@
4547
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
4648
import org.apache.polaris.core.persistence.dao.entity.EntityWithPath;
4749
import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult;
48-
import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult;
4950
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
51+
import org.apache.polaris.core.persistence.pagination.Page;
5052
import org.apache.polaris.core.persistence.pagination.PageToken;
5153
import org.apache.polaris.core.policy.PolarisPolicyMappingManager;
5254
import org.apache.polaris.core.storage.PolarisCredentialVendor;
@@ -110,25 +112,57 @@ EntityResult readEntityByName(
110112
@Nonnull String name);
111113

112114
/**
113-
* List all entities of the specified type under the specified catalogPath. If the catalogPath is
114-
* null, listed entities will be top-level entities like catalogs.
115+
* List entities where some predicate returns true and transform the entities with a function
115116
*
116117
* @param callCtx call context
117118
* @param catalogPath path inside a catalog. If null or empty, the entities to list are top-level,
118119
* like catalogs
119-
* @param entityType entity type
120-
* @param entitySubType entity subtype. Can be the special value ANY_SUBTYPE to match any subtype.
121-
* Else exact match will be performed.
122-
* @return all entities name, ids and subtype under the specified namespace.
120+
* @param entityType type of entities to list
121+
* @param entityFilter the filter to be applied to each entity. Only entities where the predicate
122+
* returns true are returned in the list
123+
* @param transformer the transformation function applied to the {@link PolarisBaseEntity} before
124+
* returning
125+
* @return the paged list of entities for which the predicate returns true
123126
*/
124127
@Nonnull
125-
ListEntitiesResult listEntities(
128+
<T> Page<T> listEntities(
126129
@Nonnull PolarisCallContext callCtx,
127130
@Nullable List<PolarisEntityCore> catalogPath,
128131
@Nonnull PolarisEntityType entityType,
129132
@Nonnull PolarisEntitySubType entitySubType,
133+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
134+
@Nonnull Function<PolarisBaseEntity, T> transformer,
130135
@Nonnull PageToken pageToken);
131136

137+
/**
138+
* List all entities and transform the entities with a function
139+
*
140+
* @param callCtx call context
141+
* @param catalogPath path inside a catalog. If null or empty, the entities to list are top-level,
142+
* like catalogs
143+
* @param entityType type of entities to list
144+
* @param transformer the transformation function applied to the {@link PolarisBaseEntity} before
145+
* returning
146+
* @return the full list of entities
147+
*/
148+
@Nonnull
149+
default <T> List<T> listAllEntities(
150+
@Nonnull PolarisCallContext callCtx,
151+
@Nullable List<PolarisEntityCore> catalogPath,
152+
@Nonnull PolarisEntityType entityType,
153+
@Nonnull PolarisEntitySubType entitySubType,
154+
@Nonnull Function<PolarisBaseEntity, T> transformer) {
155+
return listEntities(
156+
callCtx,
157+
catalogPath,
158+
entityType,
159+
entitySubType,
160+
e -> true,
161+
transformer,
162+
PageToken.readEverything())
163+
.items();
164+
}
165+
132166
/**
133167
* Generate a new unique id that can be used by the Polaris client when it needs to create a new
134168
* entity

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import java.util.Map;
2727
import java.util.Optional;
2828
import java.util.Set;
29+
import java.util.function.Function;
30+
import java.util.function.Predicate;
2931
import org.apache.polaris.core.PolarisCallContext;
3032
import org.apache.polaris.core.entity.LocationBasedEntity;
3133
import org.apache.polaris.core.entity.PolarisBaseEntity;
@@ -44,14 +46,14 @@
4446
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
4547
import org.apache.polaris.core.persistence.dao.entity.EntityWithPath;
4648
import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult;
47-
import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult;
4849
import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult;
4950
import org.apache.polaris.core.persistence.dao.entity.LoadPolicyMappingsResult;
5051
import org.apache.polaris.core.persistence.dao.entity.PolicyAttachmentResult;
5152
import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult;
5253
import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult;
5354
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
5455
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
56+
import org.apache.polaris.core.persistence.pagination.Page;
5557
import org.apache.polaris.core.persistence.pagination.PageToken;
5658
import org.apache.polaris.core.policy.PolicyEntity;
5759
import org.apache.polaris.core.policy.PolicyType;
@@ -117,11 +119,13 @@ public EntityResult readEntityByName(
117119
}
118120

119121
@Override
120-
public ListEntitiesResult listEntities(
122+
public @Nonnull <T> Page<T> listEntities(
121123
@Nonnull PolarisCallContext callCtx,
122124
@Nullable List<PolarisEntityCore> catalogPath,
123125
@Nonnull PolarisEntityType entityType,
124126
@Nonnull PolarisEntitySubType entitySubType,
127+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
128+
@Nonnull Function<PolarisBaseEntity, T> transformer,
125129
@Nonnull PageToken pageToken) {
126130
callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "listEntities");
127131
return null;

polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ListEntitiesResult.java

Lines changed: 0 additions & 66 deletions
This file was deleted.

0 commit comments

Comments
 (0)