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

Issues/13540: Transaction Propagation feature implementation #15141

Merged
merged 1 commit into from Aug 27, 2019

Conversation

@sbespalov
Copy link
Contributor

sbespalov commented Jun 10, 2019

Fixes #13540

Main changes:

  • suspend() and resume() methods added for TransactionContext interface
  • Propagation.REQUIRES_NEW support added for HazelcastTransactionManager
  • thread bound transaction management reworked within TransactionImpl class
@devOpsHazelcast

This comment has been minimized.

Copy link
Contributor

devOpsHazelcast commented Jun 10, 2019

Can one of the admins verify this patch?

@sbespalov sbespalov force-pushed the sbespalov:issues/13540 branch 2 times, most recently from 2f9ffe0 to 4b3b8db Jun 10, 2019
@mmedenjak mmedenjak requested a review from gurbuzali Jun 11, 2019
@mmedenjak mmedenjak added this to the 4.0 milestone Jun 11, 2019
@mmedenjak mmedenjak self-requested a review Jun 11, 2019
@sbespalov

This comment has been minimized.

Copy link
Contributor Author

sbespalov commented Jun 21, 2019

@mmedenjak, just want to remind that this PR waiting review: smiley:

@sbespalov

This comment has been minimized.

Copy link
Contributor Author

sbespalov commented Jul 9, 2019

@gurbuzali can you please help with review?

@petrpleshachkov petrpleshachkov self-requested a review Jul 17, 2019
Copy link
Contributor

mmedenjak left a comment

Added some initial thoughts. Please take a look. Overall it looks good. In addition, we'll have to consider if this change might have some unforeseen consequences on other systems using transactions.

@sbespalov sbespalov force-pushed the sbespalov:issues/13540 branch from ebbf7b2 to de8fbdb Jul 18, 2019
@sbespalov

This comment has been minimized.

Copy link
Contributor Author

sbespalov commented Jul 18, 2019

@mmedenjak thanks for review

can you please have a look the update?

@sbespalov sbespalov force-pushed the sbespalov:issues/13540 branch from de8fbdb to 2520d51 Jul 18, 2019
@@ -89,6 +89,18 @@ public void rollbackTransaction() {
}

@Override
public void suspendTransaction() {
// TODO: implemet

This comment has been minimized.

Copy link
@petrpleshachkov

petrpleshachkov Jul 24, 2019

Contributor

Do you plan to "implement" the methods in this PR?

This comment has been minimized.

Copy link
@sbespalov

sbespalov Jul 29, 2019

Author Contributor

Initially I thought to it within follow-up tasks, in case of this PR will be approved and merged.

Copy link
Contributor

petrpleshachkov left a comment

I am fine with this PR. Only minor issues should be fixed before merge. Thanks for the great work!

@sbespalov sbespalov force-pushed the sbespalov:issues/13540 branch 2 times, most recently from b81acd3 to f3d047a Jul 30, 2019
@sbespalov

This comment has been minimized.

Copy link
Contributor Author

sbespalov commented Jul 30, 2019

@petrpleshachkov thanks for review.
Requested changes provided.

Copy link
Contributor

mmedenjak left a comment

Looks good, two more minor comments, please take a look before it is merged. Thank you for the PR!

@sbespalov sbespalov force-pushed the sbespalov:issues/13540 branch from 995d624 to dfc701b Aug 13, 2019
@sbespalov

This comment has been minimized.

Copy link
Contributor Author

sbespalov commented Aug 13, 2019

@mmedenjak thanks for review.
Requested changes provided.

@tkountis

This comment has been minimized.

Copy link
Contributor

tkountis commented Aug 27, 2019

@petrpleshachkov please review the latest changes and lets merge it

@petrpleshachkov

This comment has been minimized.

Copy link
Contributor

petrpleshachkov commented Aug 27, 2019

The PR looks good. I will merge it into master.

@petrpleshachkov petrpleshachkov merged commit 51675d8 into hazelcast:master Aug 27, 2019
@sbespalov sbespalov mentioned this pull request Oct 9, 2019
0 of 5 tasks complete
mmedenjak pushed a commit to mmedenjak/hazelcast that referenced this pull request Dec 19, 2019
…azelcast#15141)"

This reverts commit 51675d8.

After BETA-1 release, we have encoutered issues with nested transactions
(e.g. suspend / resume logic with Weblogic, JCA adaptor) and it seems
the feature is incomplete. We are therefore reverting to prevent giving
a false feeling of confidence. We need to invest more time into the
development of the feature or, actually, we need to redesign
transactions from the ground up.
mmedenjak added a commit that referenced this pull request Dec 20, 2019
Revert "issues/13540: Transaction Propagation support implementation (#15141)"

This reverts commit 51675d8.

After BETA-1 release, we have encoutered issues with nested transactions
(e.g. suspend / resume logic with Weblogic, JCA adaptor) and it seems
the feature is incomplete. We are therefore reverting to prevent giving
a false feeling of confidence. We need to invest more time into the
development of the feature or, actually, we need to redesign
transactions from the ground up.
lukasherman pushed a commit to lukasherman/hazelcast that referenced this pull request Jan 2, 2020
Revert "issues/13540: Transaction Propagation support implementation (hazelcast#15141)"

This reverts commit 51675d8.

After BETA-1 release, we have encoutered issues with nested transactions
(e.g. suspend / resume logic with Weblogic, JCA adaptor) and it seems
the feature is incomplete. We are therefore reverting to prevent giving
a false feeling of confidence. We need to invest more time into the
development of the feature or, actually, we need to redesign
transactions from the ground up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.