[Wf-Diagnostics] add retry policy validation to diagnostics #6529
+134
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sankari165
requested review from
Shaddoll,
neil-xie,
davidporter-id-au,
Groxx,
shijiesheng,
jakobht,
3vilhamster,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
3vilhamster
reviewed
Nov 29, 2024
| @@ -85,9 +109,22 @@ func fetchStartedEvent(events []*types.HistoryEvent) *types.WorkflowExecutionSta | |||
| return nil | |||
| } | |||
|
|
|||
| func checkRetryPolicy(policy *types.RetryPolicy) IssueType { | |||
| if policy == nil { | |||
| return "" | |||
There was a problem hiding this comment.
Unsure if it is an issue - but missing retry policies for activities could be a good sign of issues - due to Cadence not being able to retry it on schedule to start/panic any timeout.
There was a problem hiding this comment.
yeah its. a grey area. I added it is an issue when an activity has heartbeating configured and no retry policy but havent raised missing retry policy by itself as an issue since I thought it would show up on a lot of workflows and might get ignored out of annoyance
3vilhamster
approved these changes
Nov 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changed?
Retry policy validation added to retry invariant in diagnostics
Why?
Customers have invalid policies running in production and hence cannot be stopped via server side validation but they still cause a lot of questions in support channel and hence it is better to convey via diagnostics
How did you test it?
unit tests
Potential risks
Release notes
Documentation Changes