Skip to content

Commit

Permalink
sql: fix check for closing connExecutor during draining
Browse files Browse the repository at this point in the history
This fixes a minor bug in which the connection would not get closed at
the earliest possible time during server shutdown.

The connection is supposed to be closed as soon as we handle a Sync
message when the conn_executor is in the draining state and not in a
transaction. Since the transaction state was checked before state
transitions occurred, this would cause the connection to remain open for
an extra bit of time.

Release note: None
  • Loading branch information
rafiss committed Mar 13, 2023
1 parent 18eac87 commit b05859d
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,7 @@ func (ex *connExecutor) run(

var err error
if err = ex.execCmd(); err != nil {
// Both of these errors are normal ways for the connExecutor to exit.
if errors.IsAny(err, io.EOF, errDrainingComplete) {
return nil
}
Expand All @@ -1934,7 +1935,7 @@ var errDrainingComplete = fmt.Errorf("draining done. this is a good time to fini
// Returns drainingComplete if the session should finish because draining is
// complete (i.e. we received a DrainRequest - possibly previously - and the
// connection is found to be idle).
func (ex *connExecutor) execCmd() error {
func (ex *connExecutor) execCmd() (retErr error) {
ctx := ex.Ctx()
cmd, pos, err := ex.stmtBuf.CurCmd()
if err != nil {
Expand Down Expand Up @@ -2121,16 +2122,16 @@ func (ex *connExecutor) execCmd() error {
// Note that the Sync result will flush results to the network connection.
res = ex.clientComm.CreateSyncResult(pos)
if ex.draining {
// If we're draining, check whether this is a good time to finish the
// If we're draining, then after handing the Sync connExecutor state
// transition, check whether this is a good time to finish the
// connection. If we're not inside a transaction, we stop processing
// now. If we are inside a transaction, we'll check again the next time
// a Sync is processed.
if ex.idleConn() {
// If we're about to close the connection, close res in order to flush
// now, as we won't have an opportunity to do it later.
res.Close(ctx, stateToTxnStatusIndicator(ex.machine.CurState()))
return errDrainingComplete
}
defer func() {
if ex.idleConn() {
retErr = errDrainingComplete
}
}()
}
case CopyIn:
ex.phaseTimes.SetSessionPhaseTime(sessionphase.SessionQueryReceived, tcmd.TimeReceived)
Expand Down

0 comments on commit b05859d

Please sign in to comment.