Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
49697: server: use unique fake node ID in StartTenant r=asubiotto a=tbg

We had previously hard-coded a NodeID of 1 (matching the underlying
TestServer's NodeID) to make "things work" for SQL tenants. The
commits in this lifted this restriction, so we now use a (static)
NodeID which is highly unlikely to match any NodeID from the KV
layer.

The main work item was to make sure DistSQL does not schedule any
flows.
There are many places in the SQL codebase that construct flows and it
was difficult to ensure that none of them attempt to schedule a nonlocal
one. (We do error out when we attempt to SetupFlow them, though).

Release note: None

49823: adding myself to AUTHORS r=DrewKimball a=DrewKimball



49880: colexec: simplify .gitignore r=yuzefovich a=yuzefovich

This commit makes git ignore all files in `colexec` folder that end with
`.eg.go` which improves the environment setup for the cases when we
change the file names as well as simplifies the maintenance.

Release note: None

Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
  • Loading branch information
4 people committed Jun 5, 2020
4 parents d5bd854 + 5ccec3f + 89fe4be + 0b19f64 commit b0c3eca
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 52 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ Dmitry Saveliev <d.e.saveliev@gmail.com>
Dmitry Vorobev <dimahabr@gmail.com>
Dominique Luna <dluna132@gmail.com>
Dong Liang <dong.liang@rubrik.com>
Drew Kimball <andrewekimball@gmail.com> <drewk@cockroachlabs.com>
Dustin Hiatt <dustin.hiatt@rkinetic.com>
Eamon Zhang <zhang.eamon@hotmail.com> EamonZhang <zhang.eamon@hotmail.com>
Eli Lindsey <eli@siliconsprawl.com>
Expand Down
4 changes: 3 additions & 1 deletion pkg/server/server_sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestSQLServer(t *testing.T) {
defer leaktest.AfterTest(t)()
ctx := context.Background()

tc := serverutils.StartTestCluster(t, 1, base.TestClusterArgs{})
tc := serverutils.StartTestCluster(t, 3, base.TestClusterArgs{})
defer tc.Stopper().Stop(ctx)

db := serverutils.StartTenant(t, tc.Server(0), base.TestTenantArgs{TenantID: roachpb.MakeTenantID(10)})
Expand All @@ -49,6 +49,8 @@ func TestSQLServer(t *testing.T) {
r.Exec(t, `CREATE DATABASE foo`)
r.Exec(t, `CREATE TABLE foo.kv (k STRING PRIMARY KEY, v STRING)`)
r.Exec(t, `INSERT INTO foo.kv VALUES('foo', 'bar')`)
// Cause an index backfill operation.
r.Exec(t, `CREATE INDEX ON foo.kv (v)`)
t.Log(sqlutils.MatrixToStr(r.QueryStr(t, `SET distsql=off; SELECT * FROM foo.kv`)))
t.Log(sqlutils.MatrixToStr(r.QueryStr(t, `SET distsql=auto; SELECT * FROM foo.kv`)))
}
Expand Down
5 changes: 1 addition & 4 deletions pkg/server/testserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,7 @@ func (d dummyProtectedTSProvider) Protect(context.Context, *kv.Txn, *ptpb.Record
return errors.New("fake protectedts.Provider")
}

// TODO(asubiotto): Jobs don't play well with a weird node ID in a multitenant
// environment, so a node ID of 1 is used here to get tests to pass. Fixing
// this is tracked in https://github.com/cockroachdb/cockroach/issues/47892.
const fakeNodeID = roachpb.NodeID(1)
const fakeNodeID = roachpb.NodeID(123456789)

func makeSQLServerArgs(
stopper *stop.Stopper, kvClusterName string, baseCfg BaseConfig, sqlCfg SQLConfig,
Expand Down
44 changes: 1 addition & 43 deletions pkg/sql/colexec/.gitignore
Original file line number Diff line number Diff line change
@@ -1,43 +1 @@
and_or_projection.eg.go
any_not_null_agg.eg.go
avg_agg.eg.go
bool_and_or_agg.eg.go
cast.eg.go
const.eg.go
count_agg.eg.go
distinct.eg.go
hashjoiner.eg.go
hashtable_distinct.eg.go
hashtable_full_default.eg.go
hashtable_full_deleting.eg.go
hash_utils.eg.go
hash_aggregator.eg.go
like_ops.eg.go
mergejoinbase.eg.go
mergejoiner_exceptall.eg.go
mergejoiner_fullouter.eg.go
mergejoiner_inner.eg.go
mergejoiner_intersectall.eg.go
mergejoiner_leftanti.eg.go
mergejoiner_leftouter.eg.go
mergejoiner_leftsemi.eg.go
mergejoiner_rightouter.eg.go
min_max_agg.eg.go
ordered_synchronizer.eg.go
overloads_test_utils.eg.go
proj_const_left_ops.eg.go
proj_const_right_ops.eg.go
proj_non_const_ops.eg.go
quicksort.eg.go
rank.eg.go
relative_rank.eg.go
row_number.eg.go
rowstovec.eg.go
selection_ops.eg.go
select_in.eg.go
sort.eg.go
substring.eg.go
sum_agg.eg.go
values_differ.eg.go
vec_comparators.eg.go
window_peer_grouper.eg.go
*.eg.go
6 changes: 5 additions & 1 deletion pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -3213,10 +3213,14 @@ func (dsp *DistSQLPlanner) createPlanForExport(

// NewPlanningCtx returns a new PlanningCtx. When distribute is false, a
// lightweight version PlanningCtx is returned that can be used when the caller
// knows plans will only be run on one node.
// knows plans will only be run on one node. It is coerced to false on SQL
// SQL tenants (in which case only local planning is supported), regardless of
// the passed-in value.
func (dsp *DistSQLPlanner) NewPlanningCtx(
ctx context.Context, evalCtx *extendedEvalContext, txn *kv.Txn, distribute bool,
) *PlanningCtx {
// Tenants can not distribute plans.
distribute = distribute && evalCtx.Codec.ForSystemTenant()
planCtx := &PlanningCtx{
ctx: ctx,
ExtendedEvalCtx: evalCtx,
Expand Down
12 changes: 9 additions & 3 deletions pkg/sql/distsql_physical_planner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,9 @@ func TestPartitionSpans(t *testing.T) {
},
}

planCtx := dsp.NewPlanningCtx(context.Background(), nil /* evalCtx */, nil /* txn */, true /* distribute */)
planCtx := dsp.NewPlanningCtx(context.Background(), &extendedEvalContext{
EvalContext: tree.EvalContext{Codec: keys.SystemSQLCodec},
}, nil /* txn */, true /* distribute */)
var spans []roachpb.Span
for _, s := range tc.spans {
spans = append(spans, roachpb.Span{Key: roachpb.Key(s[0]), EndKey: roachpb.Key(s[1])})
Expand Down Expand Up @@ -1017,7 +1019,9 @@ func TestPartitionSpansSkipsIncompatibleNodes(t *testing.T) {
},
}

planCtx := dsp.NewPlanningCtx(context.Background(), nil /* evalCtx */, nil /* txn */, true /* distribute */)
planCtx := dsp.NewPlanningCtx(context.Background(), &extendedEvalContext{
EvalContext: tree.EvalContext{Codec: keys.SystemSQLCodec},
}, nil /* txn */, true /* distribute */)
partitions, err := dsp.PartitionSpans(planCtx, roachpb.Spans{span})
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1113,7 +1117,9 @@ func TestPartitionSpansSkipsNodesNotInGossip(t *testing.T) {
},
}

planCtx := dsp.NewPlanningCtx(context.Background(), nil /* evalCtx */, nil /* txn */, true /* distribute */)
planCtx := dsp.NewPlanningCtx(context.Background(), &extendedEvalContext{
EvalContext: tree.EvalContext{Codec: keys.SystemSQLCodec},
}, nil /* txn */, true /* distribute */)
partitions, err := dsp.PartitionSpans(planCtx, roachpb.Spans{span})
if err != nil {
t.Fatal(err)
Expand Down

0 comments on commit b0c3eca

Please sign in to comment.