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 #70089.

Release note: None

Release justification: non-production code change to fix test failure.
  • Loading branch information
RaduBerinde committed Sep 12, 2021
1 parent 6eb9df1 commit df074eb
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
13 changes: 9 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,11 @@ func (h *sysTableHelper) checkInvariants() error {
h.tenantID.ToUint64(),
)
if err != nil {
return err
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
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 df074eb

Please sign in to comment.