Skip to content

Commit 46c2126

Browse files
craig[bot]rafiss
andcommitted
Merge #155656
155656: pgwire: make max repeated error count configurable via cluster setting r=rafiss a=rafiss Previously, the maximum number of repeated network read errors before aborting a connection was a hardcoded constant set to 256 (1 << 8). This change makes the value configurable via a non-public cluster setting `sql.pgwire.max_repeated_error_count`. This allows operators to tune the threshold for aborting connections experiencing repeated network errors, and allows us to backport this change along with 5f562ad. Epic: None Release note: None Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
2 parents 09c6ae2 + 340101c commit 46c2126

File tree

2 files changed

+16
-9
lines changed

2 files changed

+16
-9
lines changed

pkg/sql/pgwire/conn_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,8 +1449,11 @@ func TestConnServerAbortsOnRepeatedErrors(t *testing.T) {
14491449
conn, err := db.Conn(ctx)
14501450
require.NoError(t, err)
14511451

1452+
// Get the current value of the cluster setting.
1453+
maxErrors := int(maxRepeatedErrorCount.Get(&srv.ClusterSettings().SV))
1454+
14521455
atomic.StoreUint32(&shouldError, 1)
1453-
for i := 0; i < maxRepeatedErrorCount+100; i++ {
1456+
for i := 0; i < maxErrors+100; i++ {
14541457
var s int
14551458
err := conn.QueryRowContext(ctx, "SELECT 1").Scan(&s)
14561459
if err != nil {
@@ -1459,9 +1462,9 @@ func TestConnServerAbortsOnRepeatedErrors(t *testing.T) {
14591462
}
14601463
if errors.Is(err, driver.ErrBadConn) {
14611464
// The server closed the connection, which is what we want!
1462-
require.GreaterOrEqualf(t, i, maxRepeatedErrorCount,
1465+
require.GreaterOrEqualf(t, i, maxErrors,
14631466
"the server should have aborted after seeing %d errors",
1464-
maxRepeatedErrorCount,
1467+
maxErrors,
14651468
)
14661469
return
14671470
}

pkg/sql/pgwire/server.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ var logVerboseSessionAuth = settings.RegisterBoolSetting(
9494
false,
9595
settings.WithPublic)
9696

97+
var maxRepeatedErrorCount = settings.RegisterIntSetting(
98+
settings.ApplicationLevel,
99+
"sql.pgwire.max_repeated_error_count",
100+
"the maximum number of times an error can be received while reading from a "+
101+
"network connection before the server aborts the connection",
102+
1<<8, // 256
103+
settings.PositiveInt,
104+
)
105+
97106
const (
98107
// ErrSSLRequired is returned when a client attempts to connect to a
99108
// secure server in cleartext.
@@ -1079,11 +1088,6 @@ func (s *Server) newConn(
10791088
return c
10801089
}
10811090

1082-
// maxRepeatedErrorCount is the number of times an error can be received
1083-
// while reading from the network connection before the server decides to give
1084-
// up and abort the connection.
1085-
const maxRepeatedErrorCount = 1 << 8
1086-
10871091
// serveImpl continuously reads from the network connection and pushes execution
10881092
// instructions into a sql.StmtBuf, from where they'll be processed by a command
10891093
// "processor" goroutine (a connExecutor).
@@ -1455,7 +1459,7 @@ func (s *Server) serveImpl(
14551459
// 3. we reached an arbitrary threshold of repeated errors.
14561460
if netutil.IsClosedConnection(err) ||
14571461
errors.Is(err, context.Canceled) ||
1458-
repeatedErrorCount > maxRepeatedErrorCount {
1462+
repeatedErrorCount > int(maxRepeatedErrorCount.Get(&s.execCfg.Settings.SV)) {
14591463
break
14601464
}
14611465
} else {

0 commit comments

Comments
 (0)