Skip to content

Commit

Permalink
sql: remove connExecutor.eventLog and sql.trace.session_eventlog.enabled
Browse files Browse the repository at this point in the history
The `connExecutor.eventLog` was capturing session events when
`sql.trace.session_eventlog.enabled` was enabled (default false). This was
historically used by SQL engineers for debugging the pgwire connection state
machine, but is not in current use.

Epic: none

Release note (sql change): Remove sql.trace.session_eventlog.enabled and the
associated event log tracing. The information in these traces is still
available in the DEV log channel by enabling --vmodule=conn_executor=2.
  • Loading branch information
petermattis committed Jan 18, 2024
1 parent 6e499de commit e5c95e1
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 36 deletions.
1 change: 1 addition & 0 deletions pkg/settings/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ var retiredSettings = map[InternalKey]struct{}{
"kv.rangefeed.scheduler.enabled": {},
"physical_replication.producer.mux_rangefeeds.enabled": {},
"kv.rangefeed.use_dedicated_connection_class.enabled": {},
"sql.trace.session_eventlog.enabled": {},
}

// sqlDefaultSettings is the list of "grandfathered" existing sql.defaults
Expand Down
25 changes: 1 addition & 24 deletions pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ import (
"github.com/cockroachdb/errors"
"github.com/cockroachdb/logtags"
"github.com/cockroachdb/redact"
"golang.org/x/net/trace"
)

var maxNumNonAdminConnections = settings.RegisterIntSetting(
Expand Down Expand Up @@ -1288,11 +1287,6 @@ func (ex *connExecutor) close(ctx context.Context, closeType closeType) {
}
}

if ex.eventLog != nil {
ex.eventLog.Finish()
ex.eventLog = nil
}

// Stop idle timer if the connExecutor is closed to ensure cancel session
// is not called.
ex.mu.IdleInSessionTimeout.Stop()
Expand Down Expand Up @@ -1371,10 +1365,6 @@ type connExecutor struct {
transitionCtx transitionCtx
sessionTracing SessionTracing

// eventLog for SQL statements and other important session events. Will be set
// if traceSessionEventLogEnabled; it is used by ex.sessionEventf()
eventLog trace.EventLog

// extraTxnState groups fields scoped to a SQL txn that are not handled by
// ex.state, above. The rule of thumb is that, if the state influences state
// transitions, it should live in state, otherwise it can live here.
Expand Down Expand Up @@ -2064,16 +2054,6 @@ func (ex *connExecutor) activate(
ex.sessionMon.StartNoReserved(ctx, ex.mon)
ex.sessionPreparedMon.StartNoReserved(ctx, ex.sessionMon)

// Enable the trace if configured.
if traceSessionEventLogEnabled.Get(&ex.server.cfg.Settings.SV) {
remoteStr := "<admin>"
if ex.sessionData().RemoteAddr != nil {
remoteStr = ex.sessionData().RemoteAddr.String()
}
ex.eventLog = trace.NewEventLog(
fmt.Sprintf("sql session [%s]", ex.sessionData().User()), remoteStr)
}

ex.activated = true
}

Expand Down Expand Up @@ -2175,7 +2155,7 @@ func (ex *connExecutor) execCmd() (retErr error) {
return err // err could be io.EOF
}

if log.ExpensiveLogEnabled(ctx, 2) || ex.eventLog != nil {
if log.ExpensiveLogEnabled(ctx, 2) {
ex.sessionEventf(ctx, "[%s pos:%d] executing %s",
ex.machine.CurState(), pos, cmd)
}
Expand Down Expand Up @@ -4195,9 +4175,6 @@ func (ex *connExecutor) sessionEventf(ctx context.Context, format string, args .
if log.ExpensiveLogEnabled(ctx, 2) {
log.VEventfDepth(ctx, 1 /* depth */, 2 /* level */, format, args...)
}
if ex.eventLog != nil {
ex.eventLog.Printf(format, args...)
}
}

// notifyStatsRefresherOfNewTables is called on txn commit to inform
Expand Down
12 changes: 0 additions & 12 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,18 +272,6 @@ var TraceStmtThreshold = settings.RegisterDurationSetting(
0,
settings.WithPublic)

// traceSessionEventLogEnabled can be used to enable the event log
// that is normally kept for every SQL connection. The event log has a
// non-trivial performance impact and also reveals SQL statements
// which may be a privacy concern.
var traceSessionEventLogEnabled = settings.RegisterBoolSetting(
settings.ApplicationLevel,
"sql.trace.session_eventlog.enabled",
"set to true to enable session tracing; "+
"note that enabling this may have a negative performance impact",
false,
settings.WithPublic)

// ReorderJoinsLimitClusterSettingName is the name of the cluster setting for
// the maximum number of joins to reorder.
const ReorderJoinsLimitClusterSettingName = "sql.defaults.reorder_joins_limit"
Expand Down

0 comments on commit e5c95e1

Please sign in to comment.