worryg0d commented on code in PR #1955:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1955#discussion_r3347253850
##########
integration_test.go:
##########
@@ -977,3 +977,54 @@ func TestSliceMapMapScanCollectionTypes(t *testing.T) {
})
}
}
+
+// TestSmallTimeoutNoPoolErrors verifies that small Session.Timeout values
+// don't cause connections to timeout and reconnect constantly. This is a
+// regression test for
https://github.com/apache/cassandra-gocql-driver/issues/1919
+//
+// The issue was that the timeout was being applied to frame header reads,
+// causing connections to timeout while waiting for the next frame. The fix
+// ensures frame headers are read without timeout, while frame bodies are
+// read with timeout.
+func TestSmallTimeoutNoPoolErrors(t *testing.T) {
+ // Create a test logger to capture log messages
+ logger := newTestLogger(LogLevelDebug)
+ defer func() {
+ t.Log(logger.String())
+ }()
+
+ cluster := createCluster()
+ cluster.ConnectTimeout = 10 * time.Second
+ cluster.Timeout = 750 * time.Millisecond
+ cluster.NumConns = 1
+ cluster.Logger = logger
+
+ db, err := cluster.CreateSession()
+ if err != nil {
+ t.Fatalf("CreateSession: %v", err)
+ }
+ defer db.Close()
+
+ // Wait for connections to sit idle
+ // If the bug exists, connections will timeout while waiting for frame
headers
+ // and "Pool connection error" messages will be logged repeatedly
+ time.Sleep(5 * time.Second)
+
+ // Get log output for analysis
+ logOutput := strings.ToLower(logger.String())
+
+ // Count successful connection messages - should be exactly NumConns *
number of nodes
+ connectedCount := strings.Count(logOutput, "pool connected to node")
+ if connectedCount != *clusterSize*cluster.NumConns {
+ t.Fatalf("Expected exactly 2 'Pool connected to node' messages,
got %d:\n%s",
+ connectedCount, logOutput)
Review Comment:
nit: expected exactly *clusterSize *cluster.NumConns - there are 3 node
clusters in our CI, so it would assert for 3 conns
##########
conn.go:
##########
@@ -908,6 +929,8 @@ func (c *connReader) Read(p []byte) (n int, err error) {
var nn int
if c.timeout > 0 {
c.conn.SetReadDeadline(time.Now().Add(c.timeout))
+ } else if c.timeout == 0 {
+ c.conn.SetReadDeadline(time.Time{})
}
Review Comment:
It fails `TestSteam0` because the underlying conn is nil
##########
conn_test.go:
##########
@@ -306,12 +308,18 @@ func TestTimeout(t *testing.T) {
}
}()
- if err := db.Query("kill").WithContext(ctx).Exec(); err == nil {
+ now := time.Now()
+ err = db.Query("timeout").WithContext(ctx).Exec()
Review Comment:
At some point, we would have to get rid of WithContext usage as it is
deprected
--
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]
