This is an automated email from the ASF dual-hosted git repository. rlevas pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push: new 60a05a8 [AMBARI-25003] Starting JPA persistence service sometimes throws IllegalStateException 60a05a8 is described below commit 60a05a806fc3203cda35cc3a6be3a5c713d18be3 Author: Robert Levas <rle...@apache.org> AuthorDate: Thu Dec 6 13:30:11 2018 -0500 [AMBARI-25003] Starting JPA persistence service sometimes throws IllegalStateException --- .../persist/jpa/AmbariJpaPersistService.java | 22 ++++++++++++ .../AmbariServerConfigurationProvider.java | 14 ++++---- .../ambari/server/controller/AmbariServer.java | 2 +- .../service/AmbariLdapConfigurationProvider.java | 6 ++-- .../ambari/server/orm/GuiceJpaInitializer.java | 39 ++++++++-------------- .../jwt/JwtAuthenticationPropertiesProvider.java | 6 ++-- .../tproxy/AmbariTProxyConfigurationProvider.java | 6 ++-- .../ambari/server/upgrade/SchemaUpgradeHelper.java | 3 +- .../persist/jpa/AmbariJpaPersistServiceTest.java} | 28 ++++++++++------ .../AmbariServerConfigurationProviderTest.java | 16 +++++---- .../server/controller/KerberosHelperTest.java | 4 +-- 11 files changed, 82 insertions(+), 64 deletions(-) diff --git a/ambari-server/src/main/java/com/google/inject/persist/jpa/AmbariJpaPersistService.java b/ambari-server/src/main/java/com/google/inject/persist/jpa/AmbariJpaPersistService.java index 10a8af2..d01445f 100644 --- a/ambari-server/src/main/java/com/google/inject/persist/jpa/AmbariJpaPersistService.java +++ b/ambari-server/src/main/java/com/google/inject/persist/jpa/AmbariJpaPersistService.java @@ -18,6 +18,7 @@ package com.google.inject.persist.jpa; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import com.google.inject.Inject; @@ -26,10 +27,31 @@ import com.google.inject.Inject; */ public class AmbariJpaPersistService extends JpaPersistService { + private final AtomicBoolean jpaStarted = new AtomicBoolean(false); + @Inject public AmbariJpaPersistService(@Jpa String persistenceUnitName, @Jpa Map<?, ?> persistenceProperties) { super(persistenceUnitName, persistenceProperties); } + /** + * Starts the PersistService if it has not been previously started. + */ + @Override + public synchronized void start() { + if (!jpaStarted.get()) { + super.start(); + jpaStarted.set(true); + } + } + + /** + * Returns whether JPA has been started or not + * + * @return <code>true</code> if JPA has been started; <code>false</code> if JPA has not been started + */ + public boolean isStarted() { + return jpaStarted.get(); + } } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProvider.java index d4f31e8..c970bb6 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProvider.java @@ -26,7 +26,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.apache.ambari.server.events.AmbariConfigurationChangedEvent; import org.apache.ambari.server.events.JpaInitializedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; -import org.apache.ambari.server.orm.GuiceJpaInitializer; import org.apache.ambari.server.orm.dao.AmbariConfigurationDAO; import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import org.slf4j.Logger; @@ -35,6 +34,7 @@ import org.slf4j.LoggerFactory; import com.google.common.eventbus.Subscribe; import com.google.inject.Inject; import com.google.inject.Provider; +import com.google.inject.persist.jpa.AmbariJpaPersistService; /** * AmbariServerConfigurationProvider is an abstract class to be extended by Ambari server configuration @@ -54,13 +54,13 @@ public abstract class AmbariServerConfigurationProvider<T extends AmbariServerCo @Inject private Provider<AmbariConfigurationDAO> ambariConfigurationDAOProvider; - private final AtomicBoolean jpaInitialized = new AtomicBoolean(false); + private final AtomicBoolean jpaStarted = new AtomicBoolean(false); private final AmbariServerConfigurationCategory configurationCategory; private T instance = null; - protected AmbariServerConfigurationProvider(AmbariServerConfigurationCategory configurationCategory, AmbariEventPublisher publisher, GuiceJpaInitializer guiceJpaInitializer) { + protected AmbariServerConfigurationProvider(AmbariServerConfigurationCategory configurationCategory, AmbariEventPublisher publisher, AmbariJpaPersistService persistService) { this.configurationCategory = configurationCategory; if (publisher != null) { @@ -68,8 +68,8 @@ public abstract class AmbariServerConfigurationProvider<T extends AmbariServerCo LOGGER.info("Registered {} in event publisher", this.getClass().getName()); } - if (guiceJpaInitializer != null) { - jpaInitialized.set(guiceJpaInitializer.isInitialized()); + if (persistService != null) { + jpaStarted.set(persistService.isStarted()); } } @@ -96,7 +96,7 @@ public abstract class AmbariServerConfigurationProvider<T extends AmbariServerCo @Subscribe public void jpaInitialized(JpaInitializedEvent event) { LOGGER.info("JPA initialized event received: {}", event); - jpaInitialized.getAndSet(true); + jpaStarted.set(true); instance = loadInstance(); } @@ -118,7 +118,7 @@ public abstract class AmbariServerConfigurationProvider<T extends AmbariServerCo * @return the loaded instance data */ private T loadInstance() { - if (jpaInitialized.get()) { + if (jpaStarted.get()) { LOGGER.info("Loading {} configuration data", configurationCategory.getCategoryName()); return loadInstance(ambariConfigurationDAOProvider.get().findByCategory(configurationCategory.getCategoryName())); } else { diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java index 6f98744..40fe567 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java @@ -1096,7 +1096,7 @@ public class AmbariServer { // Start and Initialize JPA GuiceJpaInitializer jpaInitializer = injector.getInstance(GuiceJpaInitializer.class); - jpaInitializer.setInitialized(injector.getInstance(AmbariEventPublisher.class)); // This must be called to alert Ambari that JPA is initialized. + jpaInitializer.setInitialized(); // This must be called to alert Ambari that JPA is initialized. DatabaseConsistencyCheckHelper.checkDBVersionCompatible(); diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapConfigurationProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapConfigurationProvider.java index 1a977e1..8f7a111 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapConfigurationProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/service/AmbariLdapConfigurationProvider.java @@ -24,11 +24,11 @@ import org.apache.ambari.server.configuration.AmbariServerConfigurationCategory; import org.apache.ambari.server.configuration.AmbariServerConfigurationProvider; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.ldap.domain.AmbariLdapConfiguration; -import org.apache.ambari.server.orm.GuiceJpaInitializer; import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import com.google.inject.Inject; import com.google.inject.Singleton; +import com.google.inject.persist.jpa.AmbariJpaPersistService; /** * Provider implementation for LDAP configurations. @@ -46,8 +46,8 @@ import com.google.inject.Singleton; public class AmbariLdapConfigurationProvider extends AmbariServerConfigurationProvider<AmbariLdapConfiguration> { @Inject - public AmbariLdapConfigurationProvider(AmbariEventPublisher publisher, GuiceJpaInitializer guiceJpaInitializer) { - super(AmbariServerConfigurationCategory.LDAP_CONFIGURATION, publisher, guiceJpaInitializer); + public AmbariLdapConfigurationProvider(AmbariEventPublisher publisher, AmbariJpaPersistService persistService) { + super(AmbariServerConfigurationCategory.LDAP_CONFIGURATION, publisher, persistService); } @Override diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/GuiceJpaInitializer.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/GuiceJpaInitializer.java index c0049e5..c049c72 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/GuiceJpaInitializer.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/GuiceJpaInitializer.java @@ -18,8 +18,6 @@ package org.apache.ambari.server.orm; -import java.util.concurrent.atomic.AtomicBoolean; - import org.apache.ambari.server.events.JpaInitializedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; @@ -33,16 +31,20 @@ import com.google.inject.persist.PersistService; @Singleton public class GuiceJpaInitializer { - private final AtomicBoolean jpaInitialized = new AtomicBoolean(false); + private final AmbariEventPublisher publisher; /** * GuiceJpaInitializer constructor. + * <p> + * Starts the JPA service and holds on to an {@link AmbariEventPublisher} for future use. + * + * @param service the persist service + * @param publisher the Ambari event publisher */ @Inject - public GuiceJpaInitializer(PersistService persistService) { - if (persistService != null) { - persistService.start(); - } + public GuiceJpaInitializer(PersistService service, AmbariEventPublisher publisher) { + this.publisher = publisher; + service.start(); } /** @@ -51,26 +53,11 @@ public class GuiceJpaInitializer { * This means that the schema for the underlying database matches the JPA entity objects expectations * and the PersistService has been started. * <p> - * If an {@link AmbariEventPublisher} is supplied, a {@link JpaInitializedEvent} is published so - * that subscribers can perform database-related tasks when the infrastructure is ready. - * - * @param publisher an {@link AmbariEventPublisher} to use for publishing the event, optional + * A {@link JpaInitializedEvent} is published so that subscribers can perform database-related tasks + * when the infrastructure is ready. */ - public void setInitialized(AmbariEventPublisher publisher) { - jpaInitialized.set(true); - - if (publisher != null) { - publisher.publish(new JpaInitializedEvent()); - } + public void setInitialized() { + publisher.publish(new JpaInitializedEvent()); } - /** - * Returns whether JPA has been initialized or not - * - * @return <code>true</code> if JPA has been initialized; <code>false</code> if JPA has not been initialized - * @see #setInitialized(AmbariEventPublisher) - */ - public boolean isInitialized() { - return jpaInitialized.get(); - } } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/jwt/JwtAuthenticationPropertiesProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/jwt/JwtAuthenticationPropertiesProvider.java index e357379..6c45395 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/jwt/JwtAuthenticationPropertiesProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/jwt/JwtAuthenticationPropertiesProvider.java @@ -24,11 +24,11 @@ import java.util.Collection; import org.apache.ambari.server.configuration.AmbariServerConfigurationProvider; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; -import org.apache.ambari.server.orm.GuiceJpaInitializer; import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import com.google.inject.Inject; import com.google.inject.Singleton; +import com.google.inject.persist.jpa.AmbariJpaPersistService; /** * JwtAuthenticationPropertiesProvider manages a {@link JwtAuthenticationProperties} instance by @@ -41,8 +41,8 @@ import com.google.inject.Singleton; public class JwtAuthenticationPropertiesProvider extends AmbariServerConfigurationProvider<JwtAuthenticationProperties> { @Inject - public JwtAuthenticationPropertiesProvider(AmbariEventPublisher ambariEventPublisher, GuiceJpaInitializer guiceJpaInitializer) { - super(SSO_CONFIGURATION, ambariEventPublisher, guiceJpaInitializer); + public JwtAuthenticationPropertiesProvider(AmbariEventPublisher ambariEventPublisher, AmbariJpaPersistService ambariJpaPersistService) { + super(SSO_CONFIGURATION, ambariEventPublisher, ambariJpaPersistService); } /** diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/tproxy/AmbariTProxyConfigurationProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/tproxy/AmbariTProxyConfigurationProvider.java index 0bc413a..7f9172c 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/tproxy/AmbariTProxyConfigurationProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authentication/tproxy/AmbariTProxyConfigurationProvider.java @@ -23,11 +23,11 @@ import java.util.Collection; import org.apache.ambari.server.configuration.AmbariServerConfigurationCategory; import org.apache.ambari.server.configuration.AmbariServerConfigurationProvider; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; -import org.apache.ambari.server.orm.GuiceJpaInitializer; import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import com.google.inject.Inject; import com.google.inject.Singleton; +import com.google.inject.persist.jpa.AmbariJpaPersistService; /** * Provider implementation for {@link AmbariTProxyConfiguration} objects. @@ -45,8 +45,8 @@ import com.google.inject.Singleton; public class AmbariTProxyConfigurationProvider extends AmbariServerConfigurationProvider<AmbariTProxyConfiguration> { @Inject - public AmbariTProxyConfigurationProvider(AmbariEventPublisher ambariEventPublisher, GuiceJpaInitializer guiceJpaInitializer) { - super(AmbariServerConfigurationCategory.TPROXY_CONFIGURATION, ambariEventPublisher, guiceJpaInitializer); + public AmbariTProxyConfigurationProvider(AmbariEventPublisher ambariEventPublisher, AmbariJpaPersistService persistService) { + super(AmbariServerConfigurationCategory.TPROXY_CONFIGURATION, ambariEventPublisher, persistService); } /** diff --git a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java index 1aa0ff7..7563975 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java @@ -36,7 +36,6 @@ import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.audit.AuditLoggerModule; import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.controller.ControllerModule; -import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.ldap.LdapModule; import org.apache.ambari.server.orm.DBAccessor; import org.apache.ambari.server.orm.GuiceJpaInitializer; @@ -453,7 +452,7 @@ public class SchemaUpgradeHelper { // The DDL is expected to be updated, now send the JPA initialized event so Entity // implementations can be created. - jpaInitializer.setInitialized(injector.getInstance(AmbariEventPublisher.class)); + jpaInitializer.setInitialized(); schemaUpgradeHelper.executePreDMLUpdates(upgradeCatalogs); diff --git a/ambari-server/src/main/java/com/google/inject/persist/jpa/AmbariJpaPersistService.java b/ambari-server/src/test/java/com/google/inject/persist/jpa/AmbariJpaPersistServiceTest.java similarity index 62% copy from ambari-server/src/main/java/com/google/inject/persist/jpa/AmbariJpaPersistService.java copy to ambari-server/src/test/java/com/google/inject/persist/jpa/AmbariJpaPersistServiceTest.java index 10a8af2..f5c0e28 100644 --- a/ambari-server/src/main/java/com/google/inject/persist/jpa/AmbariJpaPersistService.java +++ b/ambari-server/src/test/java/com/google/inject/persist/jpa/AmbariJpaPersistServiceTest.java @@ -15,21 +15,27 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package com.google.inject.persist.jpa; -import java.util.Map; +import org.apache.ambari.server.orm.InMemoryDefaultTestModule; +import org.junit.Test; -import com.google.inject.Inject; +import com.google.inject.Guice; +import com.google.inject.Injector; -/** - * Override non-public class limitations as we need non-interface method - */ -public class AmbariJpaPersistService extends JpaPersistService { +public class AmbariJpaPersistServiceTest { - @Inject - public AmbariJpaPersistService(@Jpa String persistenceUnitName, @Jpa Map<?, ?> persistenceProperties) { - super(persistenceUnitName, persistenceProperties); - } + @Test + public void start() { + Injector injector = Guice.createInjector( + new InMemoryDefaultTestModule() + ); + AmbariJpaPersistService persistService = injector.getInstance(AmbariJpaPersistService.class); -} + persistService.start(); + // This should not fail... + persistService.start(); + } +} \ No newline at end of file diff --git a/ambari-server/src/test/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProviderTest.java index af5a67c..503921d 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/configuration/AmbariServerConfigurationProviderTest.java @@ -33,7 +33,6 @@ import javax.persistence.EntityManager; import org.apache.ambari.server.events.AmbariConfigurationChangedEvent; import org.apache.ambari.server.events.JpaInitializedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; -import org.apache.ambari.server.orm.GuiceJpaInitializer; import org.apache.ambari.server.orm.dao.AmbariConfigurationDAO; import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import org.apache.ambari.server.state.stack.OsFamily; @@ -44,6 +43,7 @@ import org.junit.Test; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; +import com.google.inject.persist.jpa.AmbariJpaPersistService; public class AmbariServerConfigurationProviderTest extends EasyMockSupport { @@ -60,11 +60,11 @@ public class AmbariServerConfigurationProviderTest extends EasyMockSupport { AmbariServerConfiguration filledTestConfiguration2 = createMock(AmbariServerConfiguration.class); AmbariEventPublisher publisher = injector.getInstance(AmbariEventPublisher.class); - GuiceJpaInitializer jpaInitializer = injector.getInstance(GuiceJpaInitializer.class); + AmbariJpaPersistService persistService = injector.getInstance(AmbariJpaPersistService.class); AmbariServerConfigurationProvider provider = createMockBuilder(AmbariServerConfigurationProvider.class) .addMockedMethod("loadInstance", Collection.class) - .withConstructor(TEST_CONFIGURATION, publisher, jpaInitializer) + .withConstructor(TEST_CONFIGURATION, publisher, persistService) .createMock(); expect(provider.loadInstance(Collections.emptyList())).andReturn(emptyTestConfiguration).once(); @@ -106,10 +106,10 @@ public class AmbariServerConfigurationProviderTest extends EasyMockSupport { Injector injector = getInjector(); AmbariEventPublisher publisher = injector.getInstance(AmbariEventPublisher.class); - GuiceJpaInitializer jpaInitializer = injector.getInstance(GuiceJpaInitializer.class); + AmbariJpaPersistService persistService = injector.getInstance(AmbariJpaPersistService.class); AmbariServerConfigurationProvider provider = createMockBuilder(AmbariServerConfigurationProvider.class) - .withConstructor(TEST_CONFIGURATION, publisher, jpaInitializer) + .withConstructor(TEST_CONFIGURATION, publisher, persistService) .createMock(); replayAll(); @@ -156,9 +156,13 @@ public class AmbariServerConfigurationProviderTest extends EasyMockSupport { @Override protected void configure() { + AmbariJpaPersistService persistService = createMockBuilder(AmbariJpaPersistService.class) + .withConstructor("test", Collections.emptyMap()) + .createMock(); + bind(OsFamily.class).toInstance(createNiceMock(OsFamily.class)); - bind(GuiceJpaInitializer.class).toInstance(new GuiceJpaInitializer(null)); bind(EntityManager.class).toInstance(createNiceMock(EntityManager.class)); + bind(AmbariJpaPersistService.class).toInstance(persistService); bind(AmbariConfigurationDAO.class).toInstance(createNiceMock(AmbariConfigurationDAO.class)); } }); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java index 25c7b9e..76b2e59 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java @@ -158,7 +158,7 @@ import org.springframework.security.crypto.password.StandardPasswordEncoder; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; -import com.google.inject.persist.PersistService; +import com.google.inject.persist.jpa.AmbariJpaPersistService; @SuppressWarnings("unchecked") @@ -243,7 +243,7 @@ public class KerberosHelperTest extends EasyMockSupport { PartialNiceMockBinder.newBuilder().addActionDBAccessorConfigsBindings().addFactoriesInstallBinding() .addPasswordEncryptorBindings().build().configure(binder()); - bind(PersistService.class).toInstance(createNiceMock(PersistService.class)); + bind(AmbariJpaPersistService.class).toInstance(createNiceMock(AmbariJpaPersistService.class)); bind(ActionDBAccessor.class).to(ActionDBAccessorImpl.class); bind(ExecutionScheduler.class).to(ExecutionSchedulerImpl.class); bind(AbstractRootServiceResponseFactory.class).to(RootServiceResponseFactory.class);