add time limit when transaction retry on the server #824

XCXCXCXCX · 2019-04-18T02:37:01Z

Ⅰ. Describe what this PR did

add time limit when transaction retry on the server
Provide configuration items in file.conf :
max.commit.retry.timeout=
max.rollback.retry.timeout=
default permanent.

The retry timeout period is determined by the user. If it is not configured or configured as a negative number, it will run permanently; if configured to 0, it will be removed from the retry list on the first detection.

Ⅱ. 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

update

merge for name change

…elop

codecov-io · 2019-04-18T02:40:47Z

Codecov Report

Merging #824 into develop will decrease coverage by 0.15%.
The diff coverage is 9.09%.

@@              Coverage Diff              @@
##             develop     #824      +/-   ##
=============================================
- Coverage       39.6%   39.44%   -0.16%     
  Complexity      1138     1138              
=============================================
  Files            221      222       +1     
  Lines           8828     8872      +44     
  Branches        1130     1143      +13     
=============================================
+ Hits            3496     3500       +4     
- Misses          4891     4931      +40     
  Partials         441      441

Impacted Files	Coverage Δ	Complexity Δ
...c/main/java/io/seata/common/util/DurationUtil.java	`0% <0%> (ø)`	`0 <0> (?)`
...o/seata/server/coordinator/DefaultCoordinator.java	`31.79% <26.66%> (-0.49%)`	`14 <0> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f2dd84...b7af7dd. Read the comment docs.

leizhiyuan · 2019-04-19T01:15:03Z

common/src/main/java/io/seata/common/util/DurationUtil.java

+        }
+    }
+
+    public static void main(String[] args) {


remove the main method？use test case。

zhangthen · 2019-04-23T07:43:22Z

Plz add new config items to the nacos-config.txt too .

xingfudeshi

LGTM

XCXCXCXCX · 2019-04-24T06:17:29Z

Plz add new config items to the nacos-config.txt too .

OK.

zhangthen · 2019-04-24T06:33:02Z

@XCXCXCXCX

I have a mind.

It shoud be retry all the time until the rollback get success, this is the guarantee of final-consistency(BASE).
Stopping retries after a certain number of times may result in transactions not being rollbacked, which is contrary to the final consistency(BASE).

So is it necessary for this PR ?
If the rollback is not necessary to be executed, then why do you need distributed transaction of Seata?

slievrly · 2019-04-24T06:37:44Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

zhangthen · 2019-04-24T06:41:28Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

If retry stop, how to make the transaction to be final consistency ?

slievrly · 2019-04-24T06:44:01Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

If retry stop, how to make the transaction to be final consistency ?

If the physical failure of the database hangs, it is known that it cannot be started, and how to ensure consistency.

zhangthen · 2019-04-24T06:50:40Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

If retry stop, how to make the transaction to be final consistency ?

If the physical failure of the database hangs, it is known that it cannot be started, and how to ensure consistency.

If retry stop, how to make the transaction to be final consistency ?

I think it shoud allways retry, even if database down or application fails , unless there is a manual intervention to make the transaction to be final consistency， not stop retry automatically, must stop by manual-intervention.

In addition , without manual judgment, how to know the cause of failure, it is too hasty to stop retrying automatically after several failures.

slievrly · 2019-04-24T06:56:50Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

If retry stop, how to make the transaction to be final consistency ?

If the physical failure of the database hangs, it is known that it cannot be started, and how to ensure consistency.

If retry stop, how to make the transaction to be final consistency ?

I think it shoud allways retry, even if database down or application fails , unless there is a manual intervention to make the transaction to be final consistency， not stop retry automatically, must stop by manual-intervention.

In addition , without manual judgment, how to know the cause of failure, it is too hasty to stop retrying automatically after several failures.

This value defaults to permanent retry. When the user determines that this cannot be recovered because of a physical failure, the user can stop the retry by configuring this value.

CoffeeLatte007 · 2019-04-24T07:06:11Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

If retry stop, how to make the transaction to be final consistency ?

If the physical failure of the database hangs, it is known that it cannot be started, and how to ensure consistency.

If retry stop, how to make the transaction to be final consistency ?
I think it shoud allways retry, even if database down or application fails , unless there is a manual intervention to make the transaction to be final consistency， not stop retry automatically, must stop by manual-intervention.
In addition , without manual judgment, how to know the cause of failure, it is too hasty to stop retrying automatically after several failures.

This value defaults to permanent retry. When the user determines that this cannot be recovered because of a physical failure, the user can stop the retry by configuring this value.

I have a suggest, the retry provides a configuration，it is up to the user to decide whether to retry permanently.
If not permanent try again, then we can modify the retry mechanism, don't use Java ScheduledThreadPoolExecutor, we can use the Time Wheel, using an Exponential Backoff way retry

CoffeeLatte007 · 2019-04-24T07:12:28Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

If retry stop, how to make the transaction to be final consistency ?

If the physical failure of the database hangs, it is known that it cannot be started, and how to ensure consistency.

If retry stop, how to make the transaction to be final consistency ?
I think it shoud allways retry, even if database down or application fails , unless there is a manual intervention to make the transaction to be final consistency， not stop retry automatically, must stop by manual-intervention.
In addition , without manual judgment, how to know the cause of failure, it is too hasty to stop retrying automatically after several failures.

This value defaults to permanent retry. When the user determines that this cannot be recovered because of a physical failure, the user can stop the retry by configuring this value.

I have a suggest, the retry provides a configuration，it is up to the user to decide whether to retry permanently.
If not permanent try again, then we can modify the retry mechanism, don't use Java ScheduledThreadPoolExecutor, we can use the Time Wheel, using an Exponential Backoff way retry

zhangthen · 2019-04-24T07:12:34Z

@XCXCXCXCX @zhangthen This requirement is what I proposed. The default should be a permanent retry. When a database downtime or a single-point application fails to start, you need to stop retrying.

If retry stop, how to make the transaction to be final consistency ?

If the physical failure of the database hangs, it is known that it cannot be started, and how to ensure consistency.

If retry stop, how to make the transaction to be final consistency ?
I think it shoud allways retry, even if database down or application fails , unless there is a manual intervention to make the transaction to be final consistency， not stop retry automatically, must stop by manual-intervention.
In addition , without manual judgment, how to know the cause of failure, it is too hasty to stop retrying automatically after several failures.

This value defaults to permanent retry. When the user determines that this cannot be recovered because of a physical failure, the user can stop the retry by configuring this value.

I have a suggest, the retry provides a configuration，it is up to the user to decide whether to retry permanently.
If not permanent try again, then we can modify the retry mechanism, don't use Java ScheduledThreadPoolExecutor, we can use the Time Wheel, using an Exponential Backoff way retry

I think this implementation is too simple, will affect the rollback of all transactions；
The retry can not stop (unless stopped manually), which is the basic guarantee for the final consistency.
If too many failures, the retry frequency can be reduced appropriately.

XCXCXCXCX · 2019-04-24T07:38:42Z

@zhangthen @slievrly
So I think we can stop its retry through a management entry. When the user determines that a transaction cannot be recovered due to a physical failure, it can be marked with a new failure status (eg. CANCEL) and removed from the retrying list.
In addition, it is better to use Time Wheel for situations where there may be a lot of retry tasks.
@CoffeeLatte007

zhangthen · 2019-04-24T07:44:15Z

@zhangthen @slievrly
So I think we can stop its retry through a management entry. When the user determines that a transaction cannot be recovered due to a physical failure, it can be marked with a new failure status (eg. CANCEL) and removed from the retrying list.
In addition, it is better to use Time Wheel for situations where there may be a lot of retry tasks.
@CoffeeLatte007

I agree this solution.

CoffeeLatte007 · 2019-04-24T09:46:53Z

@zhangthen @slievrly
So I think we can stop its retry through a management entry. When the user determines that a transaction cannot be recovered due to a physical failure, it can be marked with a new failure status (eg. CANCEL) and removed from the retrying list.
In addition, it is better to use Time Wheel for situations where there may be a lot of retry tasks.
@CoffeeLatte007

yes, i think use time wheel and Exponential Backoff is better for retry

…elop

server/src/main/resources/file.conf

config/seata-config-core/src/main/java/io/seata/config/AbstractConfiguration.java

slievrly · 2019-05-10T10:02:17Z

@zhangthen Currently configured for permanent retry, this modification entry is reserved for higher-order users. After the webconsole is perfected, the console UI needs to join the manual stop retry button.

server/src/main/resources/file.conf

CoffeeLatte007

LGTM

leizhiyuan

LGTM

slievrly

LGTM

* add: add time limit when transaction retry on the server * fix: change int to long * add: Configuration api add getDuration() * add: copyright * fix: use test case instead main * add new config items to the nacos-config.txt * fix * parse -1 and default Duration.ofMillis(-1) * add comment

XCXCXCXCX added 5 commits April 11, 2019 09:23

Merge pull request #1 from seata/develop

1994de6

update

Merge branch 'develop' of https://github.com/seata/seata into develop

e9ae2c5

Merge pull request #2 from seata/develop

68afd1c

merge for name change

Merge branch 'develop' of https://github.com/XCXCXCXCX/seata into dev…

5941272

…elop

add: add time limit when transaction retry on the server

6219aa7

XCXCXCXCX and others added 5 commits April 18, 2019 13:48

fix: change int to long

942b1df

Merge branch 'develop' into develop

93d0a56

add: Configuration api add getDuration()

8a8aa4f

Merge remote-tracking branch 'origin/develop' into develop

d43cdc0

add: copyright

a2d9275

leizhiyuan reviewed Apr 19, 2019

View reviewed changes

XCXCXCXCX and others added 2 commits April 19, 2019 09:45

fix: use test case instead main

4717fd7

Merge branch 'develop' into develop

a0306c9

XCXCXCXCX and others added 3 commits April 23, 2019 19:29

add new config items to the nacos-config.txt

655cce7

Merge remote-tracking branch 'origin/develop' into develop

92f2000

Merge branch 'develop' into develop

ddbf4ed

xingfudeshi approved these changes Apr 24, 2019

View reviewed changes

CoffeeLatte007 closed this Apr 24, 2019

lovepoem and others added 5 commits May 7, 2019 16:42

Merge branch 'develop' into develop

2c9dda9

Merge branch 'develop' of https://github.com/seata/seata into develop

99f6f59

Merge branch 'develop' of https://github.com/XCXCXCXCX/seata into dev…

8149b0b

…elop

Merge branch 'develop' of https://github.com/seata/seata into develop

be1dd55

fix

bfef09c

slievrly reviewed May 10, 2019

View reviewed changes

server/src/main/resources/file.conf Outdated Show resolved Hide resolved

slievrly reviewed May 10, 2019

View reviewed changes

config/seata-config-core/src/main/java/io/seata/config/AbstractConfiguration.java Show resolved Hide resolved

parse -1 and default Duration.ofMillis(-1)

ef521a6

CoffeeLatte007 reviewed May 10, 2019

View reviewed changes

server/src/main/resources/file.conf Show resolved Hide resolved

CoffeeLatte007 and others added 3 commits May 11, 2019 12:30

Merge branch 'develop' into develop

3dc3a47

add comment

06a21e0

Merge remote-tracking branch 'origin/develop' into develop

a26c7d0

CoffeeLatte007 approved these changes May 13, 2019

View reviewed changes

xingfudeshi added 2 commits May 16, 2019 09:30

Merge branch 'develop' into develop

d57489b

Merge branch 'develop' into develop

1abf7e0

xingfudeshi requested review from slievrly and leizhiyuan and removed request for slievrly May 19, 2019 05:57

leizhiyuan approved these changes May 20, 2019

View reviewed changes

slievrly approved these changes May 21, 2019

View reviewed changes

Merge branch 'develop' into develop

b7af7dd

leizhiyuan merged commit 415e5c8 into apache:develop May 21, 2019

leizhiyuan added this to the 0.6.0 milestone May 21, 2019

fescar-robot mentioned this pull request May 24, 2019

[WeeklyReport] Weekly report for Seata 2019-5-17 to 2019-5-24 #1104

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add time limit when transaction retry on the server #824

add time limit when transaction retry on the server #824

XCXCXCXCX commented Apr 18, 2019

codecov-io commented Apr 18, 2019 •

edited

leizhiyuan Apr 19, 2019 •

edited

XCXCXCXCX Apr 19, 2019

zhangthen commented Apr 23, 2019

xingfudeshi left a comment

XCXCXCXCX commented Apr 24, 2019

zhangthen commented Apr 24, 2019 •

edited

slievrly commented Apr 24, 2019

zhangthen commented Apr 24, 2019

slievrly commented Apr 24, 2019

zhangthen commented Apr 24, 2019 •

edited

slievrly commented Apr 24, 2019

CoffeeLatte007 commented Apr 24, 2019

CoffeeLatte007 commented Apr 24, 2019

zhangthen commented Apr 24, 2019

XCXCXCXCX commented Apr 24, 2019

zhangthen commented Apr 24, 2019

CoffeeLatte007 commented Apr 24, 2019

slievrly commented May 10, 2019

CoffeeLatte007 left a comment

leizhiyuan left a comment

slievrly left a comment

add time limit when transaction retry on the server #824

add time limit when transaction retry on the server #824

Conversation

XCXCXCXCX commented Apr 18, 2019

Ⅰ. Describe what this PR did

Ⅱ. 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-io commented Apr 18, 2019 • edited

Codecov Report

leizhiyuan Apr 19, 2019 • edited

Choose a reason for hiding this comment

XCXCXCXCX Apr 19, 2019

Choose a reason for hiding this comment

zhangthen commented Apr 23, 2019

xingfudeshi left a comment

Choose a reason for hiding this comment

XCXCXCXCX commented Apr 24, 2019

zhangthen commented Apr 24, 2019 • edited

slievrly commented Apr 24, 2019

zhangthen commented Apr 24, 2019

slievrly commented Apr 24, 2019

zhangthen commented Apr 24, 2019 • edited

slievrly commented Apr 24, 2019

CoffeeLatte007 commented Apr 24, 2019

CoffeeLatte007 commented Apr 24, 2019

zhangthen commented Apr 24, 2019

XCXCXCXCX commented Apr 24, 2019

zhangthen commented Apr 24, 2019

CoffeeLatte007 commented Apr 24, 2019

slievrly commented May 10, 2019

CoffeeLatte007 left a comment

Choose a reason for hiding this comment

leizhiyuan left a comment

Choose a reason for hiding this comment

slievrly left a comment

Choose a reason for hiding this comment

codecov-io commented Apr 18, 2019 •

edited

leizhiyuan Apr 19, 2019 •

edited

zhangthen commented Apr 24, 2019 •

edited

zhangthen commented Apr 24, 2019 •

edited