Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use RESOURCE_LOCAL transactions for JPA map storage #15315

Merged
merged 1 commit into from Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -20,6 +20,7 @@
import static org.keycloak.quarkus.runtime.KeycloakRecorder.DEFAULT_HEALTH_ENDPOINT;
import static org.keycloak.quarkus.runtime.KeycloakRecorder.DEFAULT_METRICS_ENDPOINT;
import static org.keycloak.quarkus.runtime.Providers.getProviderManager;
import static org.keycloak.quarkus.runtime.configuration.Configuration.getConfig;
import static org.keycloak.quarkus.runtime.configuration.Configuration.getPropertyNames;
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_QUARKUS;
import static org.keycloak.quarkus.runtime.configuration.QuarkusPropertiesConfigSource.QUARKUS_PROPERTY_ENABLED;
Expand Down Expand Up @@ -49,6 +50,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.function.Consumer;
Expand Down Expand Up @@ -81,6 +83,7 @@
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.jpa.boot.internal.ParsedPersistenceXmlDescriptor;
import org.hibernate.jpa.boot.internal.PersistenceXmlParser;
import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;
import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.ClassInfo;
Expand All @@ -93,6 +96,7 @@
import org.keycloak.common.crypto.FipsMode;
import org.keycloak.config.SecurityOptions;
import org.keycloak.config.StorageOptions;
import org.keycloak.config.TransactionOptions;
import org.keycloak.connections.jpa.JpaConnectionProvider;
import org.keycloak.connections.jpa.JpaConnectionSpi;
import org.keycloak.models.map.storage.jpa.JpaMapStorageProviderFactory;
Expand Down Expand Up @@ -273,7 +277,25 @@ private void configureDefaultPersistenceUnitProperties(ParsedPersistenceXmlDescr
Properties unitProperties = descriptor.getProperties();

unitProperties.setProperty(AvailableSettings.DIALECT, config.defaultPersistenceUnit.dialect.dialect.orElse(null));
unitProperties.setProperty(AvailableSettings.JPA_TRANSACTION_TYPE, PersistenceUnitTransactionType.JTA.name());
if (Objects.equals(getConfig().getConfigValue("kc.transaction-jta-enabled").getValue(), "disabled")) {
unitProperties.setProperty(AvailableSettings.JPA_TRANSACTION_TYPE, PersistenceUnitTransactionType.RESOURCE_LOCAL.name());

// Only changing this for the new map storage to keep the legacy JPA store untouched as it wasn't tested for the legacy store.
// follow-up on this in https://github.com/keycloak/keycloak/issues/13222 to re-visit the auto-commit handling
String storage = Configuration.getRawValue(
MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX.concat(StorageOptions.STORAGE.getKey()));
if (storage != null) {
// Needed to change the connection handling to avoid Hibernate returning the connection too early,
// which then interfered with the auto-commit reset that's done at the end of the transaction and PgConnection throwing a "Cannot commit when autoCommit is enabled."
// The current default is DELAYED_ACQUISITION_AND_RELEASE_BEFORE_TRANSACTION_COMPLETION which would only make sense for JTA IMHO.
// https://github.com/quarkusio/quarkus/blob/8d89101ffa65465b33d06360047095046bb726e4/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/boot/FastBootMetadataBuilder.java#L287-L288
unitProperties.setProperty(AvailableSettings.CONNECTION_HANDLING, PhysicalConnectionHandlingMode.DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION.name());
}
} else {
// will happen for both "enabled" and "xa"
unitProperties.setProperty(AvailableSettings.JPA_TRANSACTION_TYPE, PersistenceUnitTransactionType.JTA.name());
}

unitProperties.setProperty(AvailableSettings.QUERY_STARTUP_CHECKING, Boolean.FALSE.toString());

String dbKind = jdbcDataSources.get(0).getDbKind();
Expand Down
Expand Up @@ -17,10 +17,6 @@

package org.keycloak.quarkus.runtime.storage.database.jpa;

import static org.keycloak.config.StorageOptions.STORAGE;
import static org.keycloak.quarkus.runtime.configuration.Configuration.getOptionalValue;
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;

import java.lang.annotation.Annotation;
import java.sql.Connection;
import java.sql.SQLException;
Expand All @@ -31,7 +27,6 @@
import org.hibernate.internal.SessionFactoryImpl;
import org.hibernate.internal.SessionImpl;
import org.keycloak.config.StorageOptions;
import org.keycloak.models.ModelException;
import org.keycloak.models.map.storage.jpa.JpaMapStorageProviderFactory;

import io.quarkus.arc.Arc;
Expand All @@ -58,8 +53,7 @@ protected EntityManagerFactory createEntityManagerFactory() {
@Override
protected EntityManager getEntityManager() {
EntityManager em = super.getEntityManager();
try {
Connection connection = em.unwrap(SessionImpl.class).connection();
em.unwrap(SessionImpl.class).doWork(connection -> {
// In the Undertow setup, Hibernate sets the connection to non-autocommit, and in the Quarkus setup the XA transaction manager does this.
// For the Quarkus setup without a XA transaction manager, we didn't find a way to have this setup automatically.
// There is also no known option to configure this in the Agroal DB connection pool in a Quarkus setup:
Expand All @@ -70,9 +64,7 @@ protected EntityManager getEntityManager() {
if (connection.getAutoCommit()) {
connection.setAutoCommit(false);
}
} catch (SQLException e) {
throw new ModelException("unable to set non-auto-commit to false");
}
});
return em;
}

Expand Down