Skip to content

Commit

Permalink
jobs/jobspb,sql/stats: move constant from stats to jobspb
Browse files Browse the repository at this point in the history
It seemed crazy for jobspb to depend on stats over this one string constant.

Release note: None
  • Loading branch information
ajwerner committed Jun 30, 2021
1 parent d4627db commit 59756c4
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 32 deletions.
1 change: 0 additions & 1 deletion pkg/jobs/jobspb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/sql/catalog/descpb",
"//pkg/sql/stats",
"//pkg/storage/cloud",
"@com_github_cockroachdb_errors//:errors",
"@com_github_gogo_protobuf//jsonpb",
Expand Down
8 changes: 6 additions & 2 deletions pkg/jobs/jobspb/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"strings"

"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/stats"
"github.com/cockroachdb/cockroach/pkg/storage/cloud"
"github.com/cockroachdb/errors"
"github.com/gogo/protobuf/jsonpb"
Expand Down Expand Up @@ -59,6 +58,11 @@ func (p *Payload) Type() Type {
return DetailsType(p.Details)
}

// AutoStatsName is the name to use for statistics created automatically.
// The name is chosen to be something that users are unlikely to choose when
// running CREATE STATISTICS manually.
const AutoStatsName = "__auto__"

// DetailsType returns the type for a payload detail.
func DetailsType(d isPayload_Details) Type {
switch d := d.(type) {
Expand All @@ -74,7 +78,7 @@ func DetailsType(d isPayload_Details) Type {
return TypeChangefeed
case *Payload_CreateStats:
createStatsName := d.CreateStats.Name
if createStatsName == stats.AutoStatsName {
if createStatsName == AutoStatsName {
return TypeAutoCreateStats
}
return TypeCreateStats
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/create_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (n *createStatsNode) startJob(ctx context.Context, resultsCh chan<- tree.Da
return err
}

if n.Name == stats.AutoStatsName {
if n.Name == jobspb.AutoStatsName {
// Don't start the job if there is already a CREATE STATISTICS job running.
// (To handle race conditions we check this again after the job starts,
// but this check is used to prevent creating a large number of jobs that
Expand Down Expand Up @@ -266,7 +266,7 @@ func (n *createStatsNode) makeJobRecord(ctx context.Context) (*jobs.Record, erro
statement := tree.AsStringWithFQNames(n, n.p.EvalContext().Annotations)
eventLogStatement := statement
var description string
if n.Name == stats.AutoStatsName {
if n.Name == jobspb.AutoStatsName {
// Use a user-friendly description for automatic statistics.
description = fmt.Sprintf("Table statistics refresh for %s", fqTableName)
} else {
Expand Down Expand Up @@ -495,7 +495,7 @@ var _ jobs.Resumer = &createStatsResumer{}
func (r *createStatsResumer) Resume(ctx context.Context, execCtx interface{}) error {
p := execCtx.(JobExecContext)
details := r.job.Details().(jobspb.CreateStatsDetails)
if details.Name == stats.AutoStatsName {
if details.Name == jobspb.AutoStatsName {
// We want to make sure that an automatic CREATE STATISTICS job only runs if
// there are no other CREATE STATISTICS jobs running, automatic or manual.
if err := checkRunningJobs(ctx, r.job, p); err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/opt/testutils/opttester/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/build/bazel",
"//pkg/jobs/jobspb",
"//pkg/roachpb",
"//pkg/security",
"//pkg/settings/cluster",
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/opt/testutils/opttester/opt_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/build/bazel"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
Expand Down Expand Up @@ -1893,7 +1894,7 @@ func (ot *OptTester) makeStat(
columns []string, rowCount, distinctCount, nullCount uint64,
) stats.JSONStatistic {
return stats.JSONStatistic{
Name: stats.AutoStatsName,
Name: jobspb.AutoStatsName,
CreatedAt: tree.AsStringWithFlags(
&tree.DTimestamp{Time: timeutil.Now()}, tree.FmtBareStrings,
),
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/stats/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/gossip",
"//pkg/jobs/jobspb",
"//pkg/keys",
"//pkg/kv",
"//pkg/kv/kvclient/rangefeed",
Expand Down
12 changes: 4 additions & 8 deletions pkg/sql/stats/automatic_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"math/rand"
"time"

"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -116,11 +117,6 @@ var bufferedChanFullLogLimiter = log.Every(time.Second)
// Constants for automatic statistics collection.
// TODO(rytaft): Should these constants be configurable?
const (
// AutoStatsName is the name to use for statistics created automatically.
// The name is chosen to be something that users are unlikely to choose when
// running CREATE STATISTICS manually.
AutoStatsName = "__auto__"

// defaultAverageTimeBetweenRefreshes is the default time to use as the
// "average" time between refreshes when there is no information for a given
// table.
Expand Down Expand Up @@ -487,7 +483,7 @@ func (r *Refresher) refreshStats(ctx context.Context, tableID descpb.ID, asOf ti
nil, /* txn */
fmt.Sprintf(
"CREATE STATISTICS %s FROM [%d] WITH OPTIONS THROTTLING %g AS OF SYSTEM TIME '-%s'",
AutoStatsName,
jobspb.AutoStatsName,
tableID,
AutomaticStatisticsMaxIdleTime.Get(&r.st.SV),
asOf.String(),
Expand All @@ -501,7 +497,7 @@ func (r *Refresher) refreshStats(ctx context.Context, tableID descpb.ID, asOf ti
func mostRecentAutomaticStat(tableStats []*TableStatistic) *TableStatistic {
// Stats are sorted with the most recent first.
for _, stat := range tableStats {
if stat.Name == AutoStatsName {
if stat.Name == jobspb.AutoStatsName {
return stat
}
}
Expand All @@ -522,7 +518,7 @@ func avgRefreshTime(tableStats []*TableStatistic) time.Duration {
var sum time.Duration
var count int
for _, stat := range tableStats {
if stat.Name != AutoStatsName {
if stat.Name != jobspb.AutoStatsName {
continue
}
if reference == nil {
Expand Down
5 changes: 3 additions & 2 deletions pkg/sql/stats/automatic_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed"
Expand Down Expand Up @@ -275,7 +276,7 @@ func TestAverageRefreshTime(t *testing.T) {
if err != nil {
return err
}
if err := insertStat(txn, AutoStatsName, columnIDsVal, createdAt); err != nil {
if err := insertStat(txn, jobspb.AutoStatsName, columnIDsVal, createdAt); err != nil {
return err
}
}
Expand Down Expand Up @@ -326,7 +327,7 @@ func TestAverageRefreshTime(t *testing.T) {
if err != nil {
return err
}
if err := insertStat(txn, AutoStatsName, columnIDsVal, createdAt); err != nil {
if err := insertStat(txn, jobspb.AutoStatsName, columnIDsVal, createdAt); err != nil {
return err
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/stats/delete_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package stats
import (
"context"

"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
Expand Down Expand Up @@ -64,7 +65,7 @@ func DeleteOldStatsForColumns(
LIMIT $4
)`,
tableID,
AutoStatsName,
jobspb.AutoStatsName,
columnIDsVal,
keepCount,
)
Expand Down
29 changes: 15 additions & 14 deletions pkg/sql/stats/delete_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed"
Expand Down Expand Up @@ -56,7 +57,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 1,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{1},
CreatedAt: timeutil.Now().Add(-1 * time.Hour),
RowCount: 1000,
Expand All @@ -66,7 +67,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 2,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{1},
CreatedAt: timeutil.Now().Add(-2 * time.Hour),
RowCount: 1000,
Expand All @@ -86,7 +87,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 4,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{1},
CreatedAt: timeutil.Now().Add(-4 * time.Hour),
RowCount: 1000,
Expand All @@ -96,7 +97,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 5,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{1},
CreatedAt: timeutil.Now().Add(-5 * time.Hour),
RowCount: 1000,
Expand All @@ -106,7 +107,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 6,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{1},
CreatedAt: timeutil.Now().Add(-6 * time.Hour),
RowCount: 1000,
Expand All @@ -116,7 +117,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 7,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{2, 3},
CreatedAt: timeutil.Now().Add(-7 * time.Hour),
RowCount: 1000,
Expand All @@ -126,7 +127,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 8,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{2, 3},
CreatedAt: timeutil.Now().Add(-8 * time.Hour),
RowCount: 1000,
Expand All @@ -146,7 +147,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 10,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{2, 3},
CreatedAt: timeutil.Now().Add(-10 * time.Hour),
RowCount: 1000,
Expand All @@ -156,7 +157,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 11,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{2, 3},
CreatedAt: timeutil.Now().Add(-11 * time.Hour),
RowCount: 1000,
Expand All @@ -166,7 +167,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 12,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{2, 3},
CreatedAt: timeutil.Now().Add(-12 * time.Hour),
RowCount: 1000,
Expand All @@ -176,7 +177,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 13,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{2},
CreatedAt: timeutil.Now().Add(-13 * time.Hour),
RowCount: 1000,
Expand All @@ -196,7 +197,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(100),
StatisticID: 15,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{3, 2},
CreatedAt: timeutil.Now().Add(-15 * time.Hour),
RowCount: 1000,
Expand All @@ -216,7 +217,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
{
TableID: descpb.ID(102),
StatisticID: 17,
Name: AutoStatsName,
Name: jobspb.AutoStatsName,
ColumnIDs: []descpb.ColumnID{2, 3},
CreatedAt: timeutil.Now().Add(-17 * time.Hour),
RowCount: 0,
Expand Down Expand Up @@ -315,7 +316,7 @@ func TestDeleteOldStatsForColumns(t *testing.T) {
if !reflect.DeepEqual(stat.ColumnIDs, columnIDs) {
continue
}
if stat.Name == AutoStatsName && keptStats < keepCount {
if stat.Name == jobspb.AutoStatsName && keptStats < keepCount {
keptStats++
continue
}
Expand Down

0 comments on commit 59756c4

Please sign in to comment.