Skip to content

Commit a478c1d

Browse files
XN137XN137
committed
Add PolarisMetaStoreManager.loadEntities
currently `PolarisMetaStoreManager.listEntities` only exposes a limited subset of the underlying `BasePersistence.listEntities` functionality. most of the callers have to post-process the `EntityNameLookupRecord` of `ListEntitiesResult` and call `PolarisMetaStoreManager.loadEntity` on the individual items sequentually 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) as a solution we add `PolarisMetaStoreManager.loadEntities` that takes advantage of the already existing `BasePersistence` methods. since many callers dont need paging and want the result as a list, we add `PolarisMetaStoreManager.loadEntitiesAll` as a convenient wrapper. we also remove the `PolarisEntity.nameAndId` method as callers who only need name and id should not be loading the full entity to begin with. note we rework `testCatalogNotReturnedWhenDeletedAfterListBeforeGet` from `ManagementServiceTest` because the simulated race-condition scenario can no longer happen.
1 parent 8a5b80a commit a478c1d

File tree

13 files changed

+238
-157
lines changed

13 files changed

+238
-157
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

‎persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ public <T> Page<T> listEntities(
463463
@Nonnull PolarisCallContext callCtx,
464464
long catalogId,
465465
long parentId,
466-
PolarisEntityType entityType,
466+
@Nonnull PolarisEntityType entityType,
467467
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
468468
@Nonnull Function<PolarisBaseEntity, T> transformer,
469469
@Nonnull PageToken pageToken) {

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,6 @@ public PolarisEntitySubType getSubType() {
223223
return PolarisEntitySubType.fromCode(getSubTypeCode());
224224
}
225225

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

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,45 @@ private void revokeGrantRecord(
728728
return ListEntitiesResult.fromPage(resultPage);
729729
}
730730

731+
/** {@inheritDoc} */
732+
@Override
733+
public @Nonnull <T> Page<T> loadEntities(
734+
@Nonnull PolarisCallContext callCtx,
735+
@Nullable List<PolarisEntityCore> catalogPath,
736+
@Nonnull PolarisEntityType entityType,
737+
@Nonnull PolarisEntitySubType entitySubType,
738+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
739+
@Nonnull Function<PolarisBaseEntity, T> transformer,
740+
@Nonnull PageToken pageToken) {
741+
// get meta store we should be using
742+
BasePersistence ms = callCtx.getMetaStore();
743+
744+
// return list of active entities
745+
// TODO: Clean up shared logic for catalogId/parentId
746+
long catalogId = catalogPath == null || catalogPath.isEmpty() ? 0L : catalogPath.get(0).getId();
747+
long parentId =
748+
catalogPath == null || catalogPath.isEmpty()
749+
? 0L
750+
: catalogPath.get(catalogPath.size() - 1).getId();
751+
752+
Predicate<PolarisBaseEntity> filter;
753+
if (entitySubType == PolarisEntitySubType.ANY_SUBTYPE) {
754+
filter = entityFilter;
755+
} else {
756+
filter = e -> e.getSubTypeCode() == entitySubType.getCode() && entityFilter.test(e);
757+
}
758+
759+
// TODO: Use post-validation to enforce consistent view against catalogPath. In the
760+
// meantime, happens-before ordering semantics aren't guaranteed during high-concurrency
761+
// race conditions, such as first revoking a grant on a namespace before adding a table
762+
// with sensitive data; but the window of inconsistency is only the duration of a single
763+
// in-flight request (the cache-based resolution follows a different path entirely).
764+
765+
// done
766+
return ms.listEntities(
767+
callCtx, catalogId, parentId, entityType, filter, transformer, pageToken);
768+
}
769+
731770
/** {@inheritDoc} */
732771
@Override
733772
public @Nonnull CreatePrincipalResult createPrincipal(

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

Lines changed: 56 additions & 0 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;
@@ -47,6 +49,7 @@
4749
import org.apache.polaris.core.persistence.dao.entity.GenerateEntityIdResult;
4850
import org.apache.polaris.core.persistence.dao.entity.ListEntitiesResult;
4951
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
52+
import org.apache.polaris.core.persistence.pagination.Page;
5053
import org.apache.polaris.core.persistence.pagination.PageToken;
5154
import org.apache.polaris.core.policy.PolarisPolicyMappingManager;
5255
import org.apache.polaris.core.storage.PolarisCredentialVendor;
@@ -129,6 +132,59 @@ ListEntitiesResult listEntities(
129132
@Nonnull PolarisEntitySubType entitySubType,
130133
@Nonnull PageToken pageToken);
131134

135+
/**
136+
* Load entities where some predicate returns true and transform the entities with a function
137+
*
138+
* @param callCtx call context
139+
* @param catalogPath path inside a catalog. If null or empty, the entities to list are top-level,
140+
* like catalogs
141+
* @param entityType type of entities to list
142+
* @param entitySubType subType of entities to list (or ANY_SUBTYPE)
143+
* @param entityFilter the filter to be applied to each entity. Only entities where the predicate
144+
* returns true are returned in the list
145+
* @param transformer the transformation function applied to the {@link PolarisBaseEntity} before
146+
* returning
147+
* @return the paged list of entities for which the predicate returns true
148+
*/
149+
@Nonnull
150+
<T> Page<T> loadEntities(
151+
@Nonnull PolarisCallContext callCtx,
152+
@Nullable List<PolarisEntityCore> catalogPath,
153+
@Nonnull PolarisEntityType entityType,
154+
@Nonnull PolarisEntitySubType entitySubType,
155+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
156+
@Nonnull Function<PolarisBaseEntity, T> transformer,
157+
@Nonnull PageToken pageToken);
158+
159+
/**
160+
* Load all entities and transform the entities with a function into an unpaged list
161+
*
162+
* @param callCtx call context
163+
* @param catalogPath path inside a catalog. If null or empty, the entities to list are top-level,
164+
* like catalogs
165+
* @param entityType type of entities to list
166+
* @param entitySubType subType of entities to list (or ANY_SUBTYPE)
167+
* @param transformer the transformation function applied to the {@link PolarisBaseEntity} before
168+
* returning
169+
* @return the full list of entities
170+
*/
171+
default @Nonnull <T> List<T> loadEntitiesAll(
172+
@Nonnull PolarisCallContext callCtx,
173+
@Nullable List<PolarisEntityCore> catalogPath,
174+
@Nonnull PolarisEntityType entityType,
175+
@Nonnull PolarisEntitySubType entitySubType,
176+
@Nonnull Function<PolarisBaseEntity, T> transformer) {
177+
return loadEntities(
178+
callCtx,
179+
catalogPath,
180+
entityType,
181+
entitySubType,
182+
e -> true,
183+
transformer,
184+
PageToken.readEverything())
185+
.items();
186+
}
187+
132188
/**
133189
* Generate a new unique id that can be used by the Polaris client when it needs to create a new
134190
* entity

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

Lines changed: 17 additions & 1 deletion
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;
@@ -52,6 +54,7 @@
5254
import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult;
5355
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
5456
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
57+
import org.apache.polaris.core.persistence.pagination.Page;
5558
import org.apache.polaris.core.persistence.pagination.PageToken;
5659
import org.apache.polaris.core.policy.PolicyEntity;
5760
import org.apache.polaris.core.policy.PolicyType;
@@ -117,7 +120,7 @@ public EntityResult readEntityByName(
117120
}
118121

119122
@Override
120-
public ListEntitiesResult listEntities(
123+
public @Nonnull ListEntitiesResult listEntities(
121124
@Nonnull PolarisCallContext callCtx,
122125
@Nullable List<PolarisEntityCore> catalogPath,
123126
@Nonnull PolarisEntityType entityType,
@@ -127,6 +130,19 @@ public ListEntitiesResult listEntities(
127130
return null;
128131
}
129132

133+
@Override
134+
public @Nonnull <T> Page<T> loadEntities(
135+
@Nonnull PolarisCallContext callCtx,
136+
@Nullable List<PolarisEntityCore> catalogPath,
137+
@Nonnull PolarisEntityType entityType,
138+
@Nonnull PolarisEntitySubType entitySubType,
139+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
140+
@Nonnull Function<PolarisBaseEntity, T> transformer,
141+
@Nonnull PageToken pageToken) {
142+
callCtx.getDiagServices().fail("illegal_method_in_transaction_workspace", "loadEntities");
143+
return null;
144+
}
145+
130146
@Override
131147
public GenerateEntityIdResult generateNewEntityId(@Nonnull PolarisCallContext callCtx) {
132148
callCtx

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,73 @@ private void bootstrapPolarisService(
747747
() -> listEntities(callCtx, ms, catalogPath, entityType, entitySubType, pageToken));
748748
}
749749

750+
/**
751+
* See {@link PolarisMetaStoreManager#loadEntities(PolarisCallContext, List, PolarisEntityType,
752+
* PolarisEntitySubType, Predicate, Function, PageToken)}
753+
*/
754+
private @Nonnull <T> Page<T> loadEntities(
755+
@Nonnull PolarisCallContext callCtx,
756+
@Nonnull TransactionalPersistence ms,
757+
@Nullable List<PolarisEntityCore> catalogPath,
758+
@Nonnull PolarisEntityType entityType,
759+
@Nonnull PolarisEntitySubType entitySubType,
760+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
761+
@Nonnull Function<PolarisBaseEntity, T> transformer,
762+
@Nonnull PageToken pageToken) {
763+
// first resolve again the catalogPath to that entity
764+
PolarisEntityResolver resolver = new PolarisEntityResolver(callCtx, ms, catalogPath);
765+
766+
// throw if we failed to resolve
767+
if (resolver.isFailure()) {
768+
throw new IllegalArgumentException("Failed to resolve catalogPath: " + catalogPath);
769+
}
770+
771+
Predicate<PolarisBaseEntity> filter;
772+
if (entitySubType == PolarisEntitySubType.ANY_SUBTYPE) {
773+
filter = entityFilter;
774+
} else {
775+
filter = e -> e.getSubTypeCode() == entitySubType.getCode() && entityFilter.test(e);
776+
}
777+
778+
// return list of active entities
779+
return ms.listEntitiesInCurrentTxn(
780+
callCtx,
781+
resolver.getCatalogIdOrNull(),
782+
resolver.getParentId(),
783+
entityType,
784+
filter,
785+
transformer,
786+
pageToken);
787+
}
788+
789+
/** {@inheritDoc} */
790+
@Override
791+
public @Nonnull <T> Page<T> loadEntities(
792+
@Nonnull PolarisCallContext callCtx,
793+
@Nullable List<PolarisEntityCore> catalogPath,
794+
@Nonnull PolarisEntityType entityType,
795+
@Nonnull PolarisEntitySubType entitySubType,
796+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
797+
@Nonnull Function<PolarisBaseEntity, T> transformer,
798+
@Nonnull PageToken pageToken) {
799+
// get meta store we should be using
800+
TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore());
801+
802+
// run operation in a read transaction
803+
return ms.runInReadTransaction(
804+
callCtx,
805+
() ->
806+
loadEntities(
807+
callCtx,
808+
ms,
809+
catalogPath,
810+
entityType,
811+
entitySubType,
812+
entityFilter,
813+
transformer,
814+
pageToken));
815+
}
816+
750817
/** {@link #createPrincipal(PolarisCallContext, PolarisBaseEntity)} */
751818
private @Nonnull CreatePrincipalResult createPrincipal(
752819
@Nonnull PolarisCallContext callCtx,

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -288,15 +288,7 @@ public EntityNameLookupRecord lookupEntityActiveInCurrentTxn(
288288
entityActiveKey.getName()));
289289

290290
// return record
291-
return (entity == null)
292-
? null
293-
: new EntityNameLookupRecord(
294-
entity.getCatalogId(),
295-
entity.getId(),
296-
entity.getParentId(),
297-
entity.getName(),
298-
entity.getTypeCode(),
299-
entity.getSubTypeCode());
291+
return entity == null ? null : new EntityNameLookupRecord(entity);
300292
}
301293

302294
/** {@inheritDoc} */
@@ -338,14 +330,7 @@ public List<EntityNameLookupRecord> lookupEntityActiveBatchInCurrentTxn(
338330
parentId,
339331
entityType,
340332
entityFilter,
341-
entity ->
342-
new EntityNameLookupRecord(
343-
entity.getCatalogId(),
344-
entity.getId(),
345-
entity.getParentId(),
346-
entity.getName(),
347-
entity.getTypeCode(),
348-
entity.getSubTypeCode()),
333+
EntityNameLookupRecord::new,
349334
pageToken);
350335
}
351336

‎polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import java.util.stream.Stream;
3737
import org.apache.polaris.core.PolarisCallContext;
3838
import org.apache.polaris.core.entity.AsyncTaskType;
39-
import org.apache.polaris.core.entity.EntityNameLookupRecord;
4039
import org.apache.polaris.core.entity.PolarisBaseEntity;
4140
import org.apache.polaris.core.entity.PolarisEntity;
4241
import org.apache.polaris.core.entity.PolarisEntitySubType;
@@ -117,33 +116,18 @@ protected void testCreateEntities() {
117116
.extracting(PolarisEntity::toCore)
118117
.containsExactly(PolarisEntity.toCore(task1), PolarisEntity.toCore(task2));
119118

120-
List<EntityNameLookupRecord> listedEntities =
121-
metaStoreManager
122-
.listEntities(
123-
polarisTestMetaStoreManager.polarisCallContext,
124-
null,
125-
PolarisEntityType.TASK,
126-
PolarisEntitySubType.NULL_SUBTYPE,
127-
PageToken.readEverything())
128-
.getEntities();
119+
List<TaskEntity> listedEntities =
120+
metaStoreManager.loadEntitiesAll(
121+
polarisTestMetaStoreManager.polarisCallContext,
122+
null,
123+
PolarisEntityType.TASK,
124+
PolarisEntitySubType.NULL_SUBTYPE,
125+
TaskEntity::of);
129126
Assertions.assertThat(listedEntities)
130127
.isNotNull()
131128
.hasSize(2)
132-
.containsExactly(
133-
new EntityNameLookupRecord(
134-
task1.getCatalogId(),
135-
task1.getId(),
136-
task1.getParentId(),
137-
task1.getName(),
138-
task1.getTypeCode(),
139-
task1.getSubTypeCode()),
140-
new EntityNameLookupRecord(
141-
task2.getCatalogId(),
142-
task2.getId(),
143-
task2.getParentId(),
144-
task2.getName(),
145-
task2.getTypeCode(),
146-
task2.getSubTypeCode()));
129+
.extracting(PolarisEntity::toCore)
130+
.containsExactly(PolarisEntity.toCore(task1), PolarisEntity.toCore(task2));
147131
}
148132

149133
@Test

0 commit comments

Comments
 (0)