worryg0d commented on code in PR #1920:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1920#discussion_r2572099372
##########
protocol_negotiation_test.go:
##########
@@ -0,0 +1,179 @@
+//go:build all || unit
+// +build all unit
+
+package gocql
+
+import (
+ "context"
+ "encoding/binary"
+ "fmt"
+ "slices"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/require"
+)
+
+type requestHandlerForProtocolNegotiationTest struct {
+ supportedProtocolVersions []protoVersion
+ supportedBetaProtocols []protoVersion
+}
+
+func (r *requestHandlerForProtocolNegotiationTest)
supportsBetaProtocol(version protoVersion) bool {
+ return slices.Contains(r.supportedBetaProtocols, version)
+}
+
+func (r *requestHandlerForProtocolNegotiationTest) supportsProtocol(version
protoVersion) bool {
+ return slices.Contains(r.supportedProtocolVersions, version)
+}
+
+func (r *requestHandlerForProtocolNegotiationTest) hasBetaFlag(header
*frameHeader) bool {
+ return header.flags&flagBetaProtocol == flagBetaProtocol
+}
+
+func (r *requestHandlerForProtocolNegotiationTest)
createBetaFlagUnsetProtocolErrorMessage(version protoVersion) string {
+ return fmt.Sprintf("Beta version of the protocol used (%d/v%d-beta),
but USE_BETA flag is unset", version, version)
+}
+
+func (r *requestHandlerForProtocolNegotiationTest) handle(_ *TestServer,
reqFrame, respFrame *framer) error {
+ // If a client uses beta protocol, but the USE_BETA flag is not set, we
respond with an error
+ if r.supportsBetaProtocol(reqFrame.header.version) &&
!r.hasBetaFlag(reqFrame.header) {
+ respFrame.writeHeader(0, opError, reqFrame.header.stream)
+ respFrame.writeInt(ErrCodeProtocol)
+
respFrame.writeString(r.createBetaFlagUnsetProtocolErrorMessage(reqFrame.header.version))
+ return nil
+ }
+
+ // if a client uses an unsupported protocol version, we respond with an
error
+ if !r.supportsProtocol(reqFrame.header.version) {
+ respFrame.writeHeader(0, opError, reqFrame.header.stream)
Review Comment:
Could you please clarify what you exactly mean? The test server already
replies with the stream id of the request
##########
control.go:
##########
@@ -262,26 +217,27 @@ func (c *controlConn) discoverProtocol(hosts []*HostInfo)
(int, error) {
var err error
for _, host := range hosts {
- var conn *Conn
- conn, err = c.session.dial(c.session.ctx, host, &connCfg,
handler)
- if conn != nil {
- conn.Close()
- }
+ connCfg := *c.session.connCfg
+ for proto := highestProtocolVersionSupported; proto >=
lowestProtocolVersionSupported; proto-- {
+ connCfg.ProtoVersion = proto
+
+ var conn *Conn
+ conn, err = c.session.dial(c.session.ctx, host,
&connCfg, handler)
+ if conn != nil {
+ conn.Close()
+ }
- if err == nil {
- c.session.logger.Debug("Discovered protocol version
using host.",
- NewLogFieldInt("protocol_version",
connCfg.ProtoVersion), NewLogFieldIP("host_addr", host.ConnectAddress()),
NewLogFieldString("host_id", host.HostID()))
- return connCfg.ProtoVersion, nil
- }
+ if err == nil {
+ c.session.logger.Debug("Discovered protocol
version using host.",
+ NewLogFieldInt("protocol_version",
connCfg.ProtoVersion), NewLogFieldIP("host_addr", host.ConnectAddress()))
+ return connCfg.ProtoVersion, nil
+ }
- if proto := parseProtocolFromError(err); proto > 0 {
- c.session.logger.Debug("Discovered protocol version
using host after parsing protocol error.",
- NewLogFieldInt("protocol_version", proto),
NewLogFieldIP("host_addr", host.ConnectAddress()), NewLogFieldString("host_id",
host.HostID()))
- return proto, nil
+ c.session.logger.Debug("Failed to connect to the host
using protocol version.",
Review Comment:
That makes sense, we don't really have to re-try each time if the error is
not protocol-related
##########
protocol_negotiation_test.go:
##########
@@ -0,0 +1,179 @@
+//go:build all || unit
+// +build all unit
+
+package gocql
Review Comment:
Good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
