Skip to content

Commit

Permalink
Merge #29520 #29544
Browse files Browse the repository at this point in the history
29520: sql: fix a panic in txn_state.go when verbose logging r=andreimatei,knz a=arjunravinarayan

Fix a panic (reproducible with 
`make testlogic TESTFLAGS='-verbosity=2' FILES=pg_catalog` ) 
due to an uninitialized
context. The comment on Ctx says that Ctx is uninitialized if we are
in `fsm.stateNoTxn`. In that case, use the parent (session) context
which is always initialized.

First commit is just some type assertions that I found useful to understand what's going on.

Fixes #29384.

29544: roachtest: fix acceptance/version-upgrade on remote clusters r=tschottdorf a=petermattis

No need to run a versionStep for the base version as we manually do that
before executing the version steps. Duplicating this work was failing on
the second upload because we can't overwrite the executable for a
running binary.

Fixes #29542

Release note: None

Co-authored-by: Arjun Narayan <[email protected]>
Co-authored-by: Peter Mattis <[email protected]>
  • Loading branch information
3 people committed Sep 4, 2018
3 parents 07ebdf1 + fa020e9 + bc683e5 commit 2ddf654
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
1 change: 0 additions & 1 deletion pkg/cmd/roachtest/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,6 @@ func runVersionUpgrade(ctx context.Context, t *test, c *cluster) {

const baseVersion = "v1.0.6"
steps := []versionStep{
binaryVersionUpgrade(baseVersion, nodes),
// v1.1.0 is the first binary version that knows about cluster versions,
// but thinks it can only support up to 1.0-3.
binaryVersionUpgrade("v1.1.0", nodes),
Expand Down
12 changes: 12 additions & 0 deletions pkg/sql/conn_fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ const (

type stateNoTxn struct{}

var _ State = &stateNoTxn{}

func (stateNoTxn) String() string {
return NoTxnStr
}
Expand All @@ -62,6 +64,8 @@ type stateOpen struct {
RetryIntent Bool
}

var _ State = &stateOpen{}

func (stateOpen) String() string {
return OpenStateStr
}
Expand All @@ -72,18 +76,24 @@ type stateAborted struct {
RetryIntent Bool
}

var _ State = &stateAborted{}

func (stateAborted) String() string {
return AbortedStateStr
}

type stateRestartWait struct{}

var _ State = &stateRestartWait{}

func (stateRestartWait) String() string {
return RestartWaitStateStr
}

type stateCommitWait struct{}

var _ State = &stateCommitWait{}

func (stateCommitWait) String() string {
return CommitWaitStateStr
}
Expand All @@ -95,6 +105,8 @@ func (stateCommitWait) String() string {
// back.
type stateInternalError struct{}

var _ State = &stateInternalError{}

func (stateInternalError) String() string {
return InternalErrorStateStr
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/txn_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ func (ts *txnState) finishSQLTxn() {
// InternalExecutor). These guys don't want to mess with the transaction per-se,
// but still want to clean up other stuff.
func (ts *txnState) finishExternalTxn() {
ts.mon.Stop(ts.Ctx)
if ts.Ctx == nil {
ts.mon.Stop(ts.connCtx)
} else {
ts.mon.Stop(ts.Ctx)
}
if ts.cancel != nil {
ts.cancel()
ts.cancel = nil
Expand Down

0 comments on commit 2ddf654

Please sign in to comment.