Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
87449: workload,ttl: add TTL workload for benchmarking time to finish r=rafiss a=ecwall

fixes #88172

Measures time row-level TTL job takes to run on a table:
1) Drop TTL table IF EXISTS.
2) Create a table without TTL.
3) Insert initialRowCount number of rows.
4) Gets number of rows that should expire.
5) Wait for table ranges to stabilize after scattering.
6) Enable TTL on table.
7) Poll table until TTL job is complete.
Note: Ops is a no-op and no histograms are used.
Benchmarking is done inside Hooks and details are logged.

Adds useDistSQL field to TTL job progress protobuf for
visibility into which version was run during cluster
upgrades.

Release justification: Added TTL workload.

Release note: None

89317: sql,tree: improve function resolution efficiency r=ajwerner a=ajwerner

#### sql: prevent allocations by avoiding some name pointers

We don't need pointers for these names. They generally won't escape.

#### sql,tree: change SearchPath to avoid allocations
The closure-oriented interface was forcing the closures and the variables they
referenced to escape to the heap. This change, while not beautiful, ends up being
much more efficient.

```
name                                         old time/op    new time/op    delta
SQL/MultinodeCockroach/Upsert/count=1000-16    20.4ms ±11%    18.9ms ± 8%   -7.47%  (p=0.000 n=20+19)

name                                         old alloc/op   new alloc/op   delta
SQL/MultinodeCockroach/Upsert/count=1000-16    10.1MB ±29%     9.8MB ±29%     ~     (p=0.231 n=20+20)

name                                         old allocs/op  new allocs/op  delta
SQL/MultinodeCockroach/Upsert/count=1000-16     56.3k ± 7%     50.2k ±10%  -10.81%  (p=0.000 n=19+19)
```

Release note: None

89333: backupccl: enable `restore_span.target_size` r=dt,stevendanna a=adityamaru

This setting was previously disabled because of timeouts being observed when restoring our TPCCInc fixtures. The cause of those timeouts has been identified as
#88329 making it safe to re-enable merging of spans during restore. This settings prevents restore from over-splitting and leaving the cluster with a merge hangover post restore.

Informs: #86470

Release note (sql change): Sets `backup.restore_span.target_size` to default to 384 MiB so that restore merges upto that size of spans when reading from the backup before actually ingesting data. This should reduce the number of ranges created during restore and thereby reduce the merging of ranges that needs to occur post restore.

Co-authored-by: Evan Wall <wall@cockroachlabs.com>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
Co-authored-by: adityamaru <adityamaru@gmail.com>
  • Loading branch information
4 people committed Oct 4, 2022
4 parents fc50f10 + 1873aab + e99a182 + 132e195 commit 45f02ba
Show file tree
Hide file tree
Showing 32 changed files with 551 additions and 82 deletions.
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -70,6 +70,7 @@ require (
github.com/fsnotify/fsnotify v1.5.1
github.com/getsentry/sentry-go v0.12.0
github.com/ghemawat/stream v0.0.0-20171120220530-696b145b53b9
github.com/go-openapi/strfmt v0.20.2
github.com/go-sql-driver/mysql v1.6.0
github.com/go-swagger/go-swagger v0.26.1
github.com/gogo/protobuf v1.3.2
Expand Down Expand Up @@ -239,7 +240,6 @@ require (
github.com/go-openapi/loads v0.20.2 // indirect
github.com/go-openapi/runtime v0.19.29 // indirect
github.com/go-openapi/spec v0.20.3 // indirect
github.com/go-openapi/strfmt v0.20.2 // indirect
github.com/go-openapi/swag v0.19.15 // indirect
github.com/go-openapi/validate v0.20.2 // indirect
github.com/go-stack/stack v1.8.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Expand Up @@ -2112,6 +2112,7 @@ GO_TARGETS = [
"//pkg/workload/tpccchecks:tpccchecks",
"//pkg/workload/tpcds:tpcds",
"//pkg/workload/tpch:tpch",
"//pkg/workload/ttlbench:ttlbench",
"//pkg/workload/ttllogger:ttllogger",
"//pkg/workload/workloadimpl:workloadimpl",
"//pkg/workload/workloadimpl:workloadimpl_test",
Expand Down Expand Up @@ -3040,6 +3041,7 @@ GET_X_DATA_TARGETS = [
"//pkg/workload/tpccchecks:get_x_data",
"//pkg/workload/tpcds:get_x_data",
"//pkg/workload/tpch:get_x_data",
"//pkg/workload/ttlbench:get_x_data",
"//pkg/workload/ttllogger:get_x_data",
"//pkg/workload/workloadimpl:get_x_data",
"//pkg/workload/workloadsql:get_x_data",
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/restore_span_covering.go
Expand Up @@ -50,7 +50,7 @@ var targetRestoreSpanSize = settings.RegisterByteSizeSetting(
settings.TenantWritable,
"backup.restore_span.target_size",
"target size to which base spans of a restore are merged to produce a restore span (0 disables)",
0, //TODO(dt): make this something like 384 << 20,
384<<20,
)

// makeSimpleImportSpans partitions the spans of requiredSpans into a covering
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/workloadccl/allccl/BUILD.bazel
Expand Up @@ -29,6 +29,7 @@ go_library(
"//pkg/workload/tpccchecks",
"//pkg/workload/tpcds",
"//pkg/workload/tpch",
"//pkg/workload/ttlbench",
"//pkg/workload/ttllogger",
"//pkg/workload/ycsb",
],
Expand Down
1 change: 1 addition & 0 deletions pkg/ccl/workloadccl/allccl/all.go
Expand Up @@ -35,6 +35,7 @@ import (
_ "github.com/cockroachdb/cockroach/pkg/workload/tpccchecks"
_ "github.com/cockroachdb/cockroach/pkg/workload/tpcds"
_ "github.com/cockroachdb/cockroach/pkg/workload/tpch"
_ "github.com/cockroachdb/cockroach/pkg/workload/ttlbench"
_ "github.com/cockroachdb/cockroach/pkg/workload/ttllogger"
_ "github.com/cockroachdb/cockroach/pkg/workload/ycsb"
)
4 changes: 3 additions & 1 deletion pkg/ccl/workloadccl/allccl/all_test.go
Expand Up @@ -71,7 +71,7 @@ func TestAllRegisteredImportFixture(t *testing.T) {
}

switch meta.Name {
case `startrek`, `roachmart`, `interleavedpartitioned`:
case `startrek`, `roachmart`, `interleavedpartitioned`, `ttlbench`:
// These don't work with IMPORT.
continue
case `tpch`:
Expand Down Expand Up @@ -143,6 +143,8 @@ func TestAllRegisteredSetup(t *testing.T) {
case `interleavedpartitioned`:
// This require a specific node locality setup
continue
case `ttlbench`:
continue
}

t.Run(meta.Name, func(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/BUILD.bazel
Expand Up @@ -227,6 +227,7 @@ go_library(
"//pkg/workload/movr",
"//pkg/workload/tpcc",
"//pkg/workload/tpch",
"//pkg/workload/ttlbench",
"//pkg/workload/ttllogger",
"//pkg/workload/workloadsql",
"//pkg/workload/ycsb",
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/cli.go
Expand Up @@ -39,6 +39,7 @@ import (
_ "github.com/cockroachdb/cockroach/pkg/workload/movr" // registers workloads
_ "github.com/cockroachdb/cockroach/pkg/workload/tpcc" // registers workloads
_ "github.com/cockroachdb/cockroach/pkg/workload/tpch" // registers workloads
_ "github.com/cockroachdb/cockroach/pkg/workload/ttlbench" // registers workloads
_ "github.com/cockroachdb/cockroach/pkg/workload/ttllogger" // registers workloads
_ "github.com/cockroachdb/cockroach/pkg/workload/ycsb" // registers workloads
"github.com/cockroachdb/errors"
Expand Down
2 changes: 2 additions & 0 deletions pkg/jobs/jobspb/jobs.proto
Expand Up @@ -1023,6 +1023,8 @@ message RowLevelTTLProgress {
int64 job_row_count = 1;
// ProcessorProgresses is the progress per DistSQL processor.
repeated RowLevelTTLProcessorProgress processor_progresses = 2 [(gogoproto.nullable)=false];

bool use_dist_sql = 3 [(gogoproto.customname) = "UseDistSQL"];
}

message RowLevelTTLProcessorProgress {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/resolver/resolver_bench_test.go
Expand Up @@ -106,7 +106,7 @@ func BenchmarkResolveExistingObject(b *testing.B) {
require.NoError(b, err)
b.ResetTimer()
for i := 0; i < b.N; i++ {
desc, _, err := resolver.ResolveExistingObject(ctx, rs, uon, tc.flags)
desc, _, err := resolver.ResolveExistingObject(ctx, rs, &uon, tc.flags)
require.NoError(b, err)
require.NotNil(b, desc)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/parser/sql.y
Expand Up @@ -13665,7 +13665,6 @@ typed_literal:
// types, return an unimplemented error message.
var typ tree.ResolvableTypeReference
var ok bool
var err error
var unimp int
typ, ok, unimp = types.TypeForNonKeywordTypeName(typName)
if !ok {
Expand All @@ -13674,8 +13673,9 @@ typed_literal:
// In this case, we don't think this type is one of our
// known unsupported types, so make a type reference for it.
aIdx := sqllex.(*lexer).NewAnnotation()
typ, err = name.ToUnresolvedObjectName(aIdx)
un, err := name.ToUnresolvedObjectName(aIdx)
if err != nil { return setErr(sqllex, err) }
typ = &un
case -1:
return unimplemented(sqllex, "type name " + typName)
default:
Expand All @@ -13688,7 +13688,7 @@ typed_literal:
aIdx := sqllex.(*lexer).NewAnnotation()
res, err := name.ToUnresolvedObjectName(aIdx)
if err != nil { return setErr(sqllex, err) }
$$.val = &tree.CastExpr{Expr: tree.NewStrVal($2), Type: res, SyntaxMode: tree.CastPrepend}
$$.val = &tree.CastExpr{Expr: tree.NewStrVal($2), Type: &res, SyntaxMode: tree.CastPrepend}
}
}
| const_typename SCONST
Expand Down
19 changes: 10 additions & 9 deletions pkg/sql/schema_resolver.go
Expand Up @@ -334,7 +334,8 @@ func (sr *schemaResolver) canResolveDescUnderSchema(
case catalog.SchemaUserDefined:
return sr.authAccessor.CheckPrivilegeForUser(ctx, scDesc, privilege.USAGE, sr.sessionDataStack.Top().User())
default:
panic(errors.AssertionFailedf("unknown schema kind %d", kind))
forLog := kind // prevents kind from escaping
panic(errors.AssertionFailedf("unknown schema kind %d", forLog))
}
}

Expand Down Expand Up @@ -400,23 +401,23 @@ func (sr *schemaResolver) ResolveFunction(
sc := prefix.Schema
udfDef, _ = sc.GetResolvedFuncDefinition(fn.Object())
} else {
if err := path.IterateSearchPath(func(schema string) error {
for i, n := 0, path.NumElements(); i < n; i++ {
schema := path.GetSchema(i)
found, prefix, err := sr.LookupSchema(ctx, sr.CurrentDatabase(), schema)
if err != nil {
return err
return nil, err
}
if !found {
return nil
continue
}
curUdfDef, found := prefix.Schema.GetResolvedFuncDefinition(fn.Object())
if !found {
return nil
continue
}

udfDef, err = udfDef.MergeWith(curUdfDef)
return err
}); err != nil {
return nil, err
if err != nil {
return nil, err
}
}
}

Expand Down
1 change: 0 additions & 1 deletion pkg/sql/sem/tree/BUILD.bazel
Expand Up @@ -146,7 +146,6 @@ go_library(
"//pkg/util/encoding",
"//pkg/util/errorutil/unimplemented",
"//pkg/util/ipaddr",
"//pkg/util/iterutil",
"//pkg/util/json",
"//pkg/util/pretty",
"//pkg/util/stringencoding",
Expand Down
27 changes: 10 additions & 17 deletions pkg/sql/sem/tree/function_definition.go
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/iterutil"
"github.com/cockroachdb/errors"
"github.com/lib/pq/oid"
)
Expand Down Expand Up @@ -292,15 +291,10 @@ func (fd *ResolvedFunctionDefinition) MatchOverload(
if explicitSchema != "" {
findMatches(explicitSchema)
} else {
err := searchPath.IterateSearchPath(func(schema string) error {
findMatches(schema)
if found {
return iterutil.StopIteration()
for i, n := 0, searchPath.NumElements(); i < n; i++ {
if findMatches(searchPath.GetSchema(i)); found {
break
}
return nil
})
if err != nil {
return QualifiedOverload{}, err
}
}

Expand Down Expand Up @@ -386,14 +380,15 @@ func QualifyBuiltinFunctionDefinition(
// GetBuiltinFuncDefinitionOrFail is similar to GetBuiltinFuncDefinition but
// returns an error if function is not found.
func GetBuiltinFuncDefinitionOrFail(
fName *FunctionName, searchPath SearchPath,
fName FunctionName, searchPath SearchPath,
) (*ResolvedFunctionDefinition, error) {
def, err := GetBuiltinFuncDefinition(fName, searchPath)
if err != nil {
return nil, err
}
if def == nil {
return nil, errors.Wrapf(ErrFunctionUndefined, "unknown function: %s()", ErrString(fName))
forError := fName // prevent fName from escaping
return nil, errors.Wrapf(ErrFunctionUndefined, "unknown function: %s()", ErrString(&forError))
}
return def, nil
}
Expand All @@ -409,7 +404,7 @@ func GetBuiltinFuncDefinitionOrFail(
// error is still checked and return from the function signature just in case
// we change the iterating function in the future.
func GetBuiltinFuncDefinition(
fName *FunctionName, searchPath SearchPath,
fName FunctionName, searchPath SearchPath,
) (*ResolvedFunctionDefinition, error) {
if fName.ExplicitSchema {
return ResolvedBuiltinFuncDefs[fName.Schema()+"."+fName.Object()], nil
Expand All @@ -429,15 +424,13 @@ func GetBuiltinFuncDefinition(

// If not in pg_catalog, go through search path.
var resolvedDef *ResolvedFunctionDefinition
if err := searchPath.IterateSearchPath(func(schema string) error {
for i, n := 0, searchPath.NumElements(); i < n; i++ {
schema := searchPath.GetSchema(i)
fullName := schema + "." + fName.Object()
if def, ok := ResolvedBuiltinFuncDefs[fullName]; ok {
resolvedDef = def
return iterutil.StopIteration()
break
}
return nil
}); err != nil {
return nil, err
}

return resolvedDef, nil
Expand Down
13 changes: 6 additions & 7 deletions pkg/sql/sem/tree/name_part.go
Expand Up @@ -218,23 +218,22 @@ func MakeUnresolvedName(args ...string) UnresolvedName {
}

// ToUnresolvedObjectName converts an UnresolvedName to an UnresolvedObjectName.
func (u *UnresolvedName) ToUnresolvedObjectName(idx AnnotationIdx) (*UnresolvedObjectName, error) {
func (u *UnresolvedName) ToUnresolvedObjectName(idx AnnotationIdx) (UnresolvedObjectName, error) {
if u.NumParts == 4 {
return nil, pgerror.Newf(pgcode.Syntax, "improper qualified name (too many dotted names): %s", u)
return UnresolvedObjectName{}, pgerror.Newf(pgcode.Syntax, "improper qualified name (too many dotted names): %s", u)
}
return NewUnresolvedObjectName(
return MakeUnresolvedObjectName(
u.NumParts,
[3]string{u.Parts[0], u.Parts[1], u.Parts[2]},
idx,
)
}

// ToFunctionName converts an UnresolvedName to a FunctionName.
func (u *UnresolvedName) ToFunctionName() (*FunctionName, error) {
func (u *UnresolvedName) ToFunctionName() (FunctionName, error) {
un, err := u.ToUnresolvedObjectName(NoAnnotation)
if err != nil {
return nil, errors.Newf("invalid function name: %s", u.String())
return FunctionName{}, errors.Newf("invalid function name: %s", u.String())
}
fn := un.ToFunctionName()
return &fn, nil
return un.ToFunctionName(), nil
}
15 changes: 7 additions & 8 deletions pkg/sql/sem/tree/name_resolution.go
Expand Up @@ -129,22 +129,21 @@ type QualifiedNameResolver interface {
// SearchPath encapsulates the ordered list of schemas in the current database
// to search during name resolution.
type SearchPath interface {
// NumElements returns the number of elements in the SearchPath.
NumElements() int

// IterateSearchPath calls the passed function for every element of the
// SearchPath in order. If an error is returned, iteration stops. If the
// error is iterutil.StopIteration, no error will be returned from the
// method.
IterateSearchPath(func(schema string) error) error
// GetSchema returns the schema at the ord offset in the SearchPath.
// Note that it will return the empty string if the ordinal is out of range.
GetSchema(ord int) string
}

// EmptySearchPath is a SearchPath with no members.
var EmptySearchPath SearchPath = emptySearchPath{}

type emptySearchPath struct{}

func (emptySearchPath) IterateSearchPath(func(string) error) error {
return nil
}
func (emptySearchPath) NumElements() int { return 0 }
func (emptySearchPath) GetSchema(i int) string { return "" }

func newInvColRef(n *UnresolvedName) error {
return pgerror.NewWithDepthf(1, pgcode.InvalidColumnReference,
Expand Down
20 changes: 17 additions & 3 deletions pkg/sql/sem/tree/object_name.go
Expand Up @@ -136,13 +136,26 @@ func (*UnresolvedObjectName) tableExpr() {}
func NewUnresolvedObjectName(
numParts int, parts [3]string, annotationIdx AnnotationIdx,
) (*UnresolvedObjectName, error) {
u := &UnresolvedObjectName{
n, err := MakeUnresolvedObjectName(numParts, parts, annotationIdx)
if err != nil {
return nil, err
}
return &n, nil
}

// MakeUnresolvedObjectName creates an unresolved object name, verifying that it
// is well-formed.
func MakeUnresolvedObjectName(
numParts int, parts [3]string, annotationIdx AnnotationIdx,
) (UnresolvedObjectName, error) {
u := UnresolvedObjectName{
NumParts: numParts,
Parts: parts,
AnnotatedNode: AnnotatedNode{AnnIdx: annotationIdx},
}
if u.NumParts < 1 {
return nil, newInvTableNameError(u)
forErr := u // prevents u from escaping
return UnresolvedObjectName{}, newInvTableNameError(&forErr)
}

// Check that all the parts specified are not empty.
Expand All @@ -154,7 +167,8 @@ func NewUnresolvedObjectName(
}
for i := 0; i < lastCheck; i++ {
if len(u.Parts[i]) == 0 {
return nil, newInvTableNameError(u)
forErr := u // prevents u from escaping
return UnresolvedObjectName{}, newInvTableNameError(&forErr)
}
}
return u, nil
Expand Down
12 changes: 4 additions & 8 deletions pkg/sql/sem/tree/type_check.go
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/cockroach/pkg/util/iterutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil/pgdate"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/redact"
Expand Down Expand Up @@ -2989,23 +2988,20 @@ func getMostSignificantOverload(

found := false
var ret QualifiedOverload
err := searchPath.IterateSearchPath(func(schema string) error {
for i, n := 0, searchPath.NumElements(); i < n; i++ {
schema := searchPath.GetSchema(i)
for i := range overloads {
if overloads[i].(QualifiedOverload).Schema == schema {
if found {
return ambiguousError()
return QualifiedOverload{}, ambiguousError()
}
found = true
ret = overloads[i].(QualifiedOverload)
}
}
if found {
return iterutil.StopIteration()
break
}
return nil
})
if err != nil {
return QualifiedOverload{}, err
}
if !found {
// This should never happen. Otherwise, it means we get function from a
Expand Down

0 comments on commit 45f02ba

Please sign in to comment.