Skip to content

Commit 25509f2

Browse files
dennishuodennishuo
authored
Core persistence refactor phase 1 - extract BasePersistence and BaseMetaStoreManager to isolate all "transaction" behaviors (#1070)
* Restructure persistence class hierarchy and remove vestigial interfaces Extract a basic abstract base class BaseMetaStoreManager which can hold shared logic between implementations of PolarisMetaStoreManager * Remove all "entitiesDropped" members; these were vestigial from trying to implement UNDROP but for now are entirely unused. We can reintroduce it with a better design against multiple backends when/if we want to implement UNDROP. * Extract BasePersistence interface as parent interface of PolarisMetaStoreSession; only leave the transaction-specific methods in PolarisMetaStoreSession * Push all evidence of the two-phase lookupEntityByName into only the transactional-style PolarisMetaStoreSession, so that BasePersistence properly exposes a lookupEntityByName method where impls that use secondary indexes can easily just lookup an entity by name instead of doing two lookups. * Turn PolarisMetaStoreSession into an abstract class and make lookupEntityActive protected-visibility; remove all callsites where PolarisMetaStoreManagerImpl calls it. Technically, while in the same package this doesn't prevent it from leaking, but we could reposition PolarisMetaStoreSession into a separate transaction-specific package to help protect it from leaking the lower-level abstractions. * Pushdown all calls to writeToEntities into PolarisMetaStoreSession, and add writeEntity method to BasePersistence, with a default impl in PolarisMetaStoreSession containing what was previously in PolarisMetaStoreManagerImpl. This now protects all writes in PolarisMetaStoreManagerImpl from dealing with the three-table implementation detail. Technically slightly changes the ordering of updates within a transaction for renameEntity, but is arguably a more correct ordering, and the ordering doesn't interleave reads anyways. * Add originalEntity to the writeEntity method to enable compare-and-swap behavior from the underlying BasePersistence. Pushdown all the deleteFromEntities* methods into PolarisMetaStoreSession and add deleteEntity to BasePersistence which encapsulates handling the separate slices. * Break out external-integration related methods from BasePersistence into a new IntegrationPersistence interface; these methods encapsulate certain type-specific behaviors that are indirectly tied to persistence entities, such as principal secrets, storage integrations, etc. * Improve javadoc comments, rename PolarisEntityActiveRecord to EntityNameLookupRecord, remove unused method * Rename PolarisMetaStoreSession to TransactionalPersistence and move into a new package "transactional". * Also move the PolarisTreeMap* classes into the transactional package * Move PolarisMetaStoreManagerImpl into the "transactional" package per PR suggestion
1 parent 09ee269 commit 25509f2

File tree

38 files changed

+1120
-1223
lines changed

38 files changed

+1120
-1223
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828
import org.apache.polaris.core.context.RealmContext;
2929
import org.apache.polaris.core.persistence.LocalPolarisMetaStoreManagerFactory;
3030
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
31-
import org.apache.polaris.core.persistence.PolarisMetaStoreSession;
3231
import org.apache.polaris.core.persistence.bootstrap.RootCredentialsSet;
32+
import org.apache.polaris.core.persistence.transactional.TransactionalPersistence;
3333
import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider;
3434

3535
/**
@@ -60,7 +60,7 @@ protected PolarisEclipseLinkStore createBackingStore(@Nonnull PolarisDiagnostics
6060
}
6161

6262
@Override
63-
protected PolarisMetaStoreSession createMetaStoreSession(
63+
protected TransactionalPersistence createMetaStoreSession(
6464
@Nonnull PolarisEclipseLinkStore store,
6565
@Nonnull RealmContext realmContext,
6666
@Nullable RootCredentialsSet rootCredentialsSet,

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

+13-37
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,20 @@
3939
import java.util.stream.Collectors;
4040
import org.apache.polaris.core.PolarisCallContext;
4141
import org.apache.polaris.core.context.RealmContext;
42+
import org.apache.polaris.core.entity.EntityNameLookupRecord;
4243
import org.apache.polaris.core.entity.PolarisBaseEntity;
4344
import org.apache.polaris.core.entity.PolarisChangeTrackingVersions;
4445
import org.apache.polaris.core.entity.PolarisEntitiesActiveKey;
45-
import org.apache.polaris.core.entity.PolarisEntityActiveRecord;
4646
import org.apache.polaris.core.entity.PolarisEntityCore;
4747
import org.apache.polaris.core.entity.PolarisEntityId;
4848
import org.apache.polaris.core.entity.PolarisEntityType;
4949
import org.apache.polaris.core.entity.PolarisGrantRecord;
5050
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
5151
import org.apache.polaris.core.exceptions.AlreadyExistsException;
52-
import org.apache.polaris.core.persistence.PolarisMetaStoreManagerImpl;
53-
import org.apache.polaris.core.persistence.PolarisMetaStoreSession;
52+
import org.apache.polaris.core.persistence.BaseMetaStoreManager;
5453
import org.apache.polaris.core.persistence.PrincipalSecretsGenerator;
5554
import org.apache.polaris.core.persistence.RetryOnConcurrencyException;
55+
import org.apache.polaris.core.persistence.transactional.TransactionalPersistence;
5656
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
5757
import org.apache.polaris.core.storage.PolarisStorageIntegration;
5858
import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider;
@@ -68,7 +68,7 @@
6868
* EclipseLink implementation of a Polaris metadata store supporting persisting and retrieving all
6969
* Polaris metadata from/to the configured database systems.
7070
*/
71-
public class PolarisEclipseLinkMetaStoreSessionImpl implements PolarisMetaStoreSession {
71+
public class PolarisEclipseLinkMetaStoreSessionImpl extends TransactionalPersistence {
7272
private static final Logger LOGGER =
7373
LoggerFactory.getLogger(PolarisEclipseLinkMetaStoreSessionImpl.class);
7474

@@ -271,14 +271,6 @@ public void writeToEntitiesActive(
271271
this.store.writeToEntitiesActive(localSession.get(), entity);
272272
}
273273

274-
/** {@inheritDoc} */
275-
@Override
276-
public void writeToEntitiesDropped(
277-
@Nonnull PolarisCallContext callCtx, @Nonnull PolarisBaseEntity entity) {
278-
// write it
279-
this.store.writeToEntitiesDropped(localSession.get(), entity);
280-
}
281-
282274
/** {@inheritDoc} */
283275
@Override
284276
public void writeToEntitiesChangeTracking(
@@ -312,14 +304,6 @@ public void deleteFromEntitiesActive(
312304
this.store.deleteFromEntitiesActive(localSession.get(), new PolarisEntitiesActiveKey(entity));
313305
}
314306

315-
/** {@inheritDoc} */
316-
@Override
317-
public void deleteFromEntitiesDropped(
318-
@Nonnull PolarisCallContext callCtx, @Nonnull PolarisBaseEntity entity) {
319-
// delete it
320-
this.store.deleteFromEntitiesDropped(localSession.get(), entity.getCatalogId(), entity.getId());
321-
}
322-
323307
/**
324308
* {@inheritDoc}
325309
*
@@ -371,14 +355,6 @@ public void deleteAll(@Nonnull PolarisCallContext callCtx) {
371355
.toList();
372356
}
373357

374-
/** {@inheritDoc} */
375-
@Override
376-
public int lookupEntityVersion(
377-
@Nonnull PolarisCallContext callCtx, long catalogId, long entityId) {
378-
ModelEntity model = this.store.lookupEntity(localSession.get(), catalogId, entityId);
379-
return model == null ? 0 : model.getEntityVersion();
380-
}
381-
382358
/** {@inheritDoc} */
383359
@Override
384360
public @Nonnull List<PolarisChangeTrackingVersions> lookupEntityVersions(
@@ -404,7 +380,7 @@ public int lookupEntityVersion(
404380
/** {@inheritDoc} */
405381
@Override
406382
@Nullable
407-
public PolarisEntityActiveRecord lookupEntityActive(
383+
public EntityNameLookupRecord lookupEntityActive(
408384
@Nonnull PolarisCallContext callCtx, @Nonnull PolarisEntitiesActiveKey entityActiveKey) {
409385
// lookup the active entity slice
410386
return ModelEntityActive.toEntityActive(
@@ -414,7 +390,7 @@ public PolarisEntityActiveRecord lookupEntityActive(
414390
/** {@inheritDoc} */
415391
@Override
416392
@Nonnull
417-
public List<PolarisEntityActiveRecord> lookupEntityActiveBatch(
393+
public List<EntityNameLookupRecord> lookupEntityActiveBatch(
418394
@Nonnull PolarisCallContext callCtx,
419395
@Nonnull List<PolarisEntitiesActiveKey> entityActiveKeys) {
420396
// now build a list to quickly verify that nothing has changed
@@ -425,31 +401,31 @@ public List<PolarisEntityActiveRecord> lookupEntityActiveBatch(
425401

426402
/** {@inheritDoc} */
427403
@Override
428-
public @Nonnull List<PolarisEntityActiveRecord> listActiveEntities(
404+
public @Nonnull List<EntityNameLookupRecord> listEntities(
429405
@Nonnull PolarisCallContext callCtx,
430406
long catalogId,
431407
long parentId,
432408
@Nonnull PolarisEntityType entityType) {
433-
return listActiveEntities(callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue());
409+
return listEntities(callCtx, catalogId, parentId, entityType, Predicates.alwaysTrue());
434410
}
435411

436412
@Override
437-
public @Nonnull List<PolarisEntityActiveRecord> listActiveEntities(
413+
public @Nonnull List<EntityNameLookupRecord> listEntities(
438414
@Nonnull PolarisCallContext callCtx,
439415
long catalogId,
440416
long parentId,
441417
@Nonnull PolarisEntityType entityType,
442418
@Nonnull Predicate<PolarisBaseEntity> entityFilter) {
443419
// full range scan under the parent for that type
444-
return listActiveEntities(
420+
return listEntities(
445421
callCtx,
446422
catalogId,
447423
parentId,
448424
entityType,
449425
Integer.MAX_VALUE,
450426
entityFilter,
451427
entity ->
452-
new PolarisEntityActiveRecord(
428+
new EntityNameLookupRecord(
453429
entity.getCatalogId(),
454430
entity.getId(),
455431
entity.getParentId(),
@@ -459,7 +435,7 @@ public List<PolarisEntityActiveRecord> lookupEntityActiveBatch(
459435
}
460436

461437
@Override
462-
public @Nonnull <T> List<T> listActiveEntities(
438+
public @Nonnull <T> List<T> listEntities(
463439
@Nonnull PolarisCallContext callCtx,
464440
long catalogId,
465441
long parentId,
@@ -674,7 +650,7 @@ PolarisStorageIntegration<T> createStorageIntegration(
674650
PolarisStorageIntegration<T> loadPolarisStorageIntegration(
675651
@Nonnull PolarisCallContext callCtx, @Nonnull PolarisBaseEntity entity) {
676652
PolarisStorageConfigurationInfo storageConfig =
677-
PolarisMetaStoreManagerImpl.readStorageConfiguration(callCtx, entity);
653+
BaseMetaStoreManager.extractStorageConfiguration(callCtx, entity);
678654
return storageIntegrationProvider.getStorageIntegrationForConfig(storageConfig);
679655
}
680656

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

+2-40
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727
import java.util.concurrent.atomic.AtomicBoolean;
2828
import java.util.stream.Collectors;
2929
import org.apache.polaris.core.PolarisDiagnostics;
30+
import org.apache.polaris.core.entity.EntityNameLookupRecord;
3031
import org.apache.polaris.core.entity.PolarisBaseEntity;
3132
import org.apache.polaris.core.entity.PolarisEntitiesActiveKey;
32-
import org.apache.polaris.core.entity.PolarisEntityActiveRecord;
3333
import org.apache.polaris.core.entity.PolarisEntityCore;
3434
import org.apache.polaris.core.entity.PolarisEntityId;
3535
import org.apache.polaris.core.entity.PolarisEntityType;
@@ -38,7 +38,6 @@
3838
import org.apache.polaris.jpa.models.ModelEntity;
3939
import org.apache.polaris.jpa.models.ModelEntityActive;
4040
import org.apache.polaris.jpa.models.ModelEntityChangeTracking;
41-
import org.apache.polaris.jpa.models.ModelEntityDropped;
4241
import org.apache.polaris.jpa.models.ModelGrantRecord;
4342
import org.apache.polaris.jpa.models.ModelPrincipalSecrets;
4443
import org.slf4j.Logger;
@@ -100,18 +99,7 @@ void writeToEntitiesActive(EntityManager session, PolarisBaseEntity entity) {
10099

101100
ModelEntityActive model = lookupEntityActive(session, new PolarisEntitiesActiveKey(entity));
102101
if (model == null) {
103-
session.persist(ModelEntityActive.fromEntityActive(new PolarisEntityActiveRecord(entity)));
104-
}
105-
}
106-
107-
void writeToEntitiesDropped(EntityManager session, PolarisBaseEntity entity) {
108-
diagnosticServices.check(session != null, "session_is_null");
109-
checkInitialized();
110-
111-
ModelEntityDropped entityDropped =
112-
lookupEntityDropped(session, entity.getCatalogId(), entity.getId());
113-
if (entityDropped == null) {
114-
session.persist(ModelEntityDropped.fromEntity(entity));
102+
session.persist(ModelEntityActive.fromEntityActive(new EntityNameLookupRecord(entity)));
115103
}
116104
}
117105

@@ -158,16 +146,6 @@ void deleteFromEntitiesActive(EntityManager session, PolarisEntitiesActiveKey ke
158146
session.remove(entity);
159147
}
160148

161-
void deleteFromEntitiesDropped(EntityManager session, long catalogId, long entityId) {
162-
diagnosticServices.check(session != null, "session_is_null");
163-
checkInitialized();
164-
165-
ModelEntityDropped entity = lookupEntityDropped(session, catalogId, entityId);
166-
diagnosticServices.check(entity != null, "dropped_entity_not_found");
167-
168-
session.remove(entity);
169-
}
170-
171149
void deleteFromEntitiesChangeTracking(EntityManager session, PolarisEntityCore entity) {
172150
diagnosticServices.check(session != null, "session_is_null");
173151
checkInitialized();
@@ -216,7 +194,6 @@ void deleteAll(EntityManager session) {
216194

217195
session.createQuery("DELETE from ModelEntity").executeUpdate();
218196
session.createQuery("DELETE from ModelEntityActive").executeUpdate();
219-
session.createQuery("DELETE from ModelEntityDropped").executeUpdate();
220197
session.createQuery("DELETE from ModelEntityChangeTracking").executeUpdate();
221198
session.createQuery("DELETE from ModelGrantRecord").executeUpdate();
222199
session.createQuery("DELETE from ModelPrincipalSecrets").executeUpdate();
@@ -319,21 +296,6 @@ List<ModelEntity> lookupFullEntitiesActive(
319296
return query.getResultList();
320297
}
321298

322-
ModelEntityDropped lookupEntityDropped(EntityManager session, long catalogId, long entityId) {
323-
diagnosticServices.check(session != null, "session_is_null");
324-
checkInitialized();
325-
326-
return session
327-
.createQuery(
328-
"SELECT m from ModelEntityDropped m where m.catalogId=:catalogId and m.id=:id",
329-
ModelEntityDropped.class)
330-
.setParameter("catalogId", catalogId)
331-
.setParameter("id", entityId)
332-
.getResultStream()
333-
.findFirst()
334-
.orElse(null);
335-
}
336-
337299
ModelEntityChangeTracking lookupEntityChangeTracking(
338300
EntityManager session, long catalogId, long entityId) {
339301
diagnosticServices.check(session != null, "session_is_null");

‎extension/persistence/eclipselink/src/test/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreManagerTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@
4141
import org.apache.polaris.core.PolarisDiagnostics;
4242
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
4343
import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest;
44-
import org.apache.polaris.core.persistence.PolarisMetaStoreManagerImpl;
4544
import org.apache.polaris.core.persistence.PolarisTestMetaStoreManager;
45+
import org.apache.polaris.core.persistence.transactional.PolarisMetaStoreManagerImpl;
4646
import org.apache.polaris.jpa.models.ModelPrincipalSecrets;
4747
import org.junit.jupiter.api.AfterAll;
4848
import org.junit.jupiter.api.Assertions;

‎extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelEntityActive.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import jakarta.persistence.Entity;
2222
import jakarta.persistence.Id;
2323
import jakarta.persistence.Table;
24-
import org.apache.polaris.core.entity.PolarisEntityActiveRecord;
24+
import org.apache.polaris.core.entity.EntityNameLookupRecord;
2525
import org.apache.polaris.core.entity.PolarisEntitySubType;
2626
import org.apache.polaris.core.entity.PolarisEntityType;
2727

@@ -128,7 +128,7 @@ public ModelEntityActive build() {
128128
}
129129
}
130130

131-
public static ModelEntityActive fromEntityActive(PolarisEntityActiveRecord record) {
131+
public static ModelEntityActive fromEntityActive(EntityNameLookupRecord record) {
132132
return ModelEntityActive.builder()
133133
.catalogId(record.getCatalogId())
134134
.id(record.getId())
@@ -139,12 +139,12 @@ public static ModelEntityActive fromEntityActive(PolarisEntityActiveRecord recor
139139
.build();
140140
}
141141

142-
public static PolarisEntityActiveRecord toEntityActive(ModelEntityActive model) {
142+
public static EntityNameLookupRecord toEntityActive(ModelEntityActive model) {
143143
if (model == null) {
144144
return null;
145145
}
146146

147-
return new PolarisEntityActiveRecord(
147+
return new EntityNameLookupRecord(
148148
model.catalogId, model.id, model.parentId, model.name, model.typeCode, model.subTypeCode);
149149
}
150150
}

0 commit comments

Comments
 (0)