-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
feature: support configuring default global transaction timeoutMillis #2806
Conversation
Codecov Report
@@ 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
|
spring/src/main/java/io/seata/spring/annotation/GlobalTransactional.java
Outdated
Show resolved
Hide resolved
spring/src/main/java/io/seata/spring/annotation/GlobalTransactionalInterceptor.java
Outdated
Show resolved
Hide resolved
spring/src/main/java/io/seata/spring/annotation/GlobalTransactionalInterceptor.java
Outdated
Show resolved
Hide resolved
spring/src/main/java/io/seata/spring/annotation/GlobalTransactionalInterceptor.java
Outdated
Show resolved
Hide resolved
spring/src/main/java/io/seata/spring/annotation/GlobalTransactionalInterceptor.java
Outdated
Show resolved
Hide resolved
spring/src/main/java/io/seata/spring/annotation/GlobalTransactionalInterceptor.java
Outdated
Show resolved
Hide resolved
…out-global-config
…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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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) {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
当初是这么写的,但是大佬说会不好理解,就改成现在这样的。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
你可以设置成61秒就行了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
宝宝心里苦,宝宝说不出。
spring的TransactionDefinition都默认-1了,这里却指定60秒是特殊数字。
但我相信有一天大佬会踩着七彩祥云把这里改掉的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
宝宝心里苦,宝宝说不出。
spring的TransactionDefinition都默认-1了,这里却指定60秒是特殊数字。
但我相信有一天大佬会踩着七彩祥云把这里改掉的
分布式事务不应该想本地事务一样默认无超时时间,因为分布式事务通常发生在调用链当中,如果事务过久不完成,占用的资源会更多,引起的阻塞也会更多
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
有超时时间啊, -1就用全局默认超时时间60秒嘛
Ⅰ. 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