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

Add retry in some frontend API #631

Merged
merged 1 commit into from
Mar 28, 2018
Merged

Add retry in some frontend API #631

merged 1 commit into from
Mar 28, 2018

Conversation

vancexu
Copy link
Contributor

@vancexu vancexu commented Mar 24, 2018

Oncall observed multiple pollForActivity/Decision error in short period which affects our SLA.

Error log shows as:
handler.go:385 PollForDecisionTask failed. TaskList: adjust_ledgerv2, Error: code:unknown message:received unknown error calling service: "cadence-matching", procedure: "MatchingService::PollForDecisionTask", err: dial tcp 10.189.33.24:31849: getsockopt: connection refused

We need to add retry logic in Frontend to mitigate such error affecting our SLA to client.

@vancexu vancexu requested review from samarabbas and wxing1292 March 24, 2018 06:46
@coveralls
Copy link

coveralls commented Mar 26, 2018

Coverage Status

Coverage decreased (-0.02%) to 63.249% when pulling c23492c on retry into 3a65572 on master.

common/util.go Outdated
@@ -42,6 +42,10 @@ const (
historyServiceOperationInitialInterval = 50 * time.Millisecond
historyServiceOperationMaxInterval = 10 * time.Second
historyServiceOperationExpirationInterval = 30 * time.Second

frontendServiceOperationInitialInterval = 50 * time.Millisecond
Copy link
Contributor

Choose a reason for hiding this comment

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

personal preference: 50 is too low, 200 maybe a good idea

@vancexu vancexu merged commit 473c524 into master Mar 28, 2018
@vancexu vancexu deleted the retry branch March 28, 2018 00:02
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.

3 participants