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

Revert "Generate right-length nodeIdentifier/ XID" #141

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

graben
Copy link
Contributor

@graben graben commented Apr 21, 2024

Resolves #140

Update to #132/#136/quarkusio/quarkus#30491

@graben
Copy link
Contributor Author

graben commented Apr 21, 2024

For MariaDB I can confirm that the shorten transactionManagerId (xa node name) is causing an issue in command XA START. Generated hex String of xid is too long.

@geoand: Maybe hashing approach has drawback also for Quarkus. Should be verified!

@graben graben marked this pull request as draft April 22, 2024 05:37
@graben graben changed the title Introduce property shortenNodeNameIfNecessary with default false Introduce property shortenTransactionManagerIdIfNecessary with default false Apr 22, 2024
@graben graben force-pushed the gh140 branch 2 times, most recently from 49a7c20 to b4e0c5f Compare April 22, 2024 05:46
@geoand
Copy link
Member

geoand commented Apr 22, 2024

So if I understand correctly, you are changing the default behavior to not shorten the ID? Why this change of default (which is a breaking change)?

@graben
Copy link
Contributor Author

graben commented Apr 22, 2024

This feature was introduced in the last version 3.1.0 as a back port from Quarkus where it is disabled by default. So I do just add what should be there already. But it seems that this feature has drawbacks anyway in Spring boot and Quarkus which should be evaluated. 😉

@geoand
Copy link
Member

geoand commented Apr 23, 2024

Understood, thanks

@graben
Copy link
Contributor Author

graben commented Apr 23, 2024

Well, It seems the patch at Quarkus is wrong. SHA-224 creates a 28 sized byte array but after creating a UTF-8 String from it the corresponding byte array is somewhere around size 40-60 much bigger than 28 :-/

@geoand
Copy link
Member

geoand commented Apr 24, 2024

Thanks for investigating.

Please open a relevant issue in the Quarkus repo

@graben
Copy link
Contributor Author

graben commented Apr 24, 2024

In the meantime I would suggest to revert this feature for a 3.1.1 patch release until a better fix is found.

@geoand : okay for you?

@geoand
Copy link
Member

geoand commented Apr 24, 2024

Sounds good to me

@graben graben changed the title Introduce property shortenTransactionManagerIdIfNecessary with default false Revert #136 Generate right-length nodeIdentifier/ XID Apr 24, 2024
@graben graben changed the title Revert #136 Generate right-length nodeIdentifier/ XID Revert "Generate right-length nodeIdentifier/ XID" Apr 24, 2024
@graben graben marked this pull request as ready for review April 24, 2024 18:12
@graben
Copy link
Contributor Author

graben commented Apr 24, 2024

@geoand : Revert #136 as discussed and opened quarkusio/quarkus#40256 for Quarkus.
@jacobdotcosta: Could you plz do a 3.1.1 patch release after merge?

@geoand geoand merged commit 7af1731 into snowdrop:main Apr 25, 2024
2 checks passed
@graben graben deleted the gh140 branch April 25, 2024 07:26
@jacobdotcosta
Copy link
Member

I'll release 3.1.1.

@jacobdotcosta
Copy link
Member

Relased 3.1.1.Beta1, @graben could you check if it's OK before releasing the final version?

@graben
Copy link
Contributor Author

graben commented Apr 25, 2024

Looks good to me, but could really test it tomorrow, okay for you?

@jacobdotcosta
Copy link
Member

Sure, no problem. Thanks.

@graben
Copy link
Contributor Author

graben commented Apr 26, 2024

@jacobdotcosta: build and tested successfully. Ready to release! ;-)

@jacobdotcosta
Copy link
Member

3.1.1 released
@graben

@graben
Copy link
Contributor Author

graben commented Apr 29, 2024

@jacobdotcosta thx, but don't miss the scm tag 😉

@jacobdotcosta
Copy link
Member

fixed :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error enlisting after update to 3.1.0 in MariaDB and DB2
3 participants