Skip to content

Commit 114a395

Browse files
committed
sqlstats: remove sslocal controller
This struct is not very useful as it just provides an interface to reset local sql stats. This commit removes the sql stats controller defined in sslocal as we can access the local sql stats directly. Epic: none Release note: None
1 parent 200810a commit 114a395

File tree

6 files changed

+23
-90
lines changed

6 files changed

+23
-90
lines changed

pkg/server/application_api/stats_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ CREATE TABLE t.test (x INT PRIMARY KEY);
6464

6565
// Flush stats at the beginning of the test.
6666
require.NoError(t, sqlServer.GetLocalSQLStatsProvider().Reset(ctx))
67-
sqlServer.GetReportedSQLStatsController().ResetLocalSQLStats(ctx)
67+
require.NoError(t, sqlServer.GetReportedSQLStatsProvider().Reset(ctx))
6868

6969
// Run some queries mixed with diagnostics, and ensure that the statistics
7070
// are unaffected by the calls to report diagnostics.
@@ -178,8 +178,8 @@ func TestSQLStatCollection(t *testing.T) {
178178
sqlServer := srv.ApplicationLayer().SQLServer().(*sql.Server)
179179

180180
// Flush stats at the beginning of the test.
181-
sqlServer.GetSQLStatsController().ResetLocalSQLStats(ctx)
182-
sqlServer.GetReportedSQLStatsController().ResetLocalSQLStats(ctx)
181+
require.NoError(t, sqlServer.GetLocalSQLStatsProvider().Reset(ctx))
182+
require.NoError(t, sqlServer.GetReportedSQLStatsProvider().Reset(ctx))
183183

184184
// Execute some queries against the sqlDB to build up some stats.
185185
// As we are scrubbing the stats, we want to make sure the app name
@@ -397,7 +397,7 @@ func TestScrubbedReportingStatsLimit(t *testing.T) {
397397
sqlServer := srv.ApplicationLayer().SQLServer().(*sql.Server)
398398
// Flush stats at the beginning of the test.
399399
require.NoError(t, sqlServer.GetLocalSQLStatsProvider().Reset(ctx))
400-
sqlServer.GetReportedSQLStatsController().ResetLocalSQLStats(ctx)
400+
require.NoError(t, sqlServer.GetReportedSQLStatsProvider().Reset(ctx))
401401

402402
hashedAppName := "hashed app name"
403403
sqlRunner.Exec(t, `SET application_name = $1;`, hashedAppName)

pkg/server/diagnostics/reporter.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,10 @@ func (r *Reporter) ReportDiagnostics(ctx context.Context) {
281281
log.Warningf(ctx, "failed to report node usage metrics: status: %s, body: %s", res.Status, b)
282282
return
283283
}
284-
r.SQLServer.GetReportedSQLStatsController().ResetLocalSQLStats(ctx)
284+
err = r.SQLServer.GetReportedSQLStatsProvider().Reset(ctx)
285+
if err != nil {
286+
log.Warningf(ctx, "failed to reset SQL stats: %s", err)
287+
}
285288
}
286289

287290
// GetLastSuccessfulTelemetryPing will return the timestamp of when we last got

pkg/sql/conn_executor.go

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -356,10 +356,6 @@ type Server struct {
356356
// into reported stats when sqlStats is cleared.
357357
reportedStats *sslocal.SQLStats
358358

359-
// reportedStatsController is the control-plane interface for
360-
// reportedStatsController.
361-
reportedStatsController *sslocal.Controller
362-
363359
insights *insights.Provider
364360

365361
reCache *tree.RegexpCache
@@ -449,7 +445,6 @@ func NewServer(cfg *ExecutorConfig, pool *mon.BytesMonitor) *Server {
449445
nil, /* reportedProvider */
450446
cfg.SQLStatsTestingKnobs,
451447
)
452-
reportedSQLStatsController := reportedSQLStats.GetController(cfg.SQLStatusServer)
453448
memSQLStats := sslocal.New(
454449
cfg.Settings,
455450
sqlstats.MaxMemSQLStatsStmtFingerprints,
@@ -468,18 +463,17 @@ func NewServer(cfg *ExecutorConfig, pool *mon.BytesMonitor) *Server {
468463
}
469464
})
470465
s := &Server{
471-
cfg: cfg,
472-
Metrics: metrics,
473-
InternalMetrics: makeMetrics(true /* internal */, &cfg.Settings.SV),
474-
ServerMetrics: serverMetrics,
475-
pool: pool,
476-
localSqlStats: memSQLStats,
477-
reportedStats: reportedSQLStats,
478-
sqlStatsIngester: sqlStatsIngester,
479-
reportedStatsController: reportedSQLStatsController,
480-
insights: insightsProvider,
481-
reCache: tree.NewRegexpCache(512),
482-
toCharFormatCache: tochar.NewFormatCache(512),
466+
cfg: cfg,
467+
Metrics: metrics,
468+
InternalMetrics: makeMetrics(true /* internal */, &cfg.Settings.SV),
469+
ServerMetrics: serverMetrics,
470+
pool: pool,
471+
localSqlStats: memSQLStats,
472+
reportedStats: reportedSQLStats,
473+
sqlStatsIngester: sqlStatsIngester,
474+
insights: insightsProvider,
475+
reCache: tree.NewRegexpCache(512),
476+
toCharFormatCache: tochar.NewFormatCache(512),
483477
indexUsageStats: idxusage.NewLocalIndexUsageStats(&idxusage.Config{
484478
ChannelSize: idxusage.DefaultChannelSize,
485479
Setting: cfg.Settings,
@@ -725,10 +719,10 @@ func (s *Server) GetLocalSQLStatsProvider() *sslocal.SQLStats {
725719
return s.localSqlStats
726720
}
727721

728-
// GetReportedSQLStatsController returns the sqlstats.Controller for the current
729-
// sql.Server's reported SQL Stats.
730-
func (s *Server) GetReportedSQLStatsController() *sslocal.Controller {
731-
return s.reportedStatsController
722+
// GetReportedSQLStatsProvider returns the provider for the in-memory reported
723+
// sql stats sink.
724+
func (s *Server) GetReportedSQLStatsProvider() *sslocal.SQLStats {
725+
return s.reportedStats
732726
}
733727

734728
// GetTxnIDCache returns the txnidcache.Cache for the current sql.Server.

pkg/sql/sqlstats/sslocal/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ go_library(
55
srcs = [
66
"cluster_settings.go",
77
"sql_stats.go",
8-
"sql_stats_controller.go",
98
"sql_stats_ingestor.go",
109
"sslocal_iterator.go",
1110
"sslocal_provider.go",

pkg/sql/sqlstats/sslocal/sql_stats_controller.go

Lines changed: 0 additions & 56 deletions
This file was deleted.

pkg/sql/sqlstats/sslocal/sslocal_provider.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"sort"
1111
"time"
1212

13-
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
1413
"github.com/cockroachdb/cockroach/pkg/settings"
1514
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
1615
"github.com/cockroachdb/cockroach/pkg/sql/appstatspb"
@@ -47,12 +46,6 @@ func New(
4746
)
4847
}
4948

50-
// GetController returns a sqlstats.Controller responsible for the current
51-
// SQLStats.
52-
func (s *SQLStats) GetController(server serverpb.SQLStatusServer) *Controller {
53-
return NewController(s, server)
54-
}
55-
5649
func (s *SQLStats) Start(ctx context.Context, stopper *stop.Stopper) {
5750
// We run a periodic async job to clean up the in-memory stats.
5851
_ = stopper.RunAsyncTask(ctx, "sql-stats-clearer", func(ctx context.Context) {

0 commit comments

Comments
 (0)