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

feature: support configuring default global transaction timeoutMillis #2806

Merged
merged 30 commits into from
Aug 28, 2020

Conversation

wangliang181230
Copy link
Contributor

@wangliang181230 wangliang181230 commented Jun 17, 2020

Ⅰ. Describe what this PR did

optimize: Configurable default global transaction timeoutMillis

seata.client.tm.default-global-transaction-timeout=60000

Ⅱ. 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 17, 2020

Codecov Report

Merging #2806 into develop will decrease coverage by 0.00%.
The diff coverage is 54.54%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2806      +/-   ##
=============================================
- Coverage      50.37%   50.37%   -0.01%     
- Complexity      3090     3091       +1     
=============================================
  Files            600      600              
  Lines          19520    19541      +21     
  Branches        2411     2413       +2     
=============================================
+ Hits            9834     9843       +9     
- Misses          8699     8707       +8     
- Partials         987      991       +4     
Impacted Files Coverage Δ Complexity Δ
...n/src/main/java/io/seata/common/DefaultValues.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...ava/io/seata/core/constants/ConfigurationKeys.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...a/io/seata/tm/api/transaction/TransactionInfo.java 88.46% <ø> (ø) 14.00 <0.00> (ø)
.../autoconfigure/properties/client/TmProperties.java 40.00% <50.00%> (+1.90%) 4.00 <1.00> (+1.00)
...ing/annotation/GlobalTransactionalInterceptor.java 28.47% <55.55%> (+3.27%) 9.00 <2.00> (+2.00)
...o/seata/server/coordinator/DefaultCoordinator.java 54.63% <0.00%> (-0.52%) 28.00% <0.00%> (-1.00%)
...in/java/io/seata/server/session/GlobalSession.java 82.43% <0.00%> (-0.46%) 70.00% <0.00%> (-1.00%)

@slievrly slievrly added the module/spring spring module label Jun 17, 2020
@l81893521 l81893521 added this to the 1.4.0 milestone Jul 16, 2020
@wangliang181230 wangliang181230 changed the title optimize: Configurable global transaction timeoutMillis optimize: Configurable default global transaction timeoutMillis Jul 19, 2020
…global-config

# Conflicts:
#	seata-spring-boot-starter/src/main/java/io/seata/spring/boot/autoconfigure/properties/client/TmProperties.java
#	seata-spring-boot-starter/src/test/java/io/seata/spring/boot/autoconfigure/PropertiesTest.java
#	spring/src/main/java/io/seata/spring/annotation/GlobalTransactionalInterceptor.java
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

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

@lightClouds917 lightClouds917 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
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

@slievrly slievrly changed the title optimize: Configurable default global transaction timeoutMillis feature: support configuring default global transaction timeoutMillis Aug 28, 2020
@slievrly slievrly merged commit 17fa7c2 into apache:develop Aug 28, 2020
@wangliang181230 wangliang181230 deleted the timeout-global-config branch August 28, 2020 03:53
@wangliang181230 wangliang181230 added TM Relate to seata tm type: feature Category issues or prs related to feature request. labels Oct 19, 2020
Copy link
Contributor

@ls9527 ls9527 left a comment

Choose a reason for hiding this comment

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

你看还能不能在改改?

@@ -160,8 +186,14 @@ public String name() {

@Override
public TransactionInfo getTransactionInfo() {
// reset the value of timeout
int timeout = globalTrxAnno.timeoutMills();
if (timeout <= 0 || timeout == DEFAULT_GLOBAL_TRANSACTION_TIMEOUT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

对于场景: 全局默认15秒, 指定接口希望60秒。可咋整?
注解应该默认-1,然后这里取消 timeout == DEFAULT_GLOBAL_TRANSACTION_TIMEOUT 的判断才对。

file.properties:

client.tm.defaultGlobalTransactionTimeout=15000

代码如下:

    @GlobalTransactional(timeoutMills = 60000, name = "dubbo-gts-seata-example")
    public ObjectResponse handleBusiness(BusinessDTO businessDTO) {
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

当初是这么写的,但是大佬说会不好理解,就改成现在这样的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

你可以设置成61秒就行了

Copy link
Contributor

Choose a reason for hiding this comment

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

宝宝心里苦,宝宝说不出。

spring的TransactionDefinition都默认-1了,这里却指定60秒是特殊数字。

但我相信有一天大佬会踩着七彩祥云把这里改掉的

Copy link
Contributor

Choose a reason for hiding this comment

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

宝宝心里苦,宝宝说不出。

spring的TransactionDefinition都默认-1了,这里却指定60秒是特殊数字。

但我相信有一天大佬会踩着七彩祥云把这里改掉的

分布式事务不应该想本地事务一样默认无超时时间,因为分布式事务通常发生在调用链当中,如果事务过久不完成,占用的资源会更多,引起的阻塞也会更多

Copy link
Contributor

Choose a reason for hiding this comment

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

有超时时间啊, -1就用全局默认超时时间60秒嘛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/spring spring module TM Relate to seata tm type: feature Category issues or prs related to feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants