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

sql: experimental_relocate produces bad error for missing store #39835

Closed
jordanlewis opened this issue Aug 23, 2019 · 5 comments · Fixed by #41157
Closed

sql: experimental_relocate produces bad error for missing store #39835

jordanlewis opened this issue Aug 23, 2019 · 5 comments · Fixed by #41157
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. good first issue S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@jordanlewis
Copy link
Member

[email protected]:62021/movr> ALTER TABLE t EXPERIMENTAL_RELOCATE VALUES (ARRAY[3], 1);
E190823 00:14:03.192944 3737 sql/sqltelemetry/report.go:56  [n1,client=127.0.0.1:62026,user=root] encountered internal error:
assertion failure
  - error with attached stack trace:
    github.com/cockroachdb/cockroach/pkg/sql.(*relocateNode).Next
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/relocate.go:166
    github.com/cockroachdb/cockroach/pkg/sql.(*planNodeToRowSource).Next
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/plan_node_to_row_source.go:171
    github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.Run
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/base.go:171
    github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*ProcessorBase).Run
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/processors.go:793
    github.com/cockroachdb/cockroach/pkg/sql/distsqlrun.(*Flow).Run
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/flow.go:584
    github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).Run
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:334
    github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRun
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:959
    github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:808
    github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:700
    github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:416
    github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:98
    github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1211
    github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1140
    github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:442
    github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommandsAsync.func1
    	/Users/jordan/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:584
    runtime.goexit
    	/usr/local/Cellar/go/1.12.4/libexec/src/runtime/asm_amd64.s:1337
  - error with embedded safe details: error looking up store %d
    -- arg 1: 3
  - error looking up store 3:
  - KeyNotPresentError: gossip key "store:3" does not exist or has expired:
    original cause behind barrier: KeyNotPresentError: gossip key "store:3" does not exist or has expired

This shouldn't be an internal error with a stack trace - the problem is merely that the store doesn't exist.

@jordanlewis jordanlewis added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. labels Aug 23, 2019
@jordanlewis
Copy link
Member Author

We just need to replace NewAssertionErrorWithWrappedErrf in relocateNode.Next with something more appropriate.

@jagsileo
Copy link

Hi I am new to open source contribution, can I have this assigned to me ?

@jordanlewis
Copy link
Member Author

We don't assign issues to external contributors, but feel free to send a PR. Thanks!

@Gurio
Copy link
Contributor

Gurio commented Sep 13, 2019

@jagsileo: are you working on that?

@jagsileo
Copy link

Started to but had some issues with running the tests. Feel free to take it up. Sorry for not updating the thread.

craig bot pushed a commit that referenced this issue Sep 27, 2019
41157: sql: replace assert with error in EXPERIMENTAL_RELOCATE r=knz a=Gurio

Resolves #39835
GetInfoProto returns an error in relocateNode.Next,
in case if store doesn't exist. It should not produce a stacktrace.

reverts this line to pre 8cfcbb4 commit state.

Release justification: low-impact assert changed to error
Release note: None

Co-authored-by: Arseni Lapunov <[email protected]>
@craig craig bot closed this as completed in e77f56b Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. good first issue S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants