Skip to content
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

v22.1.0: sql: cannot use CheckPrivilege without a txn during prepared statement #82034

Closed
cockroach-teamcity opened this issue May 29, 2022 · 6 comments
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented May 29, 2022

This issue was autofiled by Sentry. It represents a crash or reported error on a live cluster with telemetry enabled.

Sentry link: https://sentry.io/organizations/cockroach-labs/issues/3309794281/?referrer=webhooks_plugin

Panic message:

authorization.go:129: cannot use CheckPrivilege without a txn
(1) assertion failure
Wraps: (2) attached stack trace
-- stack trace:
| github.com/cockroachdb/cockroach/pkg/sql.(*planner).CheckPrivilegeForUser
| github.com/cockroachdb/cockroach/pkg/sql/authorization.go:129
| github.com/cockroachdb/cockroach/pkg/sql.(*planner).canResolveDescUnderSchema
| github.com/cockroachdb/cockroach/pkg/sql/authorization.go:743
| github.com/cockroachdb/cockroach/pkg/sql.(*optCatalog).ResolveDataSource
| github.com/cockroachdb/cockroach/pkg/sql/opt_catalog.go:214
| github.com/cockroachdb/cockroach/pkg/sql/opt.(*Metadata).CheckDependencies
| github.com/cockroachdb/cockroach/pkg/sql/opt/metadata.go:293
| github.com/cockroachdb/cockroach/pkg/sql/opt/memo.(*Memo).IsStale
| github.com/cockroachdb/cockroach/pkg/sql/opt/memo/memo.go:333
| github.com/cockroachdb/cockroach/pkg/sql.(*planner).prepareUsingOptimizer
| github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:119
| github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).populatePrepared
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:278
| github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).prepare.func1
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:235
| github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).prepare
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:240
| github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).addPreparedStmt
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:102
| github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execPrepare
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor_prepare.go:63
| github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1975
| github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1799
| github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn
| github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:747
| github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommandsAsync.func1
| github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:724
| runtime.goexit
| GOROOT/src/runtime/asm_amd64.s:1581
Wraps: (3) cannot use CheckPrivilege without a txn
Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.leafError
-- report composition:
*errutil.leafError: cannot use CheckPrivilege without a txn
authorization.go:129: *withstack.withStack (top exception)
*assert.withAssertionFailure

Stacktrace (expand for inline code snippets):

if p.txn == nil || !p.txn.IsOpen() {
return errors.AssertionFailedf("cannot use CheckPrivilege without a txn")
}
in pkg/sql.(*planner).CheckPrivilegeForUser
case catalog.SchemaUserDefined:
return p.CheckPrivilegeForUser(ctx, scDesc, privilege.USAGE, p.User())
default:
in pkg/sql.(*planner).canResolveDescUnderSchema
// Ensure that the current user can access the target schema.
if err := oc.planner.canResolveDescUnderSchema(ctx, prefix.Schema, desc); err != nil {
return nil, cat.DataSourceName{}, err
in pkg/sql.(*optCatalog).ResolveDataSource
// Resolve data source object.
toCheck, _, err = catalog.ResolveDataSource(ctx, cat.Flags{}, &name.byName)
}
in pkg/sql/opt.(*Metadata).CheckDependencies
// access the object.
if depsUpToDate, err := m.Metadata().CheckDependencies(ctx, catalog); err != nil {
return true, err
in pkg/sql/opt/memo.(*Memo).IsStale
} else {
isStale, err := cachedData.Memo.IsStale(ctx, p.EvalContext(), &opc.catalog)
if err != nil {
in pkg/sql.(*planner).prepareUsingOptimizer
// and execute paths.
flags, err := p.prepareUsingOptimizer(ctx)
if err != nil {
in pkg/sql.(*connExecutor).populatePrepared
p.semaCtx.Annotations = tree.MakeAnnotations(stmt.NumAnnotations)
flags, err = ex.populatePrepared(ctx, txn, placeholderHints, p)
return err
in pkg/sql.(*connExecutor).prepare.func1
// Use the existing transaction.
if err := prepare(ctx, ex.state.mu.txn); err != nil && origin != PreparedStatementOriginSessionMigration {
return nil, err
in pkg/sql.(*connExecutor).prepare
// Prepare the query. This completes the typing of placeholders.
prepared, err := ex.prepare(ctx, stmt, placeholderHints, rawTypeHints, origin)
if err != nil {
in pkg/sql.(*connExecutor).addPreparedStmt
stmt := makeStatement(parseCmd.Statement, ex.generateID())
_, err := ex.addPreparedStmt(
ctx,
in pkg/sql.(*connExecutor).execPrepare
stmtCtx := withStatement(ctx, ex.curStmtAST)
ev, payload = ex.execPrepare(stmtCtx, tcmd)
case DescribeStmt:
in pkg/sql.(*connExecutor).execCmd
var err error
if err = ex.execCmd(); err != nil {
if errors.IsAny(err, io.EOF, errDrainingComplete) {
in pkg/sql.(*connExecutor).run
}()
return h.ex.run(ctx, s.pool, reserved, cancel)
}
in pkg/sql.(*Server).ServeConn
reservedOwned = false // We're about to pass ownership away.
retErr = sqlServer.ServeConn(ctx, connHandler, reserved, cancelConn)
}()
in pkg/sql/pgwire.(*conn).processCommandsAsync.func1
GOROOT/src/runtime/asm_amd64.s#L1580-L1582 in runtime.goexit

pkg/sql/authorization.go in pkg/sql.(*planner).CheckPrivilegeForUser at line 129
pkg/sql/authorization.go in pkg/sql.(*planner).canResolveDescUnderSchema at line 743
pkg/sql/opt_catalog.go in pkg/sql.(*optCatalog).ResolveDataSource at line 214
pkg/sql/opt/metadata.go in pkg/sql/opt.(*Metadata).CheckDependencies at line 293
pkg/sql/opt/memo/memo.go in pkg/sql/opt/memo.(*Memo).IsStale at line 333
pkg/sql/plan_opt.go in pkg/sql.(*planner).prepareUsingOptimizer at line 119
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).populatePrepared at line 278
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).prepare.func1 at line 235
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).prepare at line 240
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).addPreparedStmt at line 102
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).execPrepare at line 63
pkg/sql/conn_executor.go in pkg/sql.(*connExecutor).execCmd at line 1975
pkg/sql/conn_executor.go in pkg/sql.(*connExecutor).run at line 1799
pkg/sql/conn_executor.go in pkg/sql.(*Server).ServeConn at line 747
pkg/sql/pgwire/conn.go in pkg/sql/pgwire.(*conn).processCommandsAsync.func1 at line 724
GOROOT/src/runtime/asm_amd64.s in runtime.goexit at line 1581
Tag Value
Cockroach Release v22.1.0
Cockroach SHA: 5b78463
Platform linux amd64
Distribution CCL
Environment v22.1.0
Command start-single-node
Go Version ``
# of CPUs
# of Goroutines

Jira issue: CRDB-16177

gz#13333

@cockroach-teamcity cockroach-teamcity added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report. labels May 29, 2022
@rafiss rafiss changed the title sentry: authorization.go:129: cannot use CheckPrivilege without a txn (1) assertion failure Wraps: (2) attached stack trace -- stack trace: | github.com/cockroachdb/cockroach/pkg/sql.(*planner).CheckPrivi... v22.1.0: sql: cannot use CheckPrivilege without a txn during prepared statement May 29, 2022
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 29, 2022
@rafiss rafiss self-assigned this May 31, 2022
@rafiss
Copy link
Collaborator

rafiss commented May 31, 2022

According to Sentry, the statement is BEGIN TRANSACTION, but I don't know why that would call canResolveDescUnderSchema

@rafiss
Copy link
Collaborator

rafiss commented May 31, 2022

must relate to #80764

@rafiss
Copy link
Collaborator

rafiss commented Jun 8, 2022

i wonder if there's any chance that #82622 fixes this

@fqazi
Copy link
Collaborator

fqazi commented Jun 16, 2022

This still reproduces on the randomized schema changer workload with that fix on a recent master. Unfortunately, I haven't gotten the changes that repro this merged yet its on a development branch for it

@rafiss
Copy link
Collaborator

rafiss commented Jul 23, 2022

From an internal support case, I think I'm beginning to see how this can happen. This is just a theory.

If you run a statement like CREATE CHANGEFEED, part of executing it is to update the jobs table, here:

n, err := j.registry.ex.Exec(ctx, "job-update", txn, updateStmt, params...)

This internal query can fail with a TransactionRetryError, which eventually causes the connExecutor's txn to be nil'd:

func (ts *txnState) finishExternalTxn() {

Since that internal query is using placeholder arguments, it runs as a prepared statement. Perhaps that somehow leads to prepare being called here with that nil txn (this would mean the internal executor automatically retried, but failed to correctly keep track of which transaction it's using):

if err := prepare(ctx, ex.state.mu.txn); err != nil && origin != PreparedStatementOriginSessionMigration {

While preparing that statement, the optimizer needs to check privileges, and encounters the cannot use CheckPrivilege without a txn error.

craig bot pushed a commit that referenced this issue Aug 12, 2022
85996: sql: don't panic on auth check with unopen txn r=ajwerner,RichardJCai a=rafiss

Relates to #80764 and #82034

Ever since c00ea84 was merged, the KV
layer has disallowed use of an aborted txn. Therefore, the checks here
are no longer necessary.

This should actually help with debugging, since now if the aborted txn
is used, we should get back an error that has the reason for the abort
(or restart), instead of a panic that does not have that info.

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
@rafiss
Copy link
Collaborator

rafiss commented Aug 23, 2022

#85996 fixes this specific error, which usually is a sign of incorrect error handling elsewhere. closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

3 participants