-
Notifications
You must be signed in to change notification settings - Fork 805
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
Adding a framework for the Task validator service. #5446
Merged
agautam478
merged 7 commits into
cadence-workflow:master
from
agautam478:task_validator
Nov 13, 2023
Merged
Adding a framework for the Task validator service. #5446
agautam478
merged 7 commits into
cadence-workflow:master
from
agautam478:task_validator
Nov 13, 2023
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
neil-xie
reviewed
Nov 9, 2023
agautam478
requested review from
neil-xie,
allenchen2244 and
taylanisikdemir
November 13, 2023 17:38
taylanisikdemir
approved these changes
Nov 13, 2023
func (w *checkerImpl) WorkflowCheckforValidation(workflowID string, domainID string, runID string) error { | ||
// Emitting just the log to ensure that the workflow is called for now. | ||
// TODO: add some validations to check the wf for corruptions. | ||
w.logger.Info(fmt.Sprintf("WorkflowCheckforValidation. DomainID: %v, WorkflowID: %v, RunID: %v", |
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.
the common pattern is to write a static message with log tags for variable stuff.
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?
The primary goal of this framework is to establish a reliable mechanism for detecting corruption during task processing within the Cadence system. It aims to create a versatile task validation framework applicable throughout the task processing environment. Various validation checks, such as StaleWorkflowCheck and DeprecateDomainCheck, will be introduced to enhance the framework.
The current pull request comprises the fundamental structure of the service, accompanied by its corresponding unit test.
In scenarios where we identify a synchronous API call error from the worker, indicating the completion of a decision task, we occasionally encounter the error "maximum attempts exceeded to update history." This situation proves to be detrimental, as the decision task enters an endless retry loop, causing the entire task list to become stuck.
err := h.GetTaskValidator().WorkflowCheckforValidation(workflowID, domainID, "") if err != nil { return err }
To address this, we are testing the invocation of the function call on this reported decision task to observe whether the logs are generated as expected.
Why?
These changes were made to ensure a slow incremental introduction of the taskvalidator in the system. We will proceed forward once we have verified that the logs generate as expected.
How did you test it?
Added a unit test.
Tested locally.
Potential risks
No potential risks.
Release notes
NA
Documentation Changes
NA