mirror of
https://github.com/google/nomulus.git
synced 2025-07-09 20:53:26 +02:00
Lazily instantiate jpaTm (#491)
* Lazily load jpaTm This fixes an exception we were seeing in production where commands that only implemented CommandWithRemoteApi (and that had nothing to do with Cloud SQL) were nevertheless trying to initialize a JPA TM instance, and then failing. That stacktrace looked like: com.google.api.client.http.HttpResponseException: 400 Bad Request { "error": "invalid_grant", "error_description": "Bad Request" } at com.google.api.client.http.HttpRequest.execute(HttpRequest.java:1113) at com.google.auth.oauth2.UserCredentials.refreshAccessToken(UserCredentials.java:193) at com.google.auth.oauth2.OAuth2Credentials.refresh(OAuth2Credentials.java:165) at com.google.auth.oauth2.OAuth2Credentials.getRequestMetadata(OAuth2Credentials.java:151) at com.google.auth.http.HttpCredentialsAdapter.initialize(HttpCredentialsAdapter.java:96) at com.google.api.client.http.HttpRequestFactory.buildRequest(HttpRequestFactory.java:88) at com.google.api.client.googleapis.services.AbstractGoogleClientRequest.buildHttpRequest(AbstractGoogleClientRequest.java:423) at com.google.api.client.googleapis.services.AbstractGoogleClientRequest.executeUnparsed(AbstractGoogleClientRequest.java:542) at com.google.api.client.googleapis.services.AbstractGoogleClientRequest.executeUnparsed(AbstractGoogleClientRequest.java:475) at com.google.api.client.googleapis.services.AbstractGoogleClientRequest.execute(AbstractGoogleClientRequest.java:592) at google.registry.keyring.kms.KmsConnectionImpl.attemptDecrypt(KmsConnectionImpl.java:163) at google.registry.keyring.kms.KmsConnectionImpl.lambda$decrypt$0(KmsConnectionImpl.java:148) at google.registry.util.Retrier.callWithRetry(Retrier.java:153) at google.registry.util.Retrier.callWithRetry(Retrier.java:130) at google.registry.util.Retrier.callWithRetry(Retrier.java:95) at google.registry.keyring.kms.KmsConnectionImpl.decrypt(KmsConnectionImpl.java:147) at google.registry.keyring.kms.KmsKeyring.getDecryptedData(KmsKeyring.java:209) at google.registry.keyring.kms.KmsKeyring.getString(KmsKeyring.java:178) at google.registry.keyring.kms.KmsKeyring.getToolsCloudSqlPassword(KmsKeyring.java:100) at google.registry.persistence.PersistenceModule.providesNomulusToolJpaTm(PersistenceModule.java:124) at google.registry.persistence.PersistenceModule_ProvidesNomulusToolJpaTmFactory.proxyProvidesNomulusToolJpaTm(PersistenceModule_ProvidesNomulusToolJpaTmFactory.java:61) at google.registry.persistence.PersistenceModule_ProvidesNomulusToolJpaTmFactory.get(PersistenceModule_ProvidesNomulusToolJpaTmFactory.java:39) at google.registry.persistence.PersistenceModule_ProvidesNomulusToolJpaTmFactory.get(PersistenceModule_ProvidesNomulusToolJpaTmFactory.java:12) at dagger.internal.DoubleCheck.get(DoubleCheck.java:47) at google.registry.persistence.DaggerPersistenceComponent.nomulusToolJpaTransactionManager(DaggerPersistenceComponent.java:168) at google.registry.persistence.transaction.TransactionManagerFactory.createJpaTransactionManager(TransactionManagerFactory.java:38) at google.registry.persistence.transaction.TransactionManagerFactory.<clinit>(TransactionManagerFactory.java:29) at google.registry.model.registry.Registries.lambda$createFreshCache$2(Registries.java:60) at com.google.common.base.Suppliers$ExpiringMemoizingSupplier.get(Suppliers.java:243) at google.registry.model.registry.Registries.getTlds(Registries.java:85) at google.registry.model.registry.Registries.assertTldsExist(Registries.java:112) at google.registry.tools.CountDomainsCommand.run(CountDomainsCommand.java:41) at google.registry.tools.RegistryCli.runCommand(RegistryCli.java:243) at google.registry.tools.RegistryCli.run(RegistryCli.java:168) at google.registry.tools.RegistryTool.main(RegistryTool.java:127) The TL;DR is that RegistryCli was over-eagerly creating the jpaTm, because there's no reason the Registries cache (which is Datastore-only) should ever need it, but because this cache is using the Datastore transaction manager, the other one was being created too.
This commit is contained in:
parent
bbacdb9704
commit
7bb69e50c5
3 changed files with 18 additions and 6 deletions
|
@ -17,16 +17,23 @@ package google.registry.persistence.transaction;
|
||||||
import com.google.appengine.api.utils.SystemProperty;
|
import com.google.appengine.api.utils.SystemProperty;
|
||||||
import com.google.appengine.api.utils.SystemProperty.Environment.Value;
|
import com.google.appengine.api.utils.SystemProperty.Environment.Value;
|
||||||
import com.google.common.annotations.VisibleForTesting;
|
import com.google.common.annotations.VisibleForTesting;
|
||||||
|
import com.google.common.base.Suppliers;
|
||||||
import google.registry.model.ofy.DatastoreTransactionManager;
|
import google.registry.model.ofy.DatastoreTransactionManager;
|
||||||
import google.registry.persistence.DaggerPersistenceComponent;
|
import google.registry.persistence.DaggerPersistenceComponent;
|
||||||
import google.registry.tools.RegistryToolEnvironment;
|
import google.registry.tools.RegistryToolEnvironment;
|
||||||
|
import google.registry.util.NonFinalForTesting;
|
||||||
|
import java.util.function.Supplier;
|
||||||
|
|
||||||
/** Factory class to create {@link TransactionManager} instance. */
|
/** Factory class to create {@link TransactionManager} instance. */
|
||||||
// TODO: Rename this to PersistenceFactory and move to persistence package.
|
// TODO: Rename this to PersistenceFactory and move to persistence package.
|
||||||
public class TransactionManagerFactory {
|
public class TransactionManagerFactory {
|
||||||
|
|
||||||
private static final TransactionManager TM = createTransactionManager();
|
private static final TransactionManager TM = createTransactionManager();
|
||||||
@VisibleForTesting static JpaTransactionManager jpaTm = createJpaTransactionManager();
|
|
||||||
|
/** Supplier for jpaTm so that it is initialized only once, upon first usage. */
|
||||||
|
@NonFinalForTesting
|
||||||
|
private static Supplier<JpaTransactionManager> jpaTm =
|
||||||
|
Suppliers.memoize(TransactionManagerFactory::createJpaTransactionManager);
|
||||||
|
|
||||||
private TransactionManagerFactory() {}
|
private TransactionManagerFactory() {}
|
||||||
|
|
||||||
|
@ -68,6 +75,11 @@ public class TransactionManagerFactory {
|
||||||
|
|
||||||
/** Returns {@link JpaTransactionManager} instance. */
|
/** Returns {@link JpaTransactionManager} instance. */
|
||||||
public static JpaTransactionManager jpaTm() {
|
public static JpaTransactionManager jpaTm() {
|
||||||
return jpaTm;
|
return jpaTm.get();
|
||||||
|
}
|
||||||
|
|
||||||
|
@VisibleForTesting
|
||||||
|
static void setJpaTmForTesting(JpaTransactionManager newJpaTm) {
|
||||||
|
jpaTm = Suppliers.ofInstance(newJpaTm);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -237,7 +237,7 @@ final class RegistryCli implements AutoCloseable, CommandRunner {
|
||||||
// Enable Cloud SQL for command that needs remote API as they will very likely use
|
// Enable Cloud SQL for command that needs remote API as they will very likely use
|
||||||
// Cloud SQL after the database migration. Note that the DB password is stored in Datastore
|
// Cloud SQL after the database migration. Note that the DB password is stored in Datastore
|
||||||
// and it is already initialized above.
|
// and it is already initialized above.
|
||||||
RegistryToolEnvironment.get().enableJpaTm();
|
RegistryToolEnvironment.enableJpaTm();
|
||||||
}
|
}
|
||||||
|
|
||||||
command.run();
|
command.run();
|
||||||
|
|
|
@ -132,13 +132,13 @@ abstract class JpaTransactionManagerRule extends ExternalResource {
|
||||||
properties,
|
properties,
|
||||||
extraEntityClasses);
|
extraEntityClasses);
|
||||||
JpaTransactionManagerImpl txnManager = new JpaTransactionManagerImpl(emf, clock);
|
JpaTransactionManagerImpl txnManager = new JpaTransactionManagerImpl(emf, clock);
|
||||||
cachedTm = TransactionManagerFactory.jpaTm;
|
cachedTm = TransactionManagerFactory.jpaTm();
|
||||||
TransactionManagerFactory.jpaTm = txnManager;
|
TransactionManagerFactory.setJpaTmForTesting(txnManager);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void after() {
|
public void after() {
|
||||||
TransactionManagerFactory.jpaTm = cachedTm;
|
TransactionManagerFactory.setJpaTmForTesting(cachedTm);
|
||||||
if (emf != null) {
|
if (emf != null) {
|
||||||
emf.close();
|
emf.close();
|
||||||
emf = null;
|
emf = null;
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue