Skip to content

Commit

Permalink
sql: make multitenancy errors report to telemetry
Browse files Browse the repository at this point in the history
This change ensures that multitenancy errors get collected in
telemetry.

By also making the issue number non-optional, this patch also catches
a few more cases that were missing open issues.

Release note: None
  • Loading branch information
knz committed Sep 14, 2020
1 parent e53b17a commit 3fcafa8
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 31 deletions.
8 changes: 4 additions & 4 deletions pkg/sql/opt_exec_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -1692,7 +1692,7 @@ func (ef *execFactory) ConstructAlterTableSplit(
index cat.Index, input exec.Node, expiration tree.TypedExpr,
) (exec.Node, error) {
if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
return nil, errorutil.UnsupportedWithMultiTenancy()
return nil, errorutil.UnsupportedWithMultiTenancy(54254)
}

expirationTime, err := parseExpirationTime(ef.planner.EvalContext(), expiration)
Expand All @@ -1713,7 +1713,7 @@ func (ef *execFactory) ConstructAlterTableUnsplit(
index cat.Index, input exec.Node,
) (exec.Node, error) {
if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
return nil, errorutil.UnsupportedWithMultiTenancy()
return nil, errorutil.UnsupportedWithMultiTenancy(54254)
}

return &unsplitNode{
Expand All @@ -1726,7 +1726,7 @@ func (ef *execFactory) ConstructAlterTableUnsplit(
// ConstructAlterTableUnsplitAll is part of the exec.Factory interface.
func (ef *execFactory) ConstructAlterTableUnsplitAll(index cat.Index) (exec.Node, error) {
if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
return nil, errorutil.UnsupportedWithMultiTenancy()
return nil, errorutil.UnsupportedWithMultiTenancy(54254)
}

return &unsplitAllNode{
Expand All @@ -1740,7 +1740,7 @@ func (ef *execFactory) ConstructAlterTableRelocate(
index cat.Index, input exec.Node, relocateLease bool,
) (exec.Node, error) {
if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
return nil, errorutil.UnsupportedWithMultiTenancy()
return nil, errorutil.UnsupportedWithMultiTenancy(54250)
}

return &relocateNode{
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/scatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type scatterNode struct {
// Privileges: INSERT on table.
func (p *planner) Scatter(ctx context.Context, n *tree.Scatter) (planNode, error) {
if !p.ExecCfg().Codec.ForSystemTenant() {
return nil, errorutil.UnsupportedWithMultiTenancy()
return nil, errorutil.UnsupportedWithMultiTenancy(54255)
}

tableDesc, index, err := p.getTableAndIndex(ctx, &n.TableOrIndex, privilege.INSERT)
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/sem/builtins/generator_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,8 @@ func makeCheckConsistencyGenerator(
ctx *tree.EvalContext, args tree.Datums,
) (tree.ValueGenerator, error) {
if !ctx.Codec.ForSystemTenant() {
return nil, errorutil.UnsupportedWithMultiTenancy()
return nil, errorutil.UnsupportedWithMultiTenancy(
errorutil.FeatureNotAvailableToNonSystemTenantsIssue)
}

keyFrom := roachpb.Key(*args[1].(*tree.DBytes))
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/show_zone_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ const (

func (p *planner) ShowZoneConfig(ctx context.Context, n *tree.ShowZoneConfig) (planNode, error) {
if !p.ExecCfg().Codec.ForSystemTenant() {
return nil, errorutil.UnsupportedWithMultiTenancy()
return nil, errorutil.UnsupportedWithMultiTenancy(multitenancyZoneCfgIssueNo)
}

return &delayedNode{
Expand Down
30 changes: 11 additions & 19 deletions pkg/util/errorutil/tenant.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,20 @@

package errorutil

import (
"fmt"
"strings"

"github.com/cockroachdb/errors"
)
import "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"

// UnsupportedWithMultiTenancy returns an error suitable for returning when an
// operation could not be carried out due to the SQL server running in
// multi-tenancy mode. In that mode, Gossip and other components of the KV layer
// are not available.
func UnsupportedWithMultiTenancy(issues ...int) error {
var buf strings.Builder
buf.WriteString("operation is unsupported in multi-tenancy mode")
if len(issues) > 0 {
buf.WriteString("; see:\n")
const prefix = "\nhttps://github.com/cockroachdb/cockroach/issues/"
for _, n := range issues {
fmt.Fprint(&buf, prefix, n)
}
}
// TODO(knz): This should be done in a different way,
// see: https://github.com/cockroachdb/cockroach/issues/48164
return errors.Newf("%s", buf.String())
func UnsupportedWithMultiTenancy(issue int) error {
return unimplemented.NewWithIssue(issue, "operation is unsupported in multi-tenancy mode")
}

// FeatureNotAvailableToNonSystemTenantsIssue is to be used with the
// Optional and related error interfaces when a feature is simply not
// available to non-system tenants (i.e. we're not planning to change
// this).
// For all other multitenancy errors where there is a plan to
// improve the situation, a specific issue should be created instead.
const FeatureNotAvailableToNonSystemTenantsIssue = 54252
5 changes: 0 additions & 5 deletions pkg/util/errorutil/tenant_deprecated_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,3 @@ func (w TenantSQLDeprecatedWrapper) OptionalErr(issue int) (interface{}, error)
}
return v, nil
}

// FeatureNotAvailableToNonSystemTenantsIssue is to be used with the Optinal
// and related error interfaces when a feature is simply not available
// to non-system tenants (i.e. we're not planning to change this).
const FeatureNotAvailableToNonSystemTenantsIssue = 54252

0 comments on commit 3fcafa8

Please sign in to comment.