-
Notifications
You must be signed in to change notification settings - Fork 6
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
Implement retry mechanism for read endpoints #31
base: dev
Are you sure you want to change the base?
Conversation
…d update vulnerable dependencies
lgtm (except the retry code duplication - not sure if there is a way to make that nicer) |
@qubicmio I addressed that in the review |
foundation/rpc_server/rpc_server.go
Outdated
@@ -37,8 +37,7 @@ type Server struct { | |||
readRetryCount int | |||
} | |||
|
|||
func NewServer(listenAddrGRPC, listenAddrHTTP string, logger *log.Logger, | |||
qPool *qubic.Pool, maxTickFetchUrl string, readRetryCount int) *Server { | |||
func NewServer(listenAddrGRPC, listenAddrHTTP string, logger *log.Logger, qPool *qubic.Pool, maxTickFetchUrl string, readRetryCount int) *Server { |
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.
readRetryCount is not a proper variable time as from a config point of view it's not a count but a readMaxRetries
foundation/rpc_server/rpc_server.go
Outdated
return 0, errors.Wrap(err, "performing request") | ||
} | ||
defer res.Body.Close() | ||
for attempt := 0; attempt <= maxRetries; attempt++ { |
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.
shouldn't this be attempt < maxRetries
app/grpc_server/main.go
Outdated
@@ -32,6 +32,7 @@ func run(logger *log.Logger) error { | |||
HttpHost string `conf:"default:0.0.0.0:8000"` | |||
GrpcHost string `conf:"default:0.0.0.0:8001"` | |||
MaxTickFetchUrl string `conf:"default:http://127.0.0.1:8080/max-tick"` | |||
ReadRetryCount int `conf:"default:5"` |
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.
same here, it's not a count
Add retry mechanism for all endpoints that request data from nodes and update vulnerable dependencies.