Skip to content

Commit

Permalink
cli/sql: avoid taking a lease on the target db upon connect
Browse files Browse the repository at this point in the history
This makes `--debug-sql-cli` slightly more useful if something is
wrong with the database in the URL string or if there's something
wrong in the leasing subsystem.

Release note: None
  • Loading branch information
knz committed May 28, 2022
1 parent 9549e03 commit 9e1166e
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
7 changes: 7 additions & 0 deletions pkg/cli/clisqlcfg/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,13 @@ func (c *Context) Run(conn clisqlclient.Conn) error {
return err
}

if c.ConnCtx.DebugMode {
fmt.Fprintln(c.CmdOut,
"#\n# NOTE: if you intend to troubleshoot CockroachDB, you might want to set the current database\n"+
"# to the empty string (SET database = \"\"), for otherwise simple queries against crdb_internal\n"+
"# and other vtables will attempt to take a database lease and incur write traffic.\n#")
}

shell := clisqlshell.NewShell(c.CliCtx, c.ConnCtx, c.ExecCtx, &c.ShellCtx, conn)
return shell.RunInteractive(c.cmdIn, c.CmdOut, c.CmdErr)
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/cli/clisqlclient/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,9 @@ func (c *sqlConn) GetServerMetadata(
ctx context.Context,
) (nodeID int32, version, clusterID string, err error) {
// Retrieve the node ID and server build info.
rows, err := c.Query(ctx, "SELECT * FROM crdb_internal.node_build_info")
// Be careful to query against the empty database string, which avoids taking
// a lease against the current database (in case it's currently unavailable).
rows, err := c.Query(ctx, `SELECT * FROM "".crdb_internal.node_build_info`)
if errors.Is(err, driver.ErrBadConn) {
return 0, "", "", err
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/cli/interactive_tests/test_client_side_checking.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ end_test
start_test "Check that --debug-sql-cli sets suitable simplified client-side options."
send "$argv sql --debug-sql-cli\r"
eexpect "Welcome"

# Check empty db name for build info query.
eexpect "\"\".crdb_internal.node_build_info"
# Check invitation to reset db name.
eexpect "you might want to set the current database"
eexpect "to the empty string"

eexpect "root@"
send "\\set display_format csv\r\\set\r"
eexpect "check_syntax,false"
Expand Down

0 comments on commit 9e1166e

Please sign in to comment.