Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: remove connExecutor.eventLog and sql.trace.session_eventlog.enabled #117928

Merged
merged 1 commit into from
Jan 19, 2024
+2 −39
Merged

sql: remove connExecutor.eventLog and sql.trace.session_eventlog.enabled #117928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/generated/settings/settings-for-tenants.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ sql.telemetry.txn_mode.tracking_limit integer 10000 the maximum number of transa
sql.temp_object_cleaner.cleanup_interval duration 30m0s how often to clean up orphaned temporary objects application
sql.temp_object_cleaner.wait_interval duration 30m0s how long after creation a temporary object will be cleaned up application
sql.log.all_statements.enabled boolean false set to true to enable logging of all executed statements application
sql.trace.session_eventlog.enabled boolean false set to true to enable session tracing; note that enabling this may have a negative performance impact application
sql.trace.stmt.enable_threshold duration 0s enables tracing on all statements; statements executing for longer than this duration will have their trace logged (set to 0 to disable); note that enabling this may have a negative performance impact; this setting applies to individual statements within a transaction and is therefore finer-grained than sql.trace.txn.enable_threshold application
sql.trace.txn.enable_threshold duration 0s enables tracing on all transactions; transactions open for longer than this duration will have their trace logged (set to 0 to disable); note that enabling this may have a negative performance impact; this setting is coarser-grained than sql.trace.stmt.enable_threshold because it applies to all statements within a transaction as well as client communication (e.g. retries) application
sql.ttl.changefeed_replication.disabled boolean false if true, deletes issued by TTL will not be replicated via changefeeds application
Expand Down
1 change: 0 additions & 1 deletion docs/generated/settings/settings.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@
<tr><td><div id="setting-sql-temp-object-cleaner-cleanup-interval" class="anchored"><code>sql.temp_object_cleaner.cleanup_interval</code></div></td><td>duration</td><td><code>30m0s</code></td><td>how often to clean up orphaned temporary objects</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-temp-object-cleaner-wait-interval" class="anchored"><code>sql.temp_object_cleaner.wait_interval</code></div></td><td>duration</td><td><code>30m0s</code></td><td>how long after creation a temporary object will be cleaned up</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-trace-log-statement-execute" class="anchored"><code>sql.log.all_statements.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to enable logging of all executed statements</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-trace-session-eventlog-enabled" class="anchored"><code>sql.trace.session_eventlog.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>set to true to enable session tracing; note that enabling this may have a negative performance impact</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-trace-stmt-enable-threshold" class="anchored"><code>sql.trace.stmt.enable_threshold</code></div></td><td>duration</td><td><code>0s</code></td><td>enables tracing on all statements; statements executing for longer than this duration will have their trace logged (set to 0 to disable); note that enabling this may have a negative performance impact; this setting applies to individual statements within a transaction and is therefore finer-grained than sql.trace.txn.enable_threshold</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-trace-txn-enable-threshold" class="anchored"><code>sql.trace.txn.enable_threshold</code></div></td><td>duration</td><td><code>0s</code></td><td>enables tracing on all transactions; transactions open for longer than this duration will have their trace logged (set to 0 to disable); note that enabling this may have a negative performance impact; this setting is coarser-grained than sql.trace.stmt.enable_threshold because it applies to all statements within a transaction as well as client communication (e.g. retries)</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-ttl-changefeed-replication-disabled" class="anchored"><code>sql.ttl.changefeed_replication.disabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if true, deletes issued by TTL will not be replicated via changefeeds</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
Expand Down
1 change: 1 addition & 0 deletions pkg/settings/registry.go
View file
Open in desktop
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
1 change: 0 additions & 1 deletion pkg/sql/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,6 @@ go_library(
"@com_github_prometheus_client_model//go",
"@in_gopkg_yaml_v2//:yaml_v2",
"@io_opentelemetry_go_otel//attribute",
"@org_golang_x_net//trace",
],
)

Expand Down
25 changes: 1 addition & 24 deletions pkg/sql/conn_executor.go
View file
Open in desktop
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
View file
Open in desktop
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