From a342f9b22d7c93e7639fabf077f355270017cad6 Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Wed, 12 Mar 2025 18:18:09 -0400 Subject: [PATCH 1/5] Isolate Persistence objects in different threads. * Add `copyOf()` to `BasePersistence` * Call it in `PolarisCallContext.copyOf()` This change is needed for the upcoming NoSQL persistence implementation. Co-authored-by: Robert Stupp --- .../org/apache/polaris/core/PolarisCallContext.java | 2 +- .../polaris/core/persistence/BasePersistence.java | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java b/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java index 510cd0f6cd..91ee5d10e5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java @@ -61,7 +61,7 @@ public PolarisCallContext( public static PolarisCallContext copyOf(PolarisCallContext original) { return new PolarisCallContext( - original.getMetaStore(), + original.getMetaStore().copyOf(), original.getDiagServices(), original.getConfigurationStore(), original.getClock()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index dccb546848..a18ba91b2d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -392,4 +392,15 @@ boolean hasChildren( @Nullable PolarisEntityType optionalEntityType, long catalogId, long parentId); + + /** + * Creates a fresh copy of this persistence implementation that is able to function independently + * of {@code this object}. The {@code copyOf} method is intended to be used to isolate state of + * {@link BasePersistence} instances in different thread. + * + *

If a particular implementation is thread-safe, it may return {@code this} from this method. + */ + default BasePersistence copyOf() { + return this; + } } From 6c515a1d40995624f7822a0e72988cf62e47c5d3 Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Thu, 13 Mar 2025 15:34:13 -0400 Subject: [PATCH 2/5] Implement copyOf: * Trivial impl in PolarisTreeMapMetaStoreSessionImpl * Copy without re-init in PolarisEclipseLinkMetaStoreSessionImpl --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 27 +++++++++++++++---- .../core/persistence/BasePersistence.java | 8 +++--- .../PolarisTreeMapMetaStoreSessionImpl.java | 7 +++++ 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 7005d5cfea..be57f054c0 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -50,6 +50,7 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.exceptions.AlreadyExistsException; import org.apache.polaris.core.persistence.BaseMetaStoreManager; +import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; import org.apache.polaris.core.persistence.transactional.AbstractTransactionalPersistence; @@ -98,26 +99,42 @@ public PolarisEclipseLinkMetaStoreSessionImpl( @Nullable String confFile, @Nullable String persistenceUnitName, @Nonnull PrincipalSecretsGenerator secretsGenerator) { + this( + createEntityManagerFactory(realmContext, confFile, persistenceUnitName), + store, + storageIntegrationProvider, + secretsGenerator); LOGGER.debug( "Creating EclipseLink Meta Store Session for realm {}", realmContext.getRealmIdentifier()); - emf = createEntityManagerFactory(realmContext, confFile, persistenceUnitName); - - // init store - this.store = store; try (EntityManager session = emf.createEntityManager()) { this.store.initialize(session); } + } + + private PolarisEclipseLinkMetaStoreSessionImpl( + EntityManagerFactory emf, + PolarisEclipseLinkStore store, + PolarisStorageIntegrationProvider storageIntegrationProvider, + PrincipalSecretsGenerator secretsGenerator) { + this.emf = emf; + this.store = store; this.storageIntegrationProvider = storageIntegrationProvider; this.secretsGenerator = secretsGenerator; } + @Override + public BasePersistence copyOf() { + return new PolarisEclipseLinkMetaStoreSessionImpl( + emf, store, storageIntegrationProvider, secretsGenerator); + } + /** * Create EntityManagerFactory. * *

The EntityManagerFactory creation is expensive, so we are caching and reusing it for each * realm. */ - private EntityManagerFactory createEntityManagerFactory( + private static EntityManagerFactory createEntityManagerFactory( @Nonnull RealmContext realmContext, @Nullable String confFile, @Nullable String persistenceUnitName) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index a18ba91b2d..fb4d4b010f 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -395,12 +395,10 @@ boolean hasChildren( /** * Creates a fresh copy of this persistence implementation that is able to function independently - * of {@code this object}. The {@code copyOf} method is intended to be used to isolate state of - * {@link BasePersistence} instances in different thread. + * of {@code this} object. The {@code copyOf} method is intended to be used to isolate state of + * {@link BasePersistence} instances in different threads. * *

If a particular implementation is thread-safe, it may return {@code this} from this method. */ - default BasePersistence copyOf() { - return this; - } + BasePersistence copyOf(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java index 521c5dfbba..4047da924d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java @@ -37,6 +37,7 @@ import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.BaseMetaStoreManager; +import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -60,6 +61,12 @@ public PolarisTreeMapMetaStoreSessionImpl( this.secretsGenerator = secretsGenerator; } + @Override + public BasePersistence copyOf() { + return new PolarisTreeMapMetaStoreSessionImpl( + store, storageIntegrationProvider, secretsGenerator); + } + /** {@inheritDoc} */ @Override public T runInTransaction( From ba9ca8fc007e3308a34614cd69c7c748cf576757 Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Fri, 14 Mar 2025 17:37:36 -0400 Subject: [PATCH 3/5] review: remove unnecessary debug log --- .../eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index be57f054c0..938ff78a99 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -104,8 +104,7 @@ public PolarisEclipseLinkMetaStoreSessionImpl( store, storageIntegrationProvider, secretsGenerator); - LOGGER.debug( - "Creating EclipseLink Meta Store Session for realm {}", realmContext.getRealmIdentifier()); + try (EntityManager session = emf.createEntityManager()) { this.store.initialize(session); } From a38990d81e540c739e06f44a047b14db4ae8057a Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Fri, 14 Mar 2025 18:19:41 -0400 Subject: [PATCH 4/5] review: revert copyOf impl --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 28 ++++--------------- .../core/persistence/BasePersistence.java | 8 ++++-- .../PolarisTreeMapMetaStoreSessionImpl.java | 7 ----- 3 files changed, 11 insertions(+), 32 deletions(-) diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 938ff78a99..7005d5cfea 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -50,7 +50,6 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.exceptions.AlreadyExistsException; import org.apache.polaris.core.persistence.BaseMetaStoreManager; -import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.persistence.RetryOnConcurrencyException; import org.apache.polaris.core.persistence.transactional.AbstractTransactionalPersistence; @@ -99,41 +98,26 @@ public PolarisEclipseLinkMetaStoreSessionImpl( @Nullable String confFile, @Nullable String persistenceUnitName, @Nonnull PrincipalSecretsGenerator secretsGenerator) { - this( - createEntityManagerFactory(realmContext, confFile, persistenceUnitName), - store, - storageIntegrationProvider, - secretsGenerator); + LOGGER.debug( + "Creating EclipseLink Meta Store Session for realm {}", realmContext.getRealmIdentifier()); + emf = createEntityManagerFactory(realmContext, confFile, persistenceUnitName); + // init store + this.store = store; try (EntityManager session = emf.createEntityManager()) { this.store.initialize(session); } - } - - private PolarisEclipseLinkMetaStoreSessionImpl( - EntityManagerFactory emf, - PolarisEclipseLinkStore store, - PolarisStorageIntegrationProvider storageIntegrationProvider, - PrincipalSecretsGenerator secretsGenerator) { - this.emf = emf; - this.store = store; this.storageIntegrationProvider = storageIntegrationProvider; this.secretsGenerator = secretsGenerator; } - @Override - public BasePersistence copyOf() { - return new PolarisEclipseLinkMetaStoreSessionImpl( - emf, store, storageIntegrationProvider, secretsGenerator); - } - /** * Create EntityManagerFactory. * *

The EntityManagerFactory creation is expensive, so we are caching and reusing it for each * realm. */ - private static EntityManagerFactory createEntityManagerFactory( + private EntityManagerFactory createEntityManagerFactory( @Nonnull RealmContext realmContext, @Nullable String confFile, @Nullable String persistenceUnitName) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index fb4d4b010f..a18ba91b2d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -395,10 +395,12 @@ boolean hasChildren( /** * Creates a fresh copy of this persistence implementation that is able to function independently - * of {@code this} object. The {@code copyOf} method is intended to be used to isolate state of - * {@link BasePersistence} instances in different threads. + * of {@code this object}. The {@code copyOf} method is intended to be used to isolate state of + * {@link BasePersistence} instances in different thread. * *

If a particular implementation is thread-safe, it may return {@code this} from this method. */ - BasePersistence copyOf(); + default BasePersistence copyOf() { + return this; + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java index 4047da924d..521c5dfbba 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/PolarisTreeMapMetaStoreSessionImpl.java @@ -37,7 +37,6 @@ import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.BaseMetaStoreManager; -import org.apache.polaris.core.persistence.BasePersistence; import org.apache.polaris.core.persistence.PrincipalSecretsGenerator; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -61,12 +60,6 @@ public PolarisTreeMapMetaStoreSessionImpl( this.secretsGenerator = secretsGenerator; } - @Override - public BasePersistence copyOf() { - return new PolarisTreeMapMetaStoreSessionImpl( - store, storageIntegrationProvider, secretsGenerator); - } - /** {@inheritDoc} */ @Override public T runInTransaction( From 48b837c40d4220312e94009f27f8f490c7ac0485 Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Fri, 14 Mar 2025 18:24:40 -0400 Subject: [PATCH 5/5] review: rename to detach() --- .../org/apache/polaris/core/PolarisCallContext.java | 2 +- .../polaris/core/persistence/BasePersistence.java | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java b/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java index 91ee5d10e5..c7033a50d1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java @@ -61,7 +61,7 @@ public PolarisCallContext( public static PolarisCallContext copyOf(PolarisCallContext original) { return new PolarisCallContext( - original.getMetaStore().copyOf(), + original.getMetaStore().detach(), original.getDiagServices(), original.getConfigurationStore(), original.getClock()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java index a18ba91b2d..88e5143cf8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java @@ -394,13 +394,13 @@ boolean hasChildren( long parentId); /** - * Creates a fresh copy of this persistence implementation that is able to function independently - * of {@code this object}. The {@code copyOf} method is intended to be used to isolate state of - * {@link BasePersistence} instances in different thread. - * - *

If a particular implementation is thread-safe, it may return {@code this} from this method. + * Performs operations necessary to isolate the state of {@code this} {@link BasePersistence} + * instance from the state of the returned instance as far as multithreaded usage is concerned. If + * the implementation has state that is not supposed to be accessed or modified by multiple + * threads, it may return a copy from this method. If the implementation is thread-safe, it may + * return {@code this}. */ - default BasePersistence copyOf() { + default BasePersistence detach() { return this; } }