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: fix throw NPE when get the state list #2949

Merged
merged 18 commits into from
Dec 8, 2020

Conversation

wangliang181230
Copy link
Contributor

@wangliang181230 wangliang181230 commented Jul 30, 2020

Ⅰ. Describe what this PR did

optimize: opt SAGA CompensationTriggerStateHandler
此处的stateMachineInstance必定存在,无需判断空值。
而且else处代码逻辑也有问题,即使进入else了,也是必定NPE。

Ⅱ. Does this pull request fix one issue?

fixes #2925

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@wangliang181230
Copy link
Contributor Author

@long187 ,麻烦review一下。

@codecov-commenter
Copy link

codecov-commenter commented Jul 30, 2020

Codecov Report

Merging #2949 into develop will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2949      +/-   ##
=============================================
+ Coverage      50.57%   50.58%   +0.01%     
+ Complexity      3105     3104       -1     
=============================================
  Files            599      599              
  Lines          19510    19510              
  Branches        2406     2407       +1     
=============================================
+ Hits            9868     9870       +2     
+ Misses          8650     8649       -1     
+ Partials         992      991       -1     
Impacted Files Coverage Δ Complexity Δ
...torage/file/store/FileTransactionStoreManager.java 57.41% <0.00%> (+0.64%) 29.00% <0.00%> (+1.00%)

@funky-eyes funky-eyes requested a review from long187 July 30, 2020 09:42
@codecov-io
Copy link

codecov-io commented Oct 10, 2020

Codecov Report

Merging #2949 into develop will decrease coverage by 0.36%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2949      +/-   ##
=============================================
- Coverage      50.93%   50.57%   -0.37%     
+ Complexity      3221     3162      -59     
=============================================
  Files            605      598       -7     
  Lines          19887    19702     -185     
  Branches        2481     2442      -39     
=============================================
- Hits           10130     9964     -166     
- Misses          8743     8749       +6     
+ Partials        1014      989      -25     
Impacted Files Coverage Δ Complexity Δ
...java/io/seata/server/session/SessionCondition.java 46.66% <0.00%> (-36.67%) 7.00% <0.00%> (-6.00%)
...rage/redis/store/RedisTransactionStoreManager.java 36.36% <0.00%> (-28.02%) 20.00% <0.00%> (-18.00%)
.../src/main/java/io/seata/core/rpc/ShutdownHook.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
.../java/io/seata/core/store/GlobalTransactionDO.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...ver/storage/redis/session/RedisSessionManager.java 29.31% <0.00%> (ø) 8.00% <0.00%> (-2.00%)
...ain/java/io/seata/core/rpc/hook/StatusRpcHook.java
...java/io/seata/server/storage/SessionConverter.java
...n/src/main/java/io/seata/common/rpc/RpcStatus.java
...java/io/seata/common/exception/RedisException.java
.../discovery/loadbalance/LeastActiveLoadBalance.java
... and 6 more

@wangliang181230 wangliang181230 added mode: SAGA SAGA transaction mode type: optimize TC tc module and removed TC tc module labels Oct 16, 2020
@wangliang181230 wangliang181230 added this to the 1.5.0 milestone Oct 29, 2020
@long187
Copy link
Contributor

long187 commented Dec 8, 2020

这段逻辑主要是怕从statemachineInstance里拿到的stateList为空,因为要恢复时可能只查询了statemachineInstance对象没有级联查询stateInstance列表,所以这里做了一个双保险,如果stateList为空,则查询一次。所以这里我建议在你改的基础上再判断一下stateList是不是为空,如果为空则用之前else里那段代码查询一次。

@wangliang181230
Copy link
Contributor Author

这段逻辑主要是怕从statemachineInstance里拿到的stateList为空,因为要恢复时可能只查询了statemachineInstance对象没有级联查询stateInstance列表,所以这里做了一个双保险,如果stateList为空,则查询一次。所以这里我建议在你改的基础上再判断一下stateList是不是为空,如果为空则用之前else里那段代码查询一次。

done

Copy link
Contributor

@long187 long187 left a comment

Choose a reason for hiding this comment

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

LGTM

@long187 long187 self-requested a review December 8, 2020 05:43
Copy link
Member

@jsbxyyx jsbxyyx left a comment

Choose a reason for hiding this comment

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

LGTM

@wangliang181230 wangliang181230 changed the title optimize: opt SAGA CompensationTriggerStateHandler bugfix: throw NPE when get the state list Dec 8, 2020
@wangliang181230 wangliang181230 changed the title bugfix: throw NPE when get the state list bugfix: fix throw NPE when get the state list Dec 8, 2020
@long187 long187 merged commit 7ce31c6 into apache:develop Dec 8, 2020
@wangliang181230 wangliang181230 deleted the optimize-saga-comp branch December 8, 2020 08:26
@wangliang181230 wangliang181230 modified the milestones: 1.5.0, 1.4.1 Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mode: SAGA SAGA transaction mode type: optimize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saga的CompensationTriggerStateHandler空判断有问题
7 participants