Skip to content

Commit

Permalink
sql: remove vectorize=201auto option
Browse files Browse the repository at this point in the history
`201auto` option of `vectorize` setting has been removed since it no
longer makes sense to keep (it was introduced as an escape hatch for
20.2 release). Note that we don't need to bump the distsql version
because of this change since it is backwards compatible - if the gateway
is running the old version that has `vectorize=201auto` set, then we
will check whether flows for all nodes don't have non-streaming
operators and possibly use `off` option on the flow setup request, then
the newer version remote node will check the vectorize setting on the
request whether it is not `off` and setup the vectorized flow if it is
not.

Release note (sql change): `201auto` value for `vectorize` session
variable and the corresponding cluster setting has been removed.
  • Loading branch information
yuzefovich committed Oct 27, 2020
1 parent e1709ec commit 60156fb
Show file tree
Hide file tree
Showing 17 changed files with 74 additions and 218 deletions.
49 changes: 8 additions & 41 deletions pkg/sql/colexec/colbuilder/execplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,8 @@ func needHashAggregator(aggSpec *execinfrapb.AggregatorSpec) (bool, error) {
// isSupported checks whether we have a columnar operator equivalent to a
// processor described by spec. Note that it doesn't perform any other checks
// (like validity of the number of inputs).
func isSupported(mode sessiondatapb.VectorizeExecMode, spec *execinfrapb.ProcessorSpec) error {
func isSupported(spec *execinfrapb.ProcessorSpec) error {
core := spec.Core
isFullVectorization := mode == sessiondatapb.VectorizeOn ||
mode == sessiondatapb.VectorizeExperimentalAlways

switch {
case core.Noop != nil:
return nil
Expand Down Expand Up @@ -190,11 +187,6 @@ func isSupported(mode sessiondatapb.VectorizeExecMode, spec *execinfrapb.Process
if core.Distinct.ErrorOnDup != "" {
return errors.Newf("distinct with error on duplicates not supported")
}
if !isFullVectorization {
if len(core.Distinct.OrderedColumns) < len(core.Distinct.DistinctColumns) {
return errors.Newf("unordered distinct can only run in vectorize 'on' mode")
}
}
return nil

case core.Ordinality != nil:
Expand Down Expand Up @@ -246,12 +238,6 @@ func isSupported(mode sessiondatapb.VectorizeExecMode, spec *execinfrapb.Process
if _, supported := SupportedWindowFns[*wf.Func.WindowFunc]; !supported {
return errors.Newf("window function %s is not supported", wf.String())
}
if !isFullVectorization {
switch *wf.Func.WindowFunc {
case execinfrapb.WindowerSpec_PERCENT_RANK, execinfrapb.WindowerSpec_CUME_DIST:
return errors.Newf("window function %s can only run in vectorize 'on' mode", wf.String())
}
}
}
return nil

Expand Down Expand Up @@ -431,8 +417,6 @@ func (r opResult) createAndWrapRowSource(
}
if spec.Core.JoinReader == nil {
switch flowCtx.EvalCtx.SessionData.VectorizeMode {
case sessiondatapb.Vectorize201Auto:
return errors.New("rowexec processor wrapping for non-JoinReader core unsupported in vectorize=201auto mode")
case sessiondatapb.VectorizeExperimentalAlways:
return causeToWrap
}
Expand Down Expand Up @@ -477,11 +461,7 @@ func (r opResult) createAndWrapRowSource(
if err != nil {
return err
}
// We say that the wrapped processor is "streaming" because it is not a
// buffering operator (even if it is a buffering processor). This is not a
// problem for memory accounting because each processor does that on its
// own, so the used memory will be accounted for.
r.Op, r.IsStreaming = c, true
r.Op = c
r.MetadataSources = append(r.MetadataSources, c)
r.ToClose = append(r.ToClose, c)
return nil
Expand Down Expand Up @@ -551,18 +531,11 @@ func NewColOperator(
core := &spec.Core
post := &spec.Post

// By default, we safely assume that an operator is not streaming. Note that
// projections, renders, filters, limits, offsets as well as all internal
// operators (like stats collectors and cancel checkers) are streaming, so in
// order to determine whether the resulting chain of operators is streaming,
// it is sufficient to look only at the "core" operator.
result.IsStreaming = false

// resultPreSpecPlanningStateShallowCopy is a shallow copy of the result
// before any specs are planned. Used if there is a need to backtrack.
resultPreSpecPlanningStateShallowCopy := r

if err = isSupported(flowCtx.EvalCtx.SessionData.VectorizeMode, spec); err != nil {
if err = isSupported(spec); err != nil {
// We refuse to wrap LocalPlanNode processor (which is a DistSQL wrapper
// around a planNode) because it creates complications, and a flow with
// such processor probably will not benefit from the vectorization.
Expand Down Expand Up @@ -620,7 +593,7 @@ func NewColOperator(
if err := checkNumIn(inputs, 1); err != nil {
return r, err
}
result.Op, result.IsStreaming = colexec.NewNoop(inputs[0]), true
result.Op = colexec.NewNoop(inputs[0])
result.ColumnTypes = make([]*types.T, len(spec.Input[0].ColumnTypes))
copy(result.ColumnTypes, spec.Input[0].ColumnTypes)

Expand All @@ -631,7 +604,7 @@ func NewColOperator(
if core.Values.NumRows != 0 {
return r, errors.AssertionFailedf("values core only with zero rows supported, %d given", core.Values.NumRows)
}
result.Op, result.IsStreaming = colexec.NewZeroOpNoInput(), true
result.Op = colexec.NewZeroOpNoInput()
result.ColumnTypes = make([]*types.T, len(core.Values.Columns))
for i, col := range core.Values.Columns {
result.ColumnTypes[i] = col.Type
Expand All @@ -645,7 +618,7 @@ func NewColOperator(
if err != nil {
return r, err
}
result.Op, result.IsStreaming = scanOp, true
result.Op = scanOp
result.IOReader = scanOp
result.MetadataSources = append(result.MetadataSources, scanOp)
// colBatchScan is wrapped with a cancel checker below, so we need to
Expand Down Expand Up @@ -675,13 +648,13 @@ func NewColOperator(
// TODO(solon): The distsql plan for this case includes a TableReader, so
// we end up creating an orphaned colBatchScan. We should avoid that.
// Ideally the optimizer would not plan a scan in this unusual case.
result.Op, result.IsStreaming, err = colexec.NewSingleTupleNoInputOp(streamingAllocator), true, nil
result.Op, err = colexec.NewSingleTupleNoInputOp(streamingAllocator), nil
// We make ColumnTypes non-nil so that sanity check doesn't panic.
result.ColumnTypes = []*types.T{}
break
}
if aggSpec.IsRowCount() {
result.Op, result.IsStreaming, err = colexec.NewCountOp(streamingAllocator, inputs[0]), true, nil
result.Op, err = colexec.NewCountOp(streamingAllocator, inputs[0]), nil
result.ColumnTypes = []*types.T{types.Int}
break
}
Expand Down Expand Up @@ -727,7 +700,6 @@ func NewColOperator(
streamingAllocator, streamingMemAccount, inputs[0], inputTypes, aggSpec,
evalCtx, constructors, constArguments, result.ColumnTypes, aggSpec.IsScalar(),
)
result.IsStreaming = true
}
result.ToClose = append(result.ToClose, result.Op.(colexecbase.Closer))

Expand All @@ -739,7 +711,6 @@ func NewColOperator(
copy(result.ColumnTypes, spec.Input[0].ColumnTypes)
if len(core.Distinct.OrderedColumns) == len(core.Distinct.DistinctColumns) {
result.Op, err = colexec.NewOrderedDistinct(inputs[0], core.Distinct.OrderedColumns, result.ColumnTypes)
result.IsStreaming = true
} else {
distinctMemAccount := streamingMemAccount
if !useStreamingMemAccountForBuffering {
Expand Down Expand Up @@ -768,7 +739,6 @@ func NewColOperator(
}
outputIdx := len(spec.Input[0].ColumnTypes)
result.Op = colexec.NewOrdinalityOp(streamingAllocator, inputs[0], outputIdx)
result.IsStreaming = true
result.ColumnTypes = appendOneType(spec.Input[0].ColumnTypes, types.Int)

case core.HashJoiner != nil:
Expand Down Expand Up @@ -887,9 +857,6 @@ func NewColOperator(
if err := checkNumIn(inputs, 2); err != nil {
return r, err
}
// Merge joiner is a streaming operator when equality columns form a key
// for both of the inputs.
result.IsStreaming = core.MergeJoiner.LeftEqColumnsAreKey && core.MergeJoiner.RightEqColumnsAreKey

leftTypes := make([]*types.T, len(spec.Input[0].ColumnTypes))
copy(leftTypes, spec.Input[0].ColumnTypes)
Expand Down
7 changes: 3 additions & 4 deletions pkg/sql/colexec/op_creation.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ type NewColOperatorResult struct {
InternalMemUsage int
MetadataSources []execinfrapb.MetadataSource
// ToClose is a slice of components that need to be Closed.
ToClose []colexecbase.Closer
IsStreaming bool
OpMonitors []*mon.BytesMonitor
OpAccounts []*mon.BoundAccount
ToClose []colexecbase.Closer
OpMonitors []*mon.BytesMonitor
OpAccounts []*mon.BoundAccount
}
1 change: 0 additions & 1 deletion pkg/sql/colflow/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ go_library(
"//pkg/sql/pgwire/pgcode",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/rowexec",
"//pkg/sql/sessiondatapb",
"//pkg/sql/types",
"//pkg/util",
"//pkg/util/log",
Expand Down
14 changes: 0 additions & 14 deletions pkg/sql/colflow/vectorized_flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/rowexec"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -1034,11 +1033,6 @@ func (s *vectorizedFlowCreator) setupFlow(
if flowCtx.EvalCtx.SessionData.TestingVectorizeInjectPanics {
result.Op = colexec.NewPanicInjector(result.Op)
}
if flowCtx.EvalCtx.SessionData.VectorizeMode == sessiondatapb.Vectorize201Auto &&
!result.IsStreaming {
err = errors.Errorf("non-streaming operator encountered when vectorize=201auto")
return
}
// We created a streaming memory account when calling NewColOperator above,
// so there is definitely at least one memory account, and it doesn't
// matter which one we grow.
Expand Down Expand Up @@ -1076,14 +1070,6 @@ func (s *vectorizedFlowCreator) setupFlow(
}
}

if (flowCtx.EvalCtx.SessionData.VectorizeMode == sessiondatapb.Vectorize201Auto) &&
pspec.Output[0].Type == execinfrapb.OutputRouterSpec_BY_HASH {
// colexec.HashRouter is not supported when vectorize=auto since it can
// buffer an unlimited number of tuples, even though it falls back to
// disk. vectorize=on does support this.
err = errors.Errorf("hash router encountered when vectorize=201auto")
return
}
if err = s.setupOutput(
ctx, flowCtx, pspec, op, result.ColumnTypes, metadataSourcesQueue, toClose, factory,
); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/distsql_running.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (dsp *DistSQLPlanner) setupFlows(
}

if evalCtx.SessionData.VectorizeMode != sessiondatapb.VectorizeOff {
if !vectorizeThresholdMet && (evalCtx.SessionData.VectorizeMode == sessiondatapb.Vectorize201Auto || evalCtx.SessionData.VectorizeMode == sessiondatapb.VectorizeOn) {
if !vectorizeThresholdMet && evalCtx.SessionData.VectorizeMode == sessiondatapb.VectorizeOn {
// Vectorization is not justified for this flow because the expected
// amount of data is too small and the overhead of pre-allocating data
// structures needed for the vectorized engine is expected to dominate
Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,8 @@ var VectorizeClusterMode = settings.RegisterEnumSetting(
"default vectorize mode",
"on",
map[int64]string{
int64(sessiondatapb.VectorizeOff): "off",
int64(sessiondatapb.Vectorize201Auto): "201auto",
int64(sessiondatapb.VectorizeOn): "on",
int64(sessiondatapb.VectorizeOff): "off",
int64(sessiondatapb.VectorizeOn): "on",
},
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/explain_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func explainGetDistributedAndVectorized(
vectorizedThresholdMet := physicalPlan.MaxEstimatedRowCount >= ctxSessionData.VectorizeRowCountThreshold
if ctxSessionData.VectorizeMode == sessiondatapb.VectorizeOff {
willVectorize = false
} else if !vectorizedThresholdMet && (ctxSessionData.VectorizeMode == sessiondatapb.Vectorize201Auto || ctxSessionData.VectorizeMode == sessiondatapb.VectorizeOn) {
} else if !vectorizedThresholdMet && ctxSessionData.VectorizeMode == sessiondatapb.VectorizeOn {
willVectorize = false
} else {
willVectorize = true
Expand Down
45 changes: 0 additions & 45 deletions pkg/sql/logictest/logic.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,12 +489,6 @@ var logicTestConfigs = []testClusterConfig{
binaryVersion: roachpb.Version{Major: 1, Minor: 1},
disableUpgrade: true,
},
{
name: "local-vec-auto",
numNodes: 1,
overrideAutoStats: "false",
overrideVectorize: "201auto",
},
{
name: "local-mixed-19.2-20.1",
numNodes: 1,
Expand Down Expand Up @@ -535,24 +529,6 @@ var logicTestConfigs = []testClusterConfig{
overrideAutoStats: "false",
overrideVectorize: "off",
},
{
name: "fakedist-vec-auto",
numNodes: 3,
useFakeSpanResolver: true,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
},
{
name: "fakedist-vec-auto-disk",
numNodes: 3,
useFakeSpanResolver: true,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
sqlExecUseDisk: true,
skipShort: true,
},
{
name: "fakedist-metadata",
numNodes: 3,
Expand Down Expand Up @@ -585,22 +561,6 @@ var logicTestConfigs = []testClusterConfig{
overrideDistSQLMode: "on",
overrideAutoStats: "false",
},
{
name: "5node-vec-auto",
numNodes: 5,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
},
{
name: "5node-vec-disk-auto",
numNodes: 5,
overrideDistSQLMode: "on",
overrideAutoStats: "false",
overrideVectorize: "201auto",
sqlExecUseDisk: true,
skipShort: true,
},
{
name: "5node-metadata",
numNodes: 5,
Expand Down Expand Up @@ -733,12 +693,9 @@ var (
defaultConfigNames = []string{
"local",
"local-vec-off",
"local-vec-auto",
"local-spec-planning",
"fakedist",
"fakedist-vec-off",
"fakedist-vec-auto",
"fakedist-vec-auto-disk",
"fakedist-metadata",
"fakedist-disk",
"fakedist-spec-planning",
Expand All @@ -747,8 +704,6 @@ var (
fiveNodeDefaultConfigName = "5node-default-configs"
fiveNodeDefaultConfigNames = []string{
"5node",
"5node-vec-auto",
"5node-vec-disk-auto",
"5node-metadata",
"5node-disk",
"5node-spec-planning",
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/column_families
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# LogicTest: local local-vec-auto 3node-tenant
# LogicTest: local 3node-tenant

# Test that different operations still succeed when the primary key is not in column family 0.

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/explain
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# LogicTest: local local-vec-off local-vec-auto local-spec-planning
# LogicTest: local local-vec-off local-spec-planning

statement ok
CREATE TABLE t (a INT PRIMARY KEY)
Expand Down
20 changes: 0 additions & 20 deletions pkg/sql/logictest/testdata/logic_test/merge_join_192

This file was deleted.

3 changes: 0 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/set
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,6 @@ SET distsql = off
statement error invalid value for parameter "distsql": "bogus"
SET distsql = bogus

statement ok
SET vectorize = '201auto'

statement ok
SET vectorize = on

Expand Down
4 changes: 0 additions & 4 deletions pkg/sql/sessiondatapb/session_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ func (m VectorizeExecMode) String() string {
switch m {
case VectorizeOff:
return "off"
case Vectorize201Auto:
return "201auto"
case VectorizeOn:
return "on"
case VectorizeExperimentalAlways:
Expand All @@ -98,8 +96,6 @@ func VectorizeExecModeFromString(val string) (VectorizeExecMode, bool) {
switch strings.ToUpper(val) {
case "OFF":
m = VectorizeOff
case "201AUTO":
m = Vectorize201Auto
case "ON":
m = VectorizeOn
case "EXPERIMENTAL_ALWAYS":
Expand Down
Loading

0 comments on commit 60156fb

Please sign in to comment.