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.

Sign up for GitHub

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

[#4765] fix node name to be harmonized with xa node name #4806

Merged
merged 1 commit into from
Nov 7, 2019
Merged

[#4765] fix node name to be harmonized with xa node name #4806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion docs/src/main/asciidoc/transaction.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,22 @@ include::duration-format-note.adoc[]

The default value is 60 seconds.

== Configuring transaction node name identifier

Narayana, as the underlaying transaction manager, has an concept of unique node identifier.
This is important if you consider to use XA transactions with involve multiple resources.

The node name identifier plays a crucial part in the identification of a transaction.
The node name identifier is forged into the transaction id when the transaction is created.
Based on the node name identifier the transaction manager is capable to recognized the XA tranaction
counterparts created in database or JMS broker. The identifier makes possible for the transaction manager
to roll-back the transaction counterparts during recovery.

The node name identifier needs to be unique per transaction manager deployment.
And the node identifier needs to be stable over the transaction manager restarts.

The node name identifier may be configured via the property `quarkus.transaction-manager.node-name`.

== Why always having a transaction manager?

Does it work everywhere I want to?::
Expand Down Expand Up @@ -225,4 +241,3 @@ It's not a mess in Quarkus :)
Resource-level was introduced to support JPA in a non managed environment.
But Quarkus is both lean and a managed environment so we can safely always assume we are in JTA mode.
The end result is that the difficulties of running Hibernate ORM + CDI + a transaction manager in Java SE mode are solved by Quarkus.

6 changes: 5 additions & 1 deletion ...rayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/NarayanaJtaRecorder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package io.quarkus.narayana.jta.runtime;

import java.lang.reflect.Field;
import java.util.Collections;
import java.util.Properties;

import org.jboss.logging.Logger;

import com.arjuna.ats.arjuna.common.CoreEnvironmentBeanException;
import com.arjuna.ats.arjuna.common.arjPropertyManager;
import com.arjuna.ats.arjuna.coordinator.TxControl;
import com.arjuna.ats.jta.common.jtaPropertyManager;
import com.arjuna.common.util.propertyservice.PropertiesFactory;

import io.quarkus.runtime.annotations.Recorder;
Expand All @@ -23,7 +25,8 @@ public void setNodeName(final TransactionManagerConfiguration transactions) {

try {
arjPropertyManager.getCoreEnvironmentBean().setNodeIdentifier(transactions.nodeName);
TxControl.setXANodeName(transactions.xaNodeName.orElse(transactions.nodeName));
jtaPropertyManager.getJTAEnvironmentBean().setXaRecoveryNodes(Collections.singletonList(transactions.nodeName));
TxControl.setXANodeName(transactions.nodeName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we genuinely need the TxControl setting here? I would imagine it can be controlled through the arjPropertyManager directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the problematic issue of the whole narayana configuration. Once the TxControl class is first initialized (and that's decision of the JVM, not possible to be influence. normally the TxControl is initialized as class when accessed first time) then any change done in the arjPropertyManager has no effect for the further use.
As we can't be sure what setter is started first or where the TxControl data is needed for the first time during Quarkus startup I consider this as the safe way (or maybe even necessary).

A bit related point is that I created a jira about rethinking the configuration of the whole Narayana. That came from discussion with @mmusgrov, here https://issues.jboss.org/browse/JBTM-3215

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we do have to be certain of is there are no transactions that could be created before this is called and indeed the recovery manager has not started running before it. Are you sure that is the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'm not sure if it's the case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that this code must run before the transaction service is created? In which case I would expect no transactions could be created before this runs? Or is that not the case in Quarkus? IOW could there be a transaction created with the default (non-unique) node ID and that could therefore lead to data integrity issue?

/cc @mmusgrov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomjenkinson the need of use TxControl.setXANodeName is not caused by time when the transaction service is created. The need is caused by the fact that we can't be sure that nobody else calls the TxControl setter. When it happens then all data are initialized. When this method is called after the setter was used before then changes in the bean have no effect.

In summary, by me the TxControl call is needed here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming. I suppose you are less concerned about the setter being called (which as you say it is public) but rather when the class is loaded it will read the value as set in the arjPropertyManager: https://github.com/jbosstm/narayana/blob/master/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TxControl.java#L238

In that case, I understand the point - thanks!

} catch (CoreEnvironmentBeanException e) {
e.printStackTrace();
}
Expand All @@ -46,6 +49,7 @@ public void setDefaultProperties(Properties properties) {

public void setDefaultTimeout(TransactionManagerConfiguration transactions) {
transactions.defaultTransactionTimeout.ifPresent(defaultTimeout -> {
arjPropertyManager.getCoordinatorEnvironmentBean().setDefaultTimeout((int) defaultTimeout.getSeconds());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 - could the TxControl statement immediately following be removed too do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I assume it can't be. Just pointing that for WFLY we need to call TxControl. But WFLY configuration is more dynamic, so there it could be truly obvious.

TxControl.setDefaultTimeout((int) defaultTimeout.getSeconds());
});
}
Expand Down
6 changes: 0 additions & 6 deletions ...untime/src/main/java/io/quarkus/narayana/jta/runtime/TransactionManagerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ public final class TransactionManagerConfiguration {
@ConfigItem(defaultValue = "quarkus")
public String nodeName;

/**
* The XA node name used by the transaction manager
*/
@ConfigItem()
public Optional<String> xaNodeName;

/**
* The default transaction timeout
*/
Expand Down