Skip to content

Commit

Permalink
sql: remove two private settings
Browse files Browse the repository at this point in the history
This commit removes two private settings that could disable planning of
multiple join readers (i.e would disable distribution) and disable
planning merge joiners (i.e. would simply prohibit the use of merge
joiners). It also retires two settings that have been removed after 20.1
was cut.

Release note: None
  • Loading branch information
yuzefovich committed Jun 1, 2020
1 parent 1422dd5 commit 9d7d646
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 28 deletions.
6 changes: 5 additions & 1 deletion pkg/settings/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,12 @@ var retiredSettings = map[string]struct{}{
// removed as of 20.1.
"schemachanger.lease.duration": {},
"schemachanger.lease.renew_fraction": {},
// removes as of 20.2.
// removed as of 20.2.
"rocksdb.ingest_backpressure.pending_compaction_threshold": {},
"sql.distsql.temp_storage.joins": {},
"sql.distsql.temp_storage.sorts": {},
"sql.distsql.distribute_index_joins": {},
"sql.distsql.merge_joins.enabled": {},
}

// register adds a setting to the registry.
Expand Down
32 changes: 5 additions & 27 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/rpc"
"github.com/cockroachdb/cockroach/pkg/rpc/nodedialer"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/distsql"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
Expand Down Expand Up @@ -110,22 +109,6 @@ var ReplicaOraclePolicy = replicaoracle.BinPackingChoice
// debugging).
var logPlanDiagram = envutil.EnvOrDefaultBool("COCKROACH_DISTSQL_LOG_PLAN", false)

// If true, for index joins we instantiate a join reader on every node that
// has a stream (usually from a table reader). If false, there is a single join
// reader.
var distributeIndexJoin = settings.RegisterBoolSetting(
"sql.distsql.distribute_index_joins",
"if set, for index joins we instantiate a join reader on every node that has a "+
"stream; if not set, we use a single join reader",
true,
)

var planMergeJoins = settings.RegisterBoolSetting(
"sql.distsql.merge_joins.enabled",
"if set, we plan merge joins when possible",
true,
)

// NewDistSQLPlanner initializes a DistSQLPlanner.
//
// nodeDesc is the descriptor of the node on which this planner runs. It is used
Expand Down Expand Up @@ -1861,7 +1844,7 @@ func (dsp *DistSQLPlanner) createPlanForIndexJoin(
if err != nil {
return PhysicalPlan{}, err
}
if distributeIndexJoin.Get(&dsp.st.SV) && len(plan.ResultRouters) > 1 {
if len(plan.ResultRouters) > 1 {
// Instantiate one join reader for every stream.
plan.AddNoGroupingStage(
execinfrapb.ProcessorCoreUnion{JoinReader: &joinReaderSpec},
Expand All @@ -1870,14 +1853,9 @@ func (dsp *DistSQLPlanner) createPlanForIndexJoin(
dsp.convertOrdering(n.reqOrdering, plan.PlanToStreamColMap),
)
} else {
// Use a single join reader (if there is a single stream, on that node; if
// not, on the gateway node).
node := dsp.nodeDesc.NodeID
if len(plan.ResultRouters) == 1 {
node = plan.Processors[plan.ResultRouters[0]].Node
}
// We have a single stream, so use a single join reader on that node.
plan.AddSingleGroupStage(
node,
plan.Processors[plan.ResultRouters[0]].Node,
execinfrapb.ProcessorCoreUnion{JoinReader: &joinReaderSpec},
post,
types,
Expand Down Expand Up @@ -2212,7 +2190,7 @@ func (dsp *DistSQLPlanner) createPlanForJoin(
if numEq := len(n.pred.leftEqualityIndices); numEq != 0 {
nodes = findJoinProcessorNodes(leftRouters, rightRouters, p.Processors)

if planMergeJoins.Get(&dsp.st.SV) && len(n.mergeJoinOrdering) > 0 {
if len(n.mergeJoinOrdering) > 0 {
// TODO(radu): we currently only use merge joins when we have an ordering on
// all equality columns. We should relax this by either:
// - implementing a hybrid hash/merge processor which implements merge
Expand Down Expand Up @@ -3025,7 +3003,7 @@ func (dsp *DistSQLPlanner) createPlanForSetOp(
// group uses a hashmap on the remaining columns
// - or: adding a sort processor to complete the order
var core execinfrapb.ProcessorCoreUnion
if !planMergeJoins.Get(&dsp.st.SV) || len(mergeOrdering.Columns) < len(streamCols) {
if len(mergeOrdering.Columns) < len(streamCols) {
core.HashJoiner = &execinfrapb.HashJoinerSpec{
LeftEqColumns: eqCols,
RightEqColumns: eqCols,
Expand Down

0 comments on commit 9d7d646

Please sign in to comment.