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

bugfix: TM rollback fail throw the seata exception, rollback retrying throw NPE #2760

Merged
merged 10 commits into from
Jun 24, 2020

Conversation

wangliang181230
Copy link
Contributor

@wangliang181230 wangliang181230 commented Jun 4, 2020

Ⅰ. Describe what this PR did

BUG描述:

1、TM在往TC发起回滚请求时,如果连接TC超时了,会向外抛出超时异常,但实际上需要往外抛的是业务异常。
2、TM在往TC发起回滚请求时,RM端回滚失败导致全局事务进入RollbackRetrying状态时,触发了空指针异常。

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2020

Codecov Report

Merging #2760 into develop will increase coverage by 0.12%.
The diff coverage is 22.22%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2760      +/-   ##
=============================================
+ Coverage      50.05%   50.17%   +0.12%     
  Complexity      2949     2949              
=============================================
  Files            588      588              
  Lines          18907    18931      +24     
  Branches        2241     2284      +43     
=============================================
+ Hits            9463     9498      +35     
- Misses          8478     8489      +11     
+ Partials         966      944      -22     
Impacted Files Coverage Δ Complexity Δ
...ing/annotation/GlobalTransactionalInterceptor.java 24.40% <0.00%> (ø) 7.00 <0.00> (ø)
...in/java/io/seata/tm/api/TransactionalTemplate.java 64.34% <33.33%> (+0.31%) 25.00 <2.00> (ø)
...ava/io/seata/tm/api/DefaultFailureHandlerImpl.java 68.57% <50.00%> (ø) 6.00 <1.00> (ø)
...o/seata/server/storage/redis/lock/RedisLocker.java 37.96% <0.00%> (-0.72%) 10.00% <0.00%> (ø%)
...in/java/io/seata/server/session/GlobalSession.java 83.71% <0.00%> (-0.55%) 71.00% <0.00%> (ø%)
...rage/redis/store/RedisTransactionStoreManager.java 36.05% <0.00%> (-0.53%) 20.00% <0.00%> (ø%)
...a/io/seata/core/rpc/netty/AbstractRpcRemoting.java 12.50% <0.00%> (-0.30%) 5.00% <0.00%> (ø%)
...java/io/seata/spring/tcc/TccActionInterceptor.java 11.36% <0.00%> (-0.27%) 2.00% <0.00%> (ø%)
...n/java/io/seata/rm/datasource/ConnectionProxy.java 28.00% <0.00%> (-0.23%) 13.00% <0.00%> (ø%)
...in/java/io/seata/server/session/BranchSession.java 78.10% <0.00%> (+0.07%) 44.00% <0.00%> (ø%)
... and 23 more

@l81893521 l81893521 added the module/spring spring module label Jun 5, 2020
@funky-eyes
Copy link
Contributor

only TCC have the issue of appeal?

@wangliang181230
Copy link
Contributor Author

only TCC have the issue of appeal?

目前我测的是TCC会触发此BUG。其他事务模式还不确定。
不过BUG存在的依据:
io.seata.tm.api.TransactionalTemplate
168行: throw new TransactionalExecutor.ExecutionException(tx, GlobalStatus.RollbackRetrying.equals(tx.getLocalStatus()) ? TransactionalExecutor.Code.RollbackRetrying : TransactionalExecutor.Code.RollbackDone, ex);

@funky-eyes
Copy link
Contributor

only TCC have the issue of appeal?

目前我测的是TCC会触发此BUG。其他事务模式还不确定。
不过BUG存在的依据:
io.seata.tm.api.TransactionalTemplate
168行: throw new TransactionalExecutor.ExecutionException(tx, GlobalStatus.RollbackRetrying.equals(tx.getLocalStatus()) ? TransactionalExecutor.Code.RollbackRetrying : TransactionalExecutor.Code.RollbackDone, ex);

能否提供复现流程,以及如果更改后对AT是否会有影响需要一个测试

@wangliang181230
Copy link
Contributor Author

only TCC have the issue of appeal?

目前我测的是TCC会触发此BUG。其他事务模式还不确定。
不过BUG存在的依据:
io.seata.tm.api.TransactionalTemplate
168行: throw new TransactionalExecutor.ExecutionException(tx, GlobalStatus.RollbackRetrying.equals(tx.getLocalStatus()) ? TransactionalExecutor.Code.RollbackRetrying : TransactionalExecutor.Code.RollbackDone, ex);

能否提供复现流程,以及如果更改后对AT是否会有影响需要一个测试

我检索过源码,这行代码必定会抛空指针异常。TransactionalExecutor.Code为RollbackRetrying时,只写入了originalException,并没有写入cause。
所以,这个已经不是哪种分支事务会触发它的问题,而是它就是个BUG。

@funky-eyes
Copy link
Contributor

only TCC have the issue of appeal?

目前我测的是TCC会触发此BUG。其他事务模式还不确定。
不过BUG存在的依据:
io.seata.tm.api.TransactionalTemplate
168行: throw new TransactionalExecutor.ExecutionException(tx, GlobalStatus.RollbackRetrying.equals(tx.getLocalStatus()) ? TransactionalExecutor.Code.RollbackRetrying : TransactionalExecutor.Code.RollbackDone, ex);

能否提供复现流程,以及如果更改后对AT是否会有影响需要一个测试

我检索过源码,这行代码必定会抛空指针异常。TransactionalExecutor.Code为RollbackRetrying时,只写入了originalException,并没有写入cause。
所以,这个已经不是哪种分支事务会触发它的问题,而是它就是个BUG。

ok,我稍后测试下这个异常.

@wangliang181230
Copy link
Contributor Author

only TCC have the issue of appeal?

目前我测的是TCC会触发此BUG。其他事务模式还不确定。
不过BUG存在的依据:
io.seata.tm.api.TransactionalTemplate
168行: throw new TransactionalExecutor.ExecutionException(tx, GlobalStatus.RollbackRetrying.equals(tx.getLocalStatus()) ? TransactionalExecutor.Code.RollbackRetrying : TransactionalExecutor.Code.RollbackDone, ex);

能否提供复现流程,以及如果更改后对AT是否会有影响需要一个测试

复现是当全局事务异常时,二阶段回滚失败时,会复现。
你可以在TCC二阶段回滚故意抛异常,就会复现了。

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

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

why not change the other TransactionalExecutor.ExecutionException constructor in

io.seata.tm.api.TransactionalTemplate#rollbackTransaction

Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

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

LGTM

@l81893521 l81893521 added the module/tm tm module label Jun 24, 2020
@wangliang181230 wangliang181230 changed the title bugfix: NullPointerException: throw null after TM rollbacked when the globalStatus is RollbackRetrying. bugfix: TM rollback fail throw the seata exception, rollback retrying throw NPE Jun 24, 2020
Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM

@funky-eyes funky-eyes merged commit c860c6b into apache:develop Jun 24, 2020
@wangliang181230 wangliang181230 deleted the bugfix-throw-null branch June 28, 2020 02:39
@slievrly slievrly added this to the 1.3.0 milestone Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/spring spring module module/tm tm module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants