diff --git a/pkg/server/server_controller_sql.go b/pkg/server/server_controller_sql.go index 218e7e81a24d..0c1d112494c7 100644 --- a/pkg/server/server_controller_sql.go +++ b/pkg/server/server_controller_sql.go @@ -50,6 +50,10 @@ func (c *serverController) sqlMux( // The concurrent dispatch gives a chance to the succeeding // servers to see and process the cancel at approximately the // same time as every other. + // + // Cancel requests are unauthenticated so run the cancel async to prevent + // the client from deriving any info about the cancel based on how long it + // takes. if err := c.stopper.RunAsyncTask(ctx, "cancel", func(ctx context.Context) { s.handleCancel(ctx, status.CancelKey) }); err != nil { diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index a42a1dca5a7e..82bd026a9666 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -833,8 +833,12 @@ func (s *SQLServerWrapper) serveConn( pgServer := s.PGServer() switch status.State { case pgwire.PreServeCancel: - pgServer.HandleCancel(ctx, status.CancelKey) - return nil + // Cancel requests are unauthenticated so run the cancel async to prevent + // the client from deriving any info about the cancel based on how long it + // takes. + return s.stopper.RunAsyncTask(ctx, "cancel", func(ctx context.Context) { + pgServer.HandleCancel(ctx, status.CancelKey) + }) case pgwire.PreServeReady: return pgServer.ServeConn(ctx, conn, status) default: diff --git a/pkg/sql/pgwire/pre_serve.go b/pkg/sql/pgwire/pre_serve.go index 967bec8ebe89..37b9ae56281a 100644 --- a/pkg/sql/pgwire/pre_serve.go +++ b/pkg/sql/pgwire/pre_serve.go @@ -217,7 +217,6 @@ func (s *PreServeConnHandler) SendRoutingError( `Double check your "-ccluster=" connection option or your "cluster:" database name prefix.`) _ = s.sendErr(ctx, s.st, conn, err) - _ = conn.Close() } // sendErr sends errors to the client during the connection startup @@ -235,7 +234,6 @@ func (s *PreServeConnHandler) sendErr( // receive error payload are highly correlated with clients // disconnecting abruptly. _ /* err */ = w.writeErr(ctx, err, conn) - _ = conn.Close() return err } @@ -324,7 +322,7 @@ func (s *PreServeConnHandler) PreServe( case versionCancel: // The cancel message is rather peculiar: it is sent without // authentication, always over an unencrypted channel. - if ok, key := readCancelKeyAndCloseConn(ctx, conn, &buf); ok { + if ok, key := readCancelKey(ctx, &buf); ok { return conn, PreServeStatus{ State: PreServeCancel, CancelKey: key, @@ -374,7 +372,7 @@ func (s *PreServeConnHandler) PreServe( // Yet, we've found clients in the wild that send the cancel // after the TLS handshake, for example at // https://github.com/cockroachlabs/support/issues/600. - if ok, key := readCancelKeyAndCloseConn(ctx, conn, &buf); ok { + if ok, key := readCancelKey(ctx, &buf); ok { return conn, PreServeStatus{ State: PreServeCancel, CancelKey: key, diff --git a/pkg/sql/pgwire/server.go b/pkg/sql/pgwire/server.go index 4a9c5dc2dfd0..a7b9942ac017 100644 --- a/pkg/sql/pgwire/server.go +++ b/pkg/sql/pgwire/server.go @@ -928,8 +928,6 @@ func (s *Server) serveImpl( authOpt authOptions, sessionID clusterunique.ID, ) { - defer func() { _ = c.conn.Close() }() - if c.sessionArgs.User.IsRootUser() || c.sessionArgs.User.IsNodeUser() { ctx = logtags.AddTag(ctx, "user", redact.Safe(c.sessionArgs.User)) } else { @@ -1253,18 +1251,13 @@ func (s *Server) serveImpl( } } -// readCancelKeyAndCloseConn retrieves the "backend data" key that identifies +// readCancelKey retrieves the "backend data" key that identifies // a cancellable query, then closes the connection. -func readCancelKeyAndCloseConn( - ctx context.Context, conn net.Conn, buf *pgwirebase.ReadBuffer, +func readCancelKey( + ctx context.Context, buf *pgwirebase.ReadBuffer, ) (ok bool, cancelKey pgwirecancel.BackendKeyData) { telemetry.Inc(sqltelemetry.CancelRequestCounter) backendKeyDataBits, err := buf.GetUint64() - // The connection that issued the cancel is not a SQL session -- it's an - // entirely new connection that's created just to send the cancel. We close - // the connection as soon as possible after reading the data, since there - // is nothing to send back to the client. - _ = conn.Close() // The client is also unwilling to read an error payload, so we just log it locally. if err != nil { log.Sessions.Warningf(ctx, "%v", errors.Wrap(err, "reading cancel key from client")) @@ -1378,7 +1371,6 @@ func (s *Server) sendErr( // receive error payload are highly correlated with clients // disconnecting abruptly. _ /* err */ = w.writeErr(ctx, err, conn) - _ = conn.Close() return err } diff --git a/pkg/util/netutil/net.go b/pkg/util/netutil/net.go index 76cad5b855c3..6c11c7089254 100644 --- a/pkg/util/netutil/net.go +++ b/pkg/util/netutil/net.go @@ -183,6 +183,9 @@ func (s *TCPServer) ServeWith( } tempDelay = 0 err := s.stopper.RunAsyncTask(ctx, "tcp-serve", func(ctx context.Context) { + defer func() { + _ = rw.Close() + }() s.addConn(rw) defer s.rmConn(rw) serveConn(ctx, rw)