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 <[email protected]>
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);
