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
MaxRequestsPerConn cluster param was added #1747
base: master
Are you sure you want to change the base?
MaxRequestsPerConn cluster param was added #1747
Conversation
@martin-sucha Hello, will be glad to receive your review on this PR! |
conn.go
Outdated
@@ -257,7 +257,7 @@ func (s *Session) dialWithoutObserver(ctx context.Context, host *HostInfo, cfg * | |||
errorHandler: errorHandler, | |||
compressor: cfg.Compressor, | |||
session: s, | |||
streams: streams.New(cfg.ProtoVersion), | |||
streams: s.streamIDGenerator(cfg.ProtoVersion), |
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.
what if we call it newStreamIDGenerator
?
Also is it obligatory to pass config there if we access cfg from attached type?
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.
Fixed, renamed and moved the function to the streams
package.
defer h.mu.Unlock() | ||
addr, _ := h.connectAddressLocked() | ||
return net.JoinHostPort(addr.String(), strconv.Itoa(h.port)) | ||
h.mu.Lock() |
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.
I don't think it makes sense to format files which were not touched by the fix.
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.
3c070c3
to
ba0fac8
Compare
ba0fac8
to
88405bb
Compare
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.
LGTM
This PR allows a user to configure the maximum requests per connection parameter by a newly added MaxRequestsPerConn option.
Closes #1679
Functionality was adopted from similar PR from the ScyllaDB project: scylladb#113