Skip to content

Commit ab37780

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. we rename one of the `listEntities` methods to `loadEntities` for consistency. 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 596239c commit ab37780

File tree

16 files changed

+244
-155
lines changed

16 files changed

+244
-155
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ public List<EntityNameLookupRecord> lookupEntityActiveBatchInCurrentTxn(
425425
}
426426

427427
@Override
428-
public @Nonnull <T> Page<T> listEntitiesInCurrentTxn(
428+
public @Nonnull <T> Page<T> loadEntitiesInCurrentTxn(
429429
@Nonnull PolarisCallContext callCtx,
430430
long catalogId,
431431
long parentId,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ public Page<EntityNameLookupRecord> listEntities(
431431
@Nonnull PolarisEntitySubType entitySubType,
432432
@Nonnull PageToken pageToken) {
433433
// TODO: only fetch the properties required for creating an EntityNameLookupRecord
434-
return listEntities(
434+
return loadEntities(
435435
callCtx,
436436
catalogId,
437437
parentId,
@@ -444,7 +444,7 @@ public Page<EntityNameLookupRecord> listEntities(
444444

445445
@Nonnull
446446
@Override
447-
public <T> Page<T> listEntities(
447+
public <T> Page<T> loadEntities(
448448
@Nonnull PolarisCallContext callCtx,
449449
long catalogId,
450450
long parentId,

‎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: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Set;
3232
import java.util.concurrent.atomic.AtomicInteger;
3333
import java.util.function.Function;
34+
import java.util.function.Predicate;
3435
import java.util.stream.Collectors;
3536
import org.apache.polaris.core.PolarisCallContext;
3637
import org.apache.polaris.core.entity.AsyncTaskType;
@@ -717,10 +718,47 @@ private void revokeGrantRecord(
717718
// with sensitive data; but the window of inconsistency is only the duration of a single
718719
// in-flight request (the cache-based resolution follows a different path entirely).
719720

720-
// done
721721
return ListEntitiesResult.fromPage(resultPage);
722722
}
723723

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

11721210
// get the list of catalog roles, at most 2
11731211
List<PolarisBaseEntity> catalogRoles =
1174-
ms.listEntities(
1212+
ms.loadEntities(
11751213
callCtx,
11761214
catalogId,
11771215
catalogId,
@@ -1494,7 +1532,7 @@ private void revokeGrantRecord(
14941532

14951533
// find all available tasks
14961534
Page<PolarisBaseEntity> availableTasks =
1497-
ms.listEntities(
1535+
ms.loadEntities(
14981536
callCtx,
14991537
PolarisEntityConstants.getRootEntityId(),
15001538
PolarisEntityConstants.getRootEntityId(),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ Page<EntityNameLookupRecord> listEntities(
290290
@Nonnull PageToken pageToken);
291291

292292
/**
293-
* List entities where some predicate returns true and transform the entities with a function
293+
* Load entities where some predicate returns true and transform the entities with a function
294294
*
295295
* @param callCtx call context
296296
* @param catalogId catalog id for that entity, NULL_ID if the entity is top-level
@@ -304,7 +304,7 @@ Page<EntityNameLookupRecord> listEntities(
304304
* @return the list of entities for which the predicate returns true
305305
*/
306306
@Nonnull
307-
<T> Page<T> listEntities(
307+
<T> Page<T> loadEntities(
308308
@Nonnull PolarisCallContext callCtx,
309309
long catalogId,
310310
long parentId,

‎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/AbstractTransactionalPersistence.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ public Page<EntityNameLookupRecord> listEntities(
372372
/** {@inheritDoc} */
373373
@Override
374374
@Nonnull
375-
public <T> Page<T> listEntities(
375+
public <T> Page<T> loadEntities(
376376
@Nonnull PolarisCallContext callCtx,
377377
long catalogId,
378378
long parentId,
@@ -384,7 +384,7 @@ public <T> Page<T> listEntities(
384384
return runInReadTransaction(
385385
callCtx,
386386
() ->
387-
this.listEntitiesInCurrentTxn(
387+
this.loadEntitiesInCurrentTxn(
388388
callCtx,
389389
catalogId,
390390
parentId,

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

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.Optional;
3131
import java.util.Set;
3232
import java.util.function.Function;
33+
import java.util.function.Predicate;
3334
import java.util.stream.Collectors;
3435
import org.apache.polaris.core.PolarisCallContext;
3536
import org.apache.polaris.core.entity.AsyncTaskType;
@@ -717,7 +718,6 @@ private void bootstrapPolarisService(
717718
entitySubType,
718719
pageToken);
719720

720-
// done
721721
return ListEntitiesResult.fromPage(resultPage);
722722
}
723723

@@ -738,6 +738,67 @@ private void bootstrapPolarisService(
738738
() -> listEntities(callCtx, ms, catalogPath, entityType, entitySubType, pageToken));
739739
}
740740

741+
/**
742+
* See {@link PolarisMetaStoreManager#loadEntities(PolarisCallContext, List, PolarisEntityType,
743+
* PolarisEntitySubType, Predicate, Function, PageToken)}
744+
*/
745+
private @Nonnull <T> Page<T> loadEntities(
746+
@Nonnull PolarisCallContext callCtx,
747+
@Nonnull TransactionalPersistence ms,
748+
@Nullable List<PolarisEntityCore> catalogPath,
749+
@Nonnull PolarisEntityType entityType,
750+
@Nonnull PolarisEntitySubType entitySubType,
751+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
752+
@Nonnull Function<PolarisBaseEntity, T> transformer,
753+
@Nonnull PageToken pageToken) {
754+
// first resolve again the catalogPath to that entity
755+
PolarisEntityResolver resolver = new PolarisEntityResolver(callCtx, ms, catalogPath);
756+
757+
// throw if we failed to resolve
758+
if (resolver.isFailure()) {
759+
throw new IllegalArgumentException("Failed to resolve catalogPath: " + catalogPath);
760+
}
761+
762+
// return list of active entities
763+
return ms.loadEntitiesInCurrentTxn(
764+
callCtx,
765+
resolver.getCatalogIdOrNull(),
766+
resolver.getParentId(),
767+
entityType,
768+
entitySubType,
769+
entityFilter,
770+
transformer,
771+
pageToken);
772+
}
773+
774+
/** {@inheritDoc} */
775+
@Override
776+
public @Nonnull <T> Page<T> loadEntities(
777+
@Nonnull PolarisCallContext callCtx,
778+
@Nullable List<PolarisEntityCore> catalogPath,
779+
@Nonnull PolarisEntityType entityType,
780+
@Nonnull PolarisEntitySubType entitySubType,
781+
@Nonnull Predicate<PolarisBaseEntity> entityFilter,
782+
@Nonnull Function<PolarisBaseEntity, T> transformer,
783+
@Nonnull PageToken pageToken) {
784+
// get meta store we should be using
785+
TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore());
786+
787+
// run operation in a read transaction
788+
return ms.runInReadTransaction(
789+
callCtx,
790+
() ->
791+
loadEntities(
792+
callCtx,
793+
ms,
794+
catalogPath,
795+
entityType,
796+
entitySubType,
797+
entityFilter,
798+
transformer,
799+
pageToken));
800+
}
801+
741802
/** {@link #createPrincipal(PolarisCallContext, PolarisBaseEntity)} */
742803
private @Nonnull CreatePrincipalResult createPrincipal(
743804
@Nonnull PolarisCallContext callCtx,
@@ -1373,7 +1434,7 @@ private void bootstrapPolarisService(
13731434

13741435
// get the list of catalog roles, at most 2
13751436
List<PolarisBaseEntity> catalogRoles =
1376-
ms.listEntitiesInCurrentTxn(
1437+
ms.loadEntitiesInCurrentTxn(
13771438
callCtx,
13781439
catalogId,
13791440
catalogId,
@@ -1948,7 +2009,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
19482009

19492010
// find all available tasks
19502011
Page<PolarisBaseEntity> availableTasks =
1951-
ms.listEntitiesInCurrentTxn(
2012+
ms.loadEntitiesInCurrentTxn(
19522013
callCtx,
19532014
PolarisEntityConstants.getRootEntityId(),
19542015
PolarisEntityConstants.getRootEntityId(),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ List<PolarisChangeTrackingVersions> lookupEntityVersionsInCurrentTxn(
214214
@Nonnull PolarisEntityType entityType,
215215
@Nonnull PolarisEntitySubType entitySubType,
216216
@Nonnull PageToken pageToken) {
217-
return listEntitiesInCurrentTxn(
217+
return loadEntitiesInCurrentTxn(
218218
callCtx,
219219
catalogId,
220220
parentId,
@@ -227,7 +227,7 @@ List<PolarisChangeTrackingVersions> lookupEntityVersionsInCurrentTxn(
227227

228228
/** See {@link org.apache.polaris.core.persistence.BasePersistence#listEntities} */
229229
@Nonnull
230-
<T> Page<T> listEntitiesInCurrentTxn(
230+
<T> Page<T> loadEntitiesInCurrentTxn(
231231
@Nonnull PolarisCallContext callCtx,
232232
long catalogId,
233233
long parentId,

0 commit comments

Comments
 (0)