-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Save last RetryStatus for retryable failures #432
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
Save last RetryStatus for retryable failures #432
Conversation
| // TODO(maxim): is decisionTaskCompletedID the correct id? | ||
| // TODO(maxim): should we introduce new TimeoutTypes (Workflow, Run) for workflows? |
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.
Should I remove this comment?
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.
yes
| break Loop | ||
| } | ||
|
|
||
| if timerSequenceID.timerType != timerTypeScheduleToStart { |
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.
I removed this check because RetryActivity below will call IsRetryable and it will return NonRetryableFailure for ScheduleToStart.
| if retryStatus == commonpb.RetryStatus_Timeout { | ||
| timeoutFailure.GetTimeoutFailureInfo().TimeoutType = commonpb.TimeoutType_ScheduleToClose |
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.
This is how I change timeout type for timed out retries.
| // TODO(maxim): is decisionTaskCompletedID the correct id? | ||
| // TODO(maxim): should we introduce new TimeoutTypes (Workflow, Run) for workflows? |
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.
yes
What changed?
getBackoffIntevalbesides interval itself, now returns current retry status fromcommonpb.RetryStatusenum.Why?
RetryStatusis stored in history and can be used for debugging. It also allows to generate more accurate timeout errors.How did you test it?
All tests.
Potential risks
No risks.