Skip to content

Commit

Permalink
tenantcostserver: fix erroneous panic in tests
Browse files Browse the repository at this point in the history
The test-only code that checks the invariants of the `tenant_usage`
table inadvertently panics if the query hits an error (such as one
that would be expected if the server is shutting down). We now just
log the error instead.

Fixes cockroachdb#70089.

Release note: None

Release justification: non-production code change to fix test failure.
  • Loading branch information
RaduBerinde committed Sep 13, 2021
1 parent 6eb9df1 commit 50021ff
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 5 deletions.
1 change: 1 addition & 0 deletions pkg/ccl/multitenantccl/tenantcostserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ go_library(
"//pkg/sql/sem/tree",
"//pkg/sql/sessiondata",
"//pkg/util",
"//pkg/util/log",
"//pkg/util/metric",
"//pkg/util/metric/aggmetric",
"//pkg/util/syncutil",
Expand Down
14 changes: 10 additions & 4 deletions pkg/ccl/multitenantccl/tenantcostserver/system_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -392,16 +393,16 @@ func (h *sysTableHelper) accomodateNewInstance(tenant *tenantState, instance *in

// maybeCheckInvariants checks the invariants for the system table with a random
// probability and only if this is a test build.
func (h *sysTableHelper) maybeCheckInvariants() error {
func (h *sysTableHelper) maybeCheckInvariants(ctx context.Context) error {
if util.CrdbTestBuild && rand.Intn(10) == 0 {
return h.checkInvariants()
return h.checkInvariants(ctx)
}
return nil
}

// checkInvariants reads all rows in the system table for the given tenant and
// checks that the state is consistent.
func (h *sysTableHelper) checkInvariants() error {
func (h *sysTableHelper) checkInvariants(ctx context.Context) error {
// Read the two rows for the per-tenant state (instance_id = 0) and the
// per-instance state.
rows, err := h.ex.QueryBufferedEx(
Expand Down Expand Up @@ -430,7 +431,12 @@ func (h *sysTableHelper) checkInvariants() error {
h.tenantID.ToUint64(),
)
if err != nil {
return err
if h.ctx.Err() == nil {
log.Warningf(ctx, "checkInvariants query failed: %v", err)
}
// We don't want to cause a panic for a query error (which is expected
// during shutdown).
return nil
}
if len(rows) == 0 {
return nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/multitenantccl/tenantcostserver/token_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (s *instance) TokenBucketRequest(
return err
}

if err := h.maybeCheckInvariants(); err != nil {
if err := h.maybeCheckInvariants(ctx); err != nil {
panic(err)
}
consumption = tenant.Consumption
Expand Down

0 comments on commit 50021ff

Please sign in to comment.