Skip to content

Commit

Permalink
pgwire: add server.cockroach_cloud.max_client_connections_per_gateway…
Browse files Browse the repository at this point in the history
…_reason cluster setting

This setting can be used to customize the error message returned when
connections are denied due to a limit specified via
server.cockroach_cloud.max_client_connections_per_gateway.

This functionality is required for serverless. Being able to indicate to the
user why a limit is placed on their number of connections will support a better
UX. Specifically, indicating to the user which resource limits they have hit.
Part of: https://cockroachlabs.atlassian.net/browse/CC-9288

Release note: None
  • Loading branch information
alyshanjahani-crl committed Apr 10, 2023
1 parent c0d304b commit d3a9bf0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 3 deletions.
9 changes: 6 additions & 3 deletions pkg/sql/pgwire/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,13 @@ func (c *conn) checkMaxConnections(ctx context.Context, sqlServer *sql.Server) e
// serverless clusters.
maxNonRootConnectionsValue := maxNumNonRootConnections.Get(&sqlServer.GetExecutorConfig().Settings.SV)
if maxNonRootConnectionsValue >= 0 && sqlServer.GetNonRootConnectionCount() >= maxNonRootConnectionsValue {
// TODO(alyshan): Add another cluster setting that supplements the error message, so that
// clients can know *why* this ReadOnly setting is set and limiting their connections.
// Check if there is a reason to use in the error message.
msg := "cluster connections are limited"
if reason := maxNumNonRootConnectionsReason.Get(&sqlServer.GetExecutorConfig().Settings.SV); reason != "" {
msg = reason
}
return c.sendError(ctx, sqlServer.GetExecutorConfig(), errors.WithHintf(
pgerror.New(pgcode.TooManyConnections, "cluster connections are limited"),
pgerror.Newf(pgcode.TooManyConnections, "%s", msg),
"the maximum number of allowed connections is %d",
maxNonRootConnectionsValue,
))
Expand Down
20 changes: 20 additions & 0 deletions pkg/sql/pgwire/conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1946,6 +1946,13 @@ func TestPGWireRejectsNewConnIfTooManyConns(t *testing.T) {
require.NoError(t, err)
}

setMaxNonRootConnectionsReason := func(reason string) {
conn, cleanup := openConnWithUserSuccess(rootUser)
defer cleanup()
_, err := conn.Exec(ctx, "SET CLUSTER SETTING server.cockroach_cloud.max_client_connections_per_gateway_reason = $1", reason)
require.NoError(t, err)
}

createUser := func(user string, isAdmin bool) {
conn, cleanup := openConnWithUserSuccess(rootUser)
defer cleanup()
Expand Down Expand Up @@ -2053,6 +2060,19 @@ func TestPGWireRejectsNewConnIfTooManyConns(t *testing.T) {
nonAdminCleanup()
requireConnectionCount(t, 0)
})

t.Run("max_non_root_connections_reason", func(t *testing.T) {
setMaxNonRootConnections(0)
setMaxNonRootConnectionsReason("foobar")
requireConnectionCount(t, 0)
_, cleanup, err := openConnWithUser(admin)
defer cleanup()
require.Error(t, err)
var pgErr *pgconn.PgError
require.ErrorAs(t, err, &pgErr)
require.Equal(t, pgcode.TooManyConnections.String(), pgErr.Code)
require.Regexp(t, "foobar", pgErr.Error())
})
}

func TestConnCloseReleasesReservedMem(t *testing.T) {
Expand Down
13 changes: 13 additions & 0 deletions pkg/sql/pgwire/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,19 @@ var maxNumNonRootConnections = settings.RegisterIntSetting(
-1,
)

// maxNumNonRootConnectionsReason is used to supplement the error message for connections that denied due to
// server.max_non_root_connections_per_gateway.
// Note(alyshan): This setting is not public. It is intended to be used by Cockroach Cloud when limiting
// connections to serverless clusters.
// This setting may be removed one day.
var maxNumNonRootConnectionsReason = settings.RegisterStringSetting(
settings.TenantWritable,
"server.cockroach_cloud.max_client_connections_per_gateway_reason",
"a reason to provide in the error message for connections that are denied due to "+
"server.cockroach_cloud.max_client_connections_per_gateway",
"",
)

const (
// ErrSSLRequired is returned when a client attempts to connect to a
// secure server in cleartext.
Expand Down

0 comments on commit d3a9bf0

Please sign in to comment.