-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tenantcostserver: fix erroneous panic in tests #70094
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner)
df074eb
to
51554bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @RaduBerinde)
pkg/ccl/multitenantccl/tenantcostserver/system_table.go, line 434 at r2 (raw file):
) if err != nil { log.Warningf(ctx, "checkInvariants query failed: %v", err)
nit: wrap this in if ctx.Err() == nil {...}
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.
51554bf
to
50021ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFTRs!
bors r+
Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @RaduBerinde)
Build succeeded: |
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.