faster status updates

engine.go

@@ -37,6 +37,7 @@ import (
 	"github.com/dolthub/go-mysql-server/sql/rowexec"
 	"github.com/dolthub/go-mysql-server/sql/transform"
 	"github.com/dolthub/go-mysql-server/sql/types"
+	"github.com/dolthub/go-mysql-server/sql/variables"
 	_ "github.com/dolthub/go-mysql-server/sql/variables"
 )
 
@@ -168,6 +169,8 @@ func New(a *analyzer.Analyzer, cfg *Config) *Engine {
 
 	ls := sql.NewLockSubsystem()
 
+	variables.InitStatusVariables()
+
 	emptyCtx := sql.NewEmptyContext()
 
 	if fn, err := a.Catalog.Function(emptyCtx, "version"); fn == nil || err != nil {

processlist.go

@@ -70,9 +70,9 @@ func (pl *ProcessList) Processes() []sql.Process {
 }
 
 func (pl *ProcessList) AddConnection(id uint32, addr string) {
+	sql.StatusVariables.IncrementGlobal("Threads_connected", 1)
 	pl.mu.Lock()
 	defer pl.mu.Unlock()
-	sql.StatusVariables.IncrementGlobal("Threads_connected", 1)
 	pl.procs[id] = &sql.Process{
 		Connection: id,
 		Command:    sql.ProcessCommandConnect,
@@ -116,7 +116,7 @@ func (pl *ProcessList) BeginQuery(
 	pl.mu.Lock()
 	defer pl.mu.Unlock()
 
-	sql.IncrementStatusVariable(ctx, "Threads_running", 1)
+	sql.StatusVariables.IncrementGlobal("Threads_running", 1)
 
 	id := ctx.Session.ID()
 	pid := ctx.Pid()
@@ -158,7 +158,7 @@ func (pl *ProcessList) EndQuery(ctx *sql.Context) {
 			sql.IncrementStatusVariable(ctx, "Slow_queries", 1)
 		}
 
-		sql.IncrementStatusVariable(ctx, "Threads_running", -1)
+		sql.StatusVariables.IncrementGlobal("Threads_running", -1)
 		p.Command = sql.ProcessCommandSleep
 		p.Query = ""
 		p.StartedAt = time.Now()

processlist_test.go

@@ -23,10 +23,12 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/dolthub/go-mysql-server/sql"
+	"github.com/dolthub/go-mysql-server/sql/variables"
 )
 
 func TestProcessList(t *testing.T) {
 	require := require.New(t)
+	variables.InitStatusVariables()
 
 	clientHostOne := "127.0.0.1:34567"
 	clientHostTwo := "127.0.0.1:34568"

server/handler.go

@@ -94,7 +94,8 @@ func (h *Handler) NewConnection(c *mysql.Conn) {
 }
 
 func (h *Handler) ConnectionAborted(_ *mysql.Conn, _ string) error {
-	return sql.StatusVariables.IncrementGlobal("Aborted_connects", 1)
+	sql.StatusVariables.IncrementGlobal("Aborted_connects", 1)
+	return nil
 }
 
 func (h *Handler) ComInitDB(c *mysql.Conn, schemaName string) error {

sql/base_session.go

@@ -236,16 +236,16 @@ func (s *BaseSession) GetAllStatusVariables(_ *Context) map[string]StatusVarValu
 }
 
 // IncrementStatusVariable implements the Session interface.
-func (s *BaseSession) IncrementStatusVariable(_ *Context, statVarName string, val int) error {
+func (s *BaseSession) IncrementStatusVariable(ctx *Context, statVarName string, val int) {
 	if _, ok := s.statusVars[statVarName]; !ok {
-		return ErrUnknownSystemVariable.New(statVarName)
+		return
 	}
 	if val < 0 {
 		s.statusVars[statVarName].Increment(-(uint64(-val)))
 	} else {
 		s.statusVars[statVarName].Increment((uint64(val)))
 	}
-	return nil
+	return
 }
 
 // GetCharacterSet returns the character set for this session (defined by the system variable `character_set_connection`).

sql/core.go

@@ -704,16 +704,19 @@ var StatusVariables StatusVariableRegistry
 
 // StatusVariableRegistry is a registry of status variables.
 type StatusVariableRegistry interface {
-	// NewSessionMap returns a deep copy of the status variables that are not GlobalOnly scope (i.e. SessionOnly or Both)
+	// NewSessionMap returns a deep copy of the status variables that are
+	// not GlobalOnly scope (i.e. SessionOnly or Both)
 	NewSessionMap() map[string]StatusVarValue
 	// NewGlobalMap returns a deep copy of the status variables of every scope
 	NewGlobalMap() map[string]StatusVarValue
 	// GetGlobal returns the current global value of the status variable with the given name
 	GetGlobal(name string) (StatusVariable, interface{}, bool)
-	// SetGlobal sets the global value of the status variable with the given name, returns an error if the variable is SessionOnly scope
+	// SetGlobal sets the global value of the status variable with the given
+	// name, returns an error if the variable is SessionOnly scope
 	SetGlobal(name string, val interface{}) error
-	// IncrementGlobal increments the value of the status variable by the given integer value
-	IncrementGlobal(name string, val int) error
+	// IncrementGlobal increments the value of the status variable by the
+	// given integer value. Noop if the variable is session-only scoped.
+	IncrementGlobal(name string, val int)
 }
 
 // StatusVariableScope represents the scope of a status variable.
@@ -833,11 +836,8 @@ func (s *ImmutableStatusVarValue) Copy() StatusVarValue {
 }
 
 // IncrementStatusVariable increments the value of the status variable by integer val.
-// name is case-insensitive. Errors are ignored.
-// This runs in a goroutine to avoid blocking the caller, but we do not wait for it to complete.
+// |name| is case-sensitive.
 func IncrementStatusVariable(ctx *Context, name string, val int) {
-	go func() {
-		StatusVariables.IncrementGlobal(name, val)
-		ctx.Session.IncrementStatusVariable(ctx, name, val)
-	}()
+	StatusVariables.IncrementGlobal(name, val)
+	ctx.Session.IncrementStatusVariable(ctx, name, val)
 }

sql/rowexec/show_status_test.go

@@ -22,11 +22,13 @@ import (
 
 	"github.com/dolthub/go-mysql-server/sql"
 	"github.com/dolthub/go-mysql-server/sql/plan"
+	"github.com/dolthub/go-mysql-server/sql/variables"
 )
 
 func TestShowStatus(t *testing.T) {
 	require := require.New(t)
 	ctx := sql.NewEmptyContext()
+	variables.InitStatusVariables()
 
 	var res sql.Row
 	var err error

sql/session.go

@@ -91,7 +91,7 @@ type Session interface {
 	// To access global scope, use sql.StatusVariables instead.
 	GetAllStatusVariables(ctx *Context) map[string]StatusVarValue
 	// IncrementStatusVariable increments the value of the status variable by the integer value
-	IncrementStatusVariable(ctx *Context, statVarName string, val int) error
+	IncrementStatusVariable(ctx *Context, statVarName string, val int)
 	// GetCurrentDatabase gets the current database for this session
 	GetCurrentDatabase() string
 	// SetCurrentDatabase sets the current database for this session

sql/variables/status_variables.go

@@ -93,19 +93,13 @@ func (g *globalStatusVariables) SetGlobal(name string, val interface{}) error {
 }
 
 // IncrementGlobal implements sql.StatusVariableRegistry
-func (g *globalStatusVariables) IncrementGlobal(name string, val int) error {
+func (g *globalStatusVariables) IncrementGlobal(name string, val int) {
 	v, ok := g.varVals[name]
 	if !ok || v.Variable().GetScope() == sql.StatusVariableScope_Session {
-		return sql.ErrUnknownSystemVariable.New(name)
+		return
 	}
 	g.varVals[name].Increment(uint64(val))
-	return nil
-}
-
-// init initializes SystemVariables as it functions as a global variable.
-// TODO: get rid of me, make this construction the responsibility of the engine
-func init() {
-	InitStatusVariables()
+	return
 }
 
 // InitStatusVariables initializes the global status variables in sql.StatusVariables. If they have already

sql/variables/status_variables_test.go

@@ -23,6 +23,7 @@ import (
 )
 
 func TestStatusVariables(t *testing.T) {
+	InitStatusVariables()
 	defer InitStatusVariables()
 
 	require := require.New(t)