Skip to content
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

Remove frontend handler retries #3285

Merged

Conversation

alexshtin
Copy link
Member

What changed?
Remove frontend handler retries.

Why?
Retries on frontend handler level may lead to unexpected InvalidArgument errors. Closes #3282.

How did you test it?
Existing tests.

Potential risks
No risks.

Is hotfix candidate?
Yes.

@alexshtin alexshtin requested a review from a team as a code owner August 30, 2022 19:17
@alexshtin alexshtin requested a review from yycptt August 30, 2022 19:17
Copy link
Member

@yycptt yycptt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe only land to 1.17 release branch for now? And pick to master after our discussion about the long term solution tomorrow?

@alexshtin
Copy link
Member Author

We decided not to do this. All commits should go on master branch first and then get cherry picked to release branches to simplify downstream merges.

Comment on lines +66 to +69
historyClientRetryMaxAttempts = 5

matchingClientRetryInitialInterval = 1000 * time.Millisecond
matchingClientRetryMaxAttempts = 2
matchingClientRetryMaxAttempts = 5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doing this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we don't retry on handler level anymore, it makes sense to increase number of attempts from client themselves. 2 is too small for overall retry attempts.

@alexshtin alexshtin merged commit 94abd6c into temporalio:master Aug 30, 2022
@alexshtin alexshtin deleted the feature/remove-frontend-handler-retry branch August 30, 2022 23:27
alexshtin added a commit that referenced this pull request Aug 30, 2022
alexshtin added a commit to alexshtin/temporal that referenced this pull request Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frontend handler retries may lead to InvalidArgument errors when starting new workflow
3 participants