Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
73941: roachtest: make crdb crash on span-use-after-Finish r=andreimatei a=andreimatei

This patch makes roachtest pass an env var to crdb asking it to panic on
mis-use of tracing spans. I've been battling such bugs, which become
more problematic as I'm trying to introduce span reuse. In production
we'll probably continue tolerating such bugs for the time being, but I
want tests to yell. Unit tests are already running with this
use-after-Finish detection, and so far so good. I've done a manual run
of all the roachtests in this configuration and nothing crashed, so I
don't expect a tragedy.

Release note: None

74109: kv: rename gcQueue to mvccGCQueue r=tbg a=nvanbenschoten

This commit renames the "GC queue" to the "MVCC GC queue" (which GC's old MVCC
versions) to avoid confusion with the "replica GC queue" (which GC's abandoned
replicas). We've already been using this terminology in various other contexts
to avoid confusion, so this refactor updates the code to reflect this naming.

This comes in response to #73838, which found a bug that had survived for three
years and was a direct consequence of this ambiguous naming.

The commit doesn't go quite as far as renaming the `pkg/kv/kvserver/gc` package,
but that could be a follow-up to this commit.

74126: geo: move projection data to embedded compressed file r=RaduBerinde a=RaduBerinde

The geoprojbase package embeds projection info as constants, leading
to a 6MB code file. Large code files are undesirable especially from
the perspective of static analysis tools, IDEs, etc.

This change moves the projections data to an embedded json.gz file. We
define the schema of this file in a new `embeddedproj` subpackage. The
data is loaded lazily.

The data file was obtained by modifying the existing constants to fill
out an `embeddedproj.Data`:
https://github.com/RaduBerinde/cockroach/blob/geospatial-proj-data/pkg/geo/geoprojbase/embeddedproj/data_test.go

The `generate-spatial-ref-sys` command is also updated to generate
this file from the `.csv`.

The `make buildshort` binary size is decreased by ~7MB.

Fixes #63969.

Release note: None

74128: cockroach: don't import randgen in binary r=RaduBerinde a=RaduBerinde

The `sql/randgen` package creates a lot of global datums, some of
which use geospatial and require the loading of geospatial data. This
package is meant for testing and should not be part of the cockroach
binary.

This change removes the non-testing uses of randgen.

Tested via `go list -deps ./pkg/cmd/cockroach`. Note that the updated
test is ineffectual for now (tracked by #74119).

Informs #74120.

Release note: None

74159: sql: default index recommendations to be off for logic tests  r=nehageorge a=nehageorge

**sql: refactor GlobalDefault for session variables**

This commit refactors pkg/sql/vars.go to use globalFalse and
globalTrue as the setting GlobalDefault where possible.

Release note: None

**sql: default index recommendations to be off for logic tests**

This commit configures index recommendations to be off for logic tests.
This is to avoid flaky tests, as the index recommendation output can
vary depending on the best plan chosen by the optimizer.

Fixes: #74069.

Release note: None

Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Neha George <[email protected]>
  • Loading branch information
5 people committed Dec 21, 2021
6 parents 1457048 + d38e4a6 + ebf9977 + 5ea5ef4 + 744c7ed + 452515e commit 299abc8
Show file tree
Hide file tree
Showing 92 changed files with 542 additions and 94,125 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6303,7 +6303,7 @@ func TestProtectedTimestampsDuringBackup(t *testing.T) {
require.NoError(t, err)
lhServer := tc.Server(int(l.Replica.NodeID) - 1)
s, repl := getFirstStoreReplica(t, lhServer, startKey)
trace, _, err := s.ManuallyEnqueue(ctx, "gc", repl, skipShouldQueue)
trace, _, err := s.ManuallyEnqueue(ctx, "mvccGC", repl, skipShouldQueue)
require.NoError(t, err)
fmt.Fprintf(&traceBuf, "%s\n", trace.String())
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/import_into_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func TestProtectedTimestampsDuringImportInto(t *testing.T) {
require.NoError(t, err)
lhServer := tc.Server(int(l.Replica.NodeID) - 1)
s, repl := getFirstStoreReplica(t, lhServer, startKey)
trace, _, err := s.ManuallyEnqueue(ctx, "gc", repl, skipShouldQueue)
trace, _, err := s.ManuallyEnqueue(ctx, "mvccGC", repl, skipShouldQueue)
require.NoError(t, err)
fmt.Fprintf(&traceBuf, "%s\n", trace.String())
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/logictestccl/testdata/logic_test/as_of
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE j = 2
query T
EXPLAIN (OPT, MEMO) SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1ms') WHERE j = 2 AND i = 1
----
memo (optimized, ~5KB, required=[presentation: info:6])
memo (optimized, ~8KB, required=[presentation: info:6])
├── G1: (explain G2 [presentation: i:1,j:2,k:3])
│ └── [presentation: info:6]
│ ├── best: (explain G2="[presentation: i:1,j:2,k:3]" [presentation: i:1,j:2,k:3])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ select
│ ├── [/3/3/1 - /3/3/9]
│ └── [/3/4/1 - ]
└── filters
├── (c > 0) AND (c < 10)
└── (c % 2) = 1
├── (c % 2) = 1
└── (c > 0) AND (c < 10)

# Perhaps an unintuitive example. The partition constraints don't really help
# constrain the index. None of the spans created by the partitioning are constrained
Expand Down
22 changes: 0 additions & 22 deletions pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,6 @@ vectorized: true
└── • scan buffer
label: buffer 1
·
index recommendations: 2
1. type: index creation
SQL command: CREATE INDEX ON t (pk);
2. type: index creation
SQL command: CREATE INDEX ON t (c);

statement ok
INSERT INTO fk_using_implicit_columns_against_t VALUES (1, 1, 4)
Expand Down Expand Up @@ -805,14 +799,6 @@ vectorized: true
└── • scan buffer
label: buffer 1
·
index recommendations: 3
1. type: index creation
SQL command: CREATE INDEX ON t (pk);
2. type: index creation
SQL command: CREATE INDEX ON t (b);
3. type: index creation
SQL command: CREATE INDEX ON t (c, d);

statement ok
INSERT INTO t VALUES (1, 1, 1, 1, 1, 1, 1), (2, 2, 2, 2, 2, 2, 2)
Expand Down Expand Up @@ -898,14 +884,6 @@ vectorized: true
└── • scan buffer
label: buffer 1
·
index recommendations: 3
1. type: index creation
SQL command: CREATE INDEX ON t (pk) STORING (pk2, a, b, c, d);
2. type: index creation
SQL command: CREATE INDEX ON t (b);
3. type: index creation
SQL command: CREATE INDEX ON t (c, d);

# One row already exists, one row is new.
statement ok
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/streamingccl/streamclient/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ go_library(
"//pkg/roachpb:with-mocks",
"//pkg/security",
"//pkg/sql",
"//pkg/sql/catalog",
"//pkg/sql/catalog/catalogkeys",
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/tabledesc",
"//pkg/sql/randgen",
"//pkg/sql/rowenc",
"//pkg/sql/sem/tree",
"//pkg/util/hlc",
Expand Down
13 changes: 11 additions & 2 deletions pkg/ccl/streamingccl/streamclient/random_stream_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import (
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
"github.com/cockroachdb/cockroach/pkg/sql/randgen"
"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
Expand Down Expand Up @@ -419,7 +419,16 @@ func makeRandomKey(
r *rand.Rand, config randomStreamConfig, tableDesc *tabledesc.Mutable,
) roachpb.KeyValue {
// Create a key holding a random integer.
k, err := randgen.TestingMakePrimaryIndexKey(tableDesc, r.Intn(config.valueRange))
keyDatum := tree.NewDInt(tree.DInt(r.Intn(config.valueRange)))

index := tableDesc.GetPrimaryIndex()
// Create the ColumnID to index in datums slice map needed by
// MakeIndexKeyPrefix.
var colIDToRowIndex catalog.TableColMap
colIDToRowIndex.Set(index.GetKeyColumnID(0), 0)

keyPrefix := rowenc.MakeIndexKeyPrefix(keys.SystemSQLCodec, tableDesc, index.GetID())
k, _, err := rowenc.EncodeIndexKey(tableDesc, index, colIDToRowIndex, tree.Datums{keyDatum}, keyPrefix)
if err != nil {
panic(err)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func TestNoLinkForbidden(t *testing.T) {
"testing", // defines flags
"go/build", // probably not something we want in the main binary
"github.com/cockroachdb/cockroach/pkg/security/securitytest", // contains certificates
"github.com/cockroachdb/cockroach/pkg/sql/randgen", // meant for testing code only
},
[]string{
"github.com/cockroachdb/cockroach/pkg/testutils", // meant for testing code only
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/generate-spatial-ref-sys/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ go_library(
deps = [
"//pkg/geo/geoproj",
"//pkg/geo/geoprojbase",
"//pkg/geo/geoprojbase/embeddedproj",
"//pkg/util/ctxgroup",
"//pkg/util/httputil",
"//pkg/util/syncutil",
"//pkg/util/timeutil",
],
)

Expand Down
45 changes: 0 additions & 45 deletions pkg/cmd/generate-spatial-ref-sys/generate.tmpl

This file was deleted.

Loading

0 comments on commit 299abc8

Please sign in to comment.