Ensure the Transaction Manager node name is less than or equal to 28 bytes

[marcosgopen]

We need to include the node name in the fixed size Xid data structure(limited to 28 bytes) so that the TM can determine which XA resource branches it is responsible for.

related to PR: #36752

please note: this is fixing the issue truncating the byte array which means that it impacts the collision avoidance. This is why I added a stress test to 'test' the collision. I think that the risk of collision is still very low but a preferable solution may be to have a byte array type 'nodeName' so that the collision resistance exclusively relies on the hash algorithm.

• Resolves: Patch from #30491 "Generate right-length node name" seems invalid #40256

[marcosgopen]

FYI @turing85 and @mmusgrov

[mmusgrov]

I added a comment to the issue indicating that the spring boot port should just avoid using the UTF-8 encoding and use ISO-LATIN-1 instead. This would mean the original code submitted by @turing85 would stand.

[mmusgrov]

I am declining this PR because the node name is less than or equal to 28 bytes when a fixed-length character encoding is used to convert it to a byte array.

[marcosgopen]

Closed as not a bug

[geoand]

This is going to need a better title

[yrodiere]

LGTM, though I'll wait for @mmusgrov's approval before actually approving -- he knows the implications in Narayana, I really don't.

[graben]

Hi @marcosgopen, is there still something open preventing to merge this PR? I ask because I'd like to port this solution to narayana-spring-boot :)

FYI @geoand

[mmusgrov]

@graben We created and merged a narayana PR for https://issues.redhat.com/browse/JBTM-3883 (Add a setNodeIdentifier config method that accepts a byte array) which should render this PR superfluous but it would need to be replaced with a change that calls the new setNodeIdentifier config method that accepts a byte array. All being well, we should be releasing a version that contains that change this week.

[graben]

Hi @mmusgrov, thanks for the info. Does exist a timeline for the new narayana release containing this patch?

[mmusgrov]

All being well, we should be releasing a version that contains that change this week.

FYI The test that validates the new setting is https://github.com/jbosstm/narayana/blob/main/ArjunaJTA/jta/tests/classes/com/hp/mwtests/ts/jta/xa/NodeIdentifierUnitTest.java#L38 @graben you may notice that the test is largely based on the test you included with the original issue and thanks for contributing it :-)

[mmusgrov]

I'd recommend leaving this PR open until the new patch has been tested against Quarkus.

[tomjenkinson]

LGTM - thank you

[yrodiere]

Hey,

The implementation LGTM, thanks. Documentation contains a mistake, though: the name of the property.

I would also simplify phrasing in the documentation, but feel free to ignore that suggestion.

[marcosgopen]

Hey,

The implementation LGTM, thanks. Documentation contains a mistake, though: the name of the property.

I would also simplify phrasing in the documentation, but feel free to ignore that suggestion.

Thank you very much @yrodiere , updated and rebased.

[yrodiere]

Tom approved from the Narayana side, and the Quarkus side LGTM: let's merge once CI passes. Thanks!

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit e14067b.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[yrodiere]

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/hibernate-reactive/deployment 
! Skipped: extensions/panache/hibernate-reactive-panache-common/deployment extensions/panache/hibernate-reactive-panache-kotlin/deployment extensions/panache/hibernate-reactive-panache/deployment and 21 more

📦 extensions/hibernate-reactive/deployment

✖ Failed to execute goal io.fabric8:docker-maven-plugin:0.45.1:start (docker-start) on project quarkus-hibernate-reactive-deployment: I/O Error

This looks like an infra issue/flake, I restarted that build.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit e14067b.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 integration-tests/mongodb-panache

✖ io.quarkus.it.mongodb.panache.ReflectionFreeSerializationTest.testReactiveBookEntity - History

• com.fasterxml.jackson.databind.exc.MismatchedInputException: No content to map due to end-of-input at [Source: REDACTED (\StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1]-java.lang.RuntimeException`

java.lang.RuntimeException: 
com.fasterxml.jackson.databind.exc.MismatchedInputException: No content to map due to end-of-input
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1]
	at io.restassured.internal.path.json.mapping.JsonPathJackson2ObjectDeserializer.deserialize(JsonPathJackson2ObjectDeserializer.java:30)
	at io.restassured.path.json.mapping.JsonPathObjectDeserializer$deserialize.call(Unknown Source)
	at io.restassured.internal.mapping.Jackson2Mapper.deserialize(Jackson2Mapper.groovy:58)
	at io.restassured.mapper.ObjectMapper$deserialize.call(Unknown Source)
	at io.restassured.internal.mapping.ObjectMapping.parseWithJackson2(ObjectMapping.groovy:254)