From 69d5d3d3c2f5c64d7c00583ff77ccabeea0d29c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Fri, 3 May 2024 15:10:12 +0200 Subject: [PATCH] Rework `quarkus.transaction-manager.allow-unsafe-multiple-last-resources` into `quarkus.transaction-manager.unsafe-multiple-last-resources` * The property is now named `quarkus.transaction-manager.unsafe-multiple-last-resources` * It has three possible values: * `allow`: allow unsafe multiple last resources, no warning per offending transaction * `warn`: allow unsafe multiple last resources, one warning log per offending transaction * `fail`: fail on unsafe multiple last resources * It will log a warning on startup if *explicitly* set to `allow` or `warn`. * It defaults to `fail` in this commit. * The plan is to make it default to `warn` in 3.8, which means no log on startup, but one per offending transaction. People will be able to set it to `allow` explicitly to suppress the warning per offending transaction, but will get a warning on startup (since they set the property explicitly). (cherry picked from commit 25954987a21dc7b06ffa1596cc3ed474517773a3) --- docs/src/main/asciidoc/datasource.adoc | 8 +++- .../jta/deployment/NarayanaJtaProcessor.java | 39 +++++++++++++------ .../jta/runtime/NarayanaJtaRecorder.java | 13 ++++--- .../TransactionManagerBuildTimeConfig.java | 37 +++++++++++++++--- 4 files changed, 74 insertions(+), 23 deletions(-) diff --git a/docs/src/main/asciidoc/datasource.adoc b/docs/src/main/asciidoc/datasource.adoc index 749c8e22b0d1d..74789deccc678 100644 --- a/docs/src/main/asciidoc/datasource.adoc +++ b/docs/src/main/asciidoc/datasource.adoc @@ -572,13 +572,17 @@ or xref:transaction.adoc#legacy-api-approach[`UserTransaction`] (for more comple ==== As a last resort, and for compatibility with Quarkus 3.8 and earlier, you may allow unsafe transaction handling across multiple non-XA datasources -by setting `quarkus.transaction-manager.allow-unsafe-multiple-last-resources` to `true`. +by setting `quarkus.transaction-manager.unsafe-multiple-last-resources` to `allow`. -With this property set to `true`, a transaction rollback +With this property set to `allow`, a transaction rollback could possibly be applied to only some of the non-XA datasources, with other non-XA datasources having already committed their changes, leaving your overall system in an inconsistent state. +Setting this property to `warn` leads to the same unsafe behavior, +but with a warning being logged on each offending transaction, +instead of a single one on startup. + We do not recommend using this configuration property, and we plan to remove it in the future, so you should plan fixing your application accordingly. diff --git a/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java b/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java index ef48f23452d5c..63b968dac6f9c 100644 --- a/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java +++ b/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java @@ -70,6 +70,7 @@ import io.quarkus.narayana.jta.runtime.NarayanaJtaProducers; import io.quarkus.narayana.jta.runtime.NarayanaJtaRecorder; import io.quarkus.narayana.jta.runtime.TransactionManagerBuildTimeConfig; +import io.quarkus.narayana.jta.runtime.TransactionManagerBuildTimeConfig.UnsafeMultipleLastResourcesMode; import io.quarkus.narayana.jta.runtime.TransactionManagerConfiguration; import io.quarkus.narayana.jta.runtime.context.TransactionContext; import io.quarkus.narayana.jta.runtime.graal.DisableLoggingFeature; @@ -145,9 +146,11 @@ public void build(NarayanaJtaRecorder recorder, builder.addBeanClass(TransactionalInterceptorNotSupported.class); additionalBeans.produce(builder.build()); - if (transactionManagerBuildTimeConfig.allowUnsafeMultipleLastResources) { - recorder.logAllowUnsafeMultipleLastResources(); - } + transactionManagerBuildTimeConfig.unsafeMultipleLastResources.ifPresent(mode -> { + if (!mode.equals(UnsafeMultipleLastResourcesMode.FAIL)) { + recorder.logUnsafeMultipleLastResourcesOnStartup(mode); + } + }); //we want to force Arjuna to init at static init time Properties defaultProperties = PropertiesFactory.getDefaultProperties(); @@ -171,20 +174,34 @@ public void allowUnsafeMultipleLastResources(NarayanaJtaRecorder recorder, TransactionManagerBuildTimeConfig transactionManagerBuildTimeConfig, Capabilities capabilities, BuildProducer logCleanupFilters, BuildProducer nativeImageFeatures) { - if (transactionManagerBuildTimeConfig.allowUnsafeMultipleLastResources) { - recorder.allowUnsafeMultipleLastResources(capabilities.isPresent(Capability.AGROAL)); - - // we will handle these warnings ourselves at runtime init - logCleanupFilters.produce( - new LogCleanupFilterBuildItem("com.arjuna.ats.arjuna", "ARJUNA012139", "ARJUNA012141", "ARJUNA012142")); + switch (transactionManagerBuildTimeConfig.unsafeMultipleLastResources + .orElse(UnsafeMultipleLastResourcesMode.DEFAULT)) { + case ALLOW -> { + recorder.allowUnsafeMultipleLastResources(capabilities.isPresent(Capability.AGROAL), true); + // we will handle the warnings ourselves at runtime init when the option is set explicitly + logCleanupFilters.produce( + new LogCleanupFilterBuildItem("com.arjuna.ats.arjuna", "ARJUNA012139", "ARJUNA012141", "ARJUNA012142")); + } + case WARN -> { + recorder.allowUnsafeMultipleLastResources(capabilities.isPresent(Capability.AGROAL), false); + // we will handle the warnings ourselves at runtime init when the option is set explicitly + // but we still want Narayana to produce one warning per offending transaction + logCleanupFilters.produce( + new LogCleanupFilterBuildItem("com.arjuna.ats.arjuna", "ARJUNA012139", "ARJUNA012142")); + } + case FAIL -> { // No need to do anything, this is the default behavior of Narayana + } } } @BuildStep(onlyIf = NativeOrNativeSourcesBuild.class) public void nativeImageFeature(TransactionManagerBuildTimeConfig transactionManagerBuildTimeConfig, BuildProducer nativeImageFeatures) { - if (transactionManagerBuildTimeConfig.allowUnsafeMultipleLastResources) { - nativeImageFeatures.produce(new NativeImageFeatureBuildItem(DisableLoggingFeature.class)); + switch (transactionManagerBuildTimeConfig.unsafeMultipleLastResources + .orElse(UnsafeMultipleLastResourcesMode.DEFAULT)) { + case ALLOW, WARN -> { + nativeImageFeatures.produce(new NativeImageFeatureBuildItem(DisableLoggingFeature.class)); + } } } diff --git a/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/NarayanaJtaRecorder.java b/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/NarayanaJtaRecorder.java index 1833d285e6d95..3f858f774e592 100644 --- a/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/NarayanaJtaRecorder.java +++ b/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/NarayanaJtaRecorder.java @@ -7,6 +7,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.Set; @@ -114,9 +115,9 @@ public void setConfig(final TransactionManagerConfiguration transactions) { * This should be removed in the future. */ @Deprecated(forRemoval = true) - public void allowUnsafeMultipleLastResources(boolean agroalPresent) { + public void allowUnsafeMultipleLastResources(boolean agroalPresent, boolean disableMultipleLastResourcesWarning) { arjPropertyManager.getCoreEnvironmentBean().setAllowMultipleLastResources(true); - arjPropertyManager.getCoreEnvironmentBean().setDisableMultipleLastResourcesWarning(true); + arjPropertyManager.getCoreEnvironmentBean().setDisableMultipleLastResourcesWarning(disableMultipleLastResourcesWarning); if (agroalPresent) { jtaPropertyManager.getJTAEnvironmentBean() .setLastResourceOptimisationInterfaceClassName("io.agroal.narayana.LocalXAResource"); @@ -127,9 +128,11 @@ public void allowUnsafeMultipleLastResources(boolean agroalPresent) { * This should be removed in the future. */ @Deprecated(forRemoval = true) - public void logAllowUnsafeMultipleLastResources() { - log.warn( - "Setting quarkus.transaction-manager.allow-unsafe-multiple-last-resources to true makes adding multiple resources to the same transaction unsafe."); + public void logUnsafeMultipleLastResourcesOnStartup( + TransactionManagerBuildTimeConfig.UnsafeMultipleLastResourcesMode mode) { + log.warnf( + "Setting quarkus.transaction-manager.unsafe-multiple-last-resources to '%s' makes adding multiple resources to the same transaction unsafe.", + mode.name().toLowerCase(Locale.ROOT)); } private void setObjectStoreDir(String name, TransactionManagerConfiguration config) { diff --git a/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/TransactionManagerBuildTimeConfig.java b/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/TransactionManagerBuildTimeConfig.java index b8822e6893916..70cde49f3efe7 100644 --- a/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/TransactionManagerBuildTimeConfig.java +++ b/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/TransactionManagerBuildTimeConfig.java @@ -1,5 +1,7 @@ package io.quarkus.narayana.jta.runtime; +import java.util.Optional; + import io.quarkus.runtime.annotations.ConfigItem; import io.quarkus.runtime.annotations.ConfigPhase; import io.quarkus.runtime.annotations.ConfigRoot; @@ -7,9 +9,10 @@ @ConfigRoot(phase = ConfigPhase.BUILD_TIME) public final class TransactionManagerBuildTimeConfig { /** - * Allow using multiple XA unaware resources in the same transactional demarcation. - * This is UNSAFE and may only be used for compatibility. + * Define the behavior when using multiple XA unaware resources in the same transactional demarcation. *

+ * Defaults to {@code fail}. + * {@code warn} and {@code allow} are UNSAFE and should only be used for compatibility. * Either use XA for all resources if you want consistency, or split the code into separate * methods with separate transactions. *

@@ -28,7 +31,31 @@ public final class TransactionManagerBuildTimeConfig { * @deprecated This property is planned for removal in a future version. */ @Deprecated(forRemoval = true) - @ConfigItem(defaultValue = "false") - public boolean allowUnsafeMultipleLastResources; + @ConfigItem(defaultValueDocumentation = "fail") + public Optional unsafeMultipleLastResources; + + public enum UnsafeMultipleLastResourcesMode { + /** + * Allow using multiple XA unaware resources in the same transactional demarcation. + *

+ * This will log a warning once on application startup, + * but not on each use of multiple XA unaware resources in the same transactional demarcation. + */ + ALLOW, + /** + * Allow using multiple XA unaware resources in the same transactional demarcation, + * but log a warning on each occurrence. + */ + WARN, + /** + * Allow using multiple XA unaware resources in the same transactional demarcation, + * but log a warning on each occurrence. + */ + FAIL; + + // The default is WARN in Quarkus 3.8, FAIL in Quarkus 3.9+ + // Make sure to update defaultValueDocumentation on unsafeMultipleLastResources when changing this. + public static final UnsafeMultipleLastResourcesMode DEFAULT = FAIL; + } -} \ No newline at end of file +}