[SCB-962] Support ForwardPolicy to configure the maximum of retries#322
[SCB-962] Support ForwardPolicy to configure the maximum of retries#322zhfeng merged 1 commit intoapache:masterfrom KomachiSion:SCB-962
Conversation
| - compensation - user-defined compensation that executed by the Saga. | ||
| - method - user-defined, HTTP method. | ||
| - path - user-defined, HTTP path. | ||
| - retries - int, optional, default 3. The max retry times for compensation. |
There was a problem hiding this comment.
Does it support -1 setting?
There was a problem hiding this comment.
When compensation set retries less than or equal to 0, it will be set default value.
This is the logic that was there, I have not changed it.
| } | ||
| throw new TransactionAbortedException( | ||
| String.format( | ||
| "Too many failures in transaction %s of service %s, stop transaction!", |
There was a problem hiding this comment.
How about using "abort the transaction"?
|
|
||
| @Override | ||
| public int retries() { | ||
| return 0; |
There was a problem hiding this comment.
why it return 0 here ? it means to retry infinite ?
There was a problem hiding this comment.
No, Fallback does not retry currently.
Because I abstracted the retires method from Compensation to Operation, Fallback needs to implement this method temporarily.
Considering the default number of retries if Fallback needs to retry in the future.
| } | ||
|
|
||
| private boolean isRetryable(int i, Transaction transaction) { | ||
| return transaction.retries() == Transaction.INFINITE_RETRY || i <= transaction.retries(); |
There was a problem hiding this comment.
I think it could be an issue here. if the transaction.retries() is 0, it just run sout of the loop and has no retry at all. But you mention in the document
If this parameter is less than or equal to 0, transaction will retry infinitely.
There was a problem hiding this comment.
When Transaction is generated from Json and retries is 0, retires will be set to Transaction.INFINITE_RETRY.
The correlation can be viewed from JacksonSQLTransaction or JacksonRestTransaction.
Of course, the problem you said does exist, I fix it right now.
There was a problem hiding this comment.
Thanks @KomachiSion , as i starts from 0
for(int i = 0; isRetryable(i, request.transaction()); i++)
it looks like the condition should be i < transcaction.retries() ?
|
@KomachiSion Thanks for your contribution and can you check the travis-ci message at first. We have to make sure the all the tests pass. |
|
@KomachiSion How to config the retries parameter globally as we discussed in the mailing list ? I think it could be possible to make the DEFAULT_RETRIES configurable ? Also can you squash these changes to one commit ? |
|
I think it may not be reasonable to configure retries parameter globally. Because the retry features of transaction, compensation, and fallback may be inconsistent. If you need to configure global retries parameter, it may not be compatible with multiple cases. About squashing commit, I execute as soon as possible. |
SCB-962 ForwardRecovery support the maximum of retries. SCB-962 Add ForwardRecovery retry unit case. SCB-962 Implement retries in Transcation unit case. SCB-962 Implement retries for Rest and SQL Transaction. SCB-962 Abstract retries to Operation. SCB-962 Implement retries for Rest and SQL Operation. SCB-962 Move retries from transaction/compensation to operation. SCB-962 Abort transaction while reaching the max of retries. SCB-962 Update api docs for retries SCB-962 Catch and throw Aborted exception in consumer SCB-962 Add unit case for maximum retries SCB-962 Fix default value of transaction retries SCB-962 Change description when transaction aborted SCB-962 Fix transaction does not retry when transaction.retires = 0
|
Thanks @KomachiSion and it looks good to me. @WillemJiang do you have any input here ? |
|
@KomachiSion Thanks for your contribution ! |
4 participants
Support ForwardPolicy to configure the maximum of retries