Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
…db#97443 cockroachdb#97473 cockroachdb#97484

96235: sem/tree: add support for producing vectorized data from strings r=cucaroach a=cucaroach

tree.ValueHandler exposes raw machine type hooks that are used by
vec_handler to build coldata.Vec's.

Epic: CRDB-18892
Informs: cockroachdb#91831
Release note: None


96460: upgrades,systemschema: add covering index to system.privileges on `path` r=andyyang890 a=e-mbrown

Resolves cockroachdb#93525

Queries to the `system.privileges` table are usually filtered by path. With a primary key using `(user,path)` all those queries were full scans.  This commit changes the primary key to be `(path,user)`.

Release note: None

97272: *: update to use full package name for `option go_package` in protos r=rail,stevendanna,RaduBerinde a=rickystewart

Everywhere in the entire tree we use `option go_package` statements like
the following:

```
option go_package = "build";
```

We do this *instead of* using the entire fully-qualified package name,
like:

```
option go_package = "github.com/cockroachdb/cockroach/pkg/build";
```

This is apparently just an error. All the documentation I have seen
(https://github.com/cockroachdb/gogoproto/blob/master/README) suggests
that the fully-qualified package name should be used here. Further, this
caused the `make` build to break after a refactor as the code generator
doesn't know how to import a package like `"roachpb"` (since it should
instead be importing `"github.com/cockroachdb/cockroach/pkg/roachpb"`).

Correct this problem *everywhere* and update the `Makefile` to set
`paths=source_relative` to tell `protoc` that output files should be
placed next to input files.

Epic: none
Release note: None

97443: kv: configure transaction uncertainty interval for reverse scan r=arulajmani a=nvanbenschoten

This commit configures the transaction uncertainty intervals for reverse scans. This was initially missed in 261fc35, which broke the use of observed timestamps for reverse scans. In 0ee3bbd, we started using this new plumbing path for the entire uncertainty interval, so the pessimization became a correctness bug that could lead to a loss of real-time ordering between a write and a reverse scan in cases with moderate but bounded clock skew between nodes.

The commit adds testing for this case and the ScanRequest case. I should have exercised these cases back in 8445150 when I noticed there was a complete absence of end-to-end testing for uncertainty interval errors. Unfortunately, that commit only added testing for the GetRequest case, allowing the bug to slip in for ReverseScanRequest.

Release note (bug fix): Transaction uncertainty intervals are correctly configured for reverse scans again, ensuring that reverse scans cannot serve stale reads when clocks in a cluster are skewed.

Epic: None

97473: ccl/multiregionccl: Skip stress testing flaky secondary region test r=rafiss a=e-mbrown

Informs: cockroachdb#92235
Release note: None

97484: release: fetch all tags for pick sha r=celiala a=rail

Previously, the pick sha step relies on querying existing tags, but when a release is created on cherry-pick branch (staging-*), we don't have those tags in the local git checkout.

This PR explicitly fetches all tags to have the cherry-pick release tags available locally.

Fixes: RE-386
Epic: none
Release note: None

Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: e-mbrown <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
  • Loading branch information
6 people committed Feb 22, 2023
7 parents 7d45878 + e475009 + ba38bb9 + e890c33 + 629834f + 4ea7bc8 + 9d9b69b commit 3a532ac
Show file tree
Hide file tree
Showing 172 changed files with 1,090 additions and 234 deletions.
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1359,15 +1359,15 @@ $(ERRORS_PROTO): bin/.submodules-initialized
bin/.go_protobuf_sources: $(GO_PROTOS) $(GOGOPROTO_PROTO) $(ERRORS_PROTO) bin/.bootstrap bin/protoc-gen-gogoroach c-deps/proto-rebuild vendor/modules.txt
$(FIND_RELEVANT) -type f -name '*.pb.go' -exec rm {} +
set -e; for dir in $(sort $(dir $(GO_PROTOS))); do \
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) -I$(ERRORS_PATH) --gogoroach_out=$(PROTO_MAPPINGS)plugins=grpc,import_prefix=github.com/cockroachdb/cockroach/pkg/:./pkg $$dir/*.proto; \
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) -I$(ERRORS_PATH) --gogoroach_out=$(PROTO_MAPPINGS)plugins=grpc,import_prefix=github.com/cockroachdb/cockroach/pkg/,paths=source_relative:./pkg $$dir/*.proto; \
done
gofmt -s -w $(GO_SOURCES)
touch $@

bin/.gw_protobuf_sources: $(GW_SERVER_PROTOS) $(GW_TS_PROTOS) $(GO_PROTOS) $(GOGOPROTO_PROTO) $(ERRORS_PROTO) bin/.bootstrap c-deps/proto-rebuild vendor/modules.txt
$(FIND_RELEVANT) -type f -name '*.pb.gw.go' -exec rm {} +
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(ERRORS_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) --grpc-gateway_out=logtostderr=true,request_context=true:./pkg $(GW_SERVER_PROTOS)
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(ERRORS_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) --grpc-gateway_out=logtostderr=true,request_context=true:./pkg $(GW_TS_PROTOS)
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(ERRORS_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) --grpc-gateway_out=logtostderr=true,request_context=true,paths=source_relative:./pkg $(GW_SERVER_PROTOS)
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(ERRORS_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) --grpc-gateway_out=logtostderr=true,request_context=true,paths=source_relative:./pkg $(GW_TS_PROTOS)
gofmt -s -w $(GW_SOURCES)
@# TODO(jordan,benesch) This can be removed along with the above TODO.
goimports -w $(GW_SOURCES)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ if [[ -z "${DRY_RUN}" ]] ; then
fi

# run git fetch in order to get all remote branches
git fetch -q origin
git fetch --tags -q origin
bazel build --config=crosslinux //pkg/cmd/release

$(bazel info --config=crosslinux bazel-bin)/pkg/cmd/release/release_/release \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ log_into_gcloud
export GOOGLE_APPLICATION_CREDENTIALS="$PWD/.google-credentials.json"

# run git fetch in order to get all remote branches
git fetch -q origin
git fetch --tags -q origin
bazel build --config=crosslinux //pkg/cmd/release

$(bazel info --config=crosslinux bazel-bin)/pkg/cmd/release/release_/release \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ if [[ -z "${DRY_RUN}" ]] ; then
fi

# run git fetch in order to get all remote branches
git fetch -q origin
git fetch --tags -q origin
bazel build --config=crosslinux //pkg/cmd/release

$(bazel info --config=crosslinux bazel-bin)/pkg/cmd/release/release_/release \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ if [[ -z "${DRY_RUN}" ]] ; then
fi

# run git fetch in order to get all remote branches
git fetch -q origin
git fetch --tags -q origin

# install gh
wget -O /tmp/gh.tar.gz https://github.com/cli/cli/releases/download/v2.13.0/gh_2.13.0_linux_amd64.tar.gz
Expand Down
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,4 @@ trace.jaeger.agent string the address of a Jaeger agent to receive traces using
trace.opentelemetry.collector string address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as <host>:<port>. If no port is specified, 4317 will be used.
trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
trace.zipkin.collector string the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
version version 1000022.2-50 set the active cluster version in the format '<major>.<minor>'
version version 1000022.2-52 set the active cluster version in the format '<major>.<minor>'
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,6 @@
<tr><td><div id="setting-trace-opentelemetry-collector" class="anchored"><code>trace.opentelemetry.collector</code></div></td><td>string</td><td><code></code></td><td>address of an OpenTelemetry trace collector to receive traces using the otel gRPC protocol, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 4317 will be used.</td></tr>
<tr><td><div id="setting-trace-span-registry-enabled" class="anchored"><code>trace.span_registry.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://&lt;ui&gt;/#/debug/tracez</td></tr>
<tr><td><div id="setting-trace-zipkin-collector" class="anchored"><code>trace.zipkin.collector</code></div></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 9411 will be used.</td></tr>
<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000022.2-50</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td></tr>
<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000022.2-52</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td></tr>
</tbody>
</table>
2 changes: 1 addition & 1 deletion pkg/acceptance/cluster/testconfig.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

syntax = "proto2";
package cockroach.acceptance.cluster;
option go_package = "cluster";
option go_package = "github.com/cockroachdb/cockroach/pkg/acceptance/cluster";

import "gogoproto/gogo.proto";

Expand Down
2 changes: 1 addition & 1 deletion pkg/blobs/blobspb/blobs.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

syntax = "proto3";
package cockroach.blobs;
option go_package = "blobspb";
option go_package = "github.com/cockroachdb/cockroach/pkg/blobs/blobspb";

// GetRequest is used to read a file from a remote node.
// It's path is specified by `filename`, which can either
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/info.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

syntax = "proto2";
package cockroach.build;
option go_package = "build";
option go_package = "github.com/cockroachdb/cockroach/pkg/build";

import "gogoproto/gogo.proto";

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/backuppb/backup.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

syntax = "proto3";
package cockroach.ccl.backupccl;
option go_package = "backuppb";
option go_package = "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuppb";

import "build/info.proto";
import "cloud/cloudpb/external_storage.proto";
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/baseccl/encryption_options.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

syntax = "proto3";
package cockroach.ccl.baseccl;
option go_package = "baseccl";
option go_package = "github.com/cockroachdb/cockroach/pkg/ccl/baseccl";

enum EncryptionKeySource {
// Plain key files.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

syntax = "proto3";
package cockroach.ccl.changefeedccl;
option go_package = "changefeedpb";
option go_package = "github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/changefeedpb";

// ScheduledExportExecutionArgs is the arguments to the scheduled backup executor.
message ScheduledChangefeedExecutionArgs {
Expand Down
6 changes: 5 additions & 1 deletion pkg/ccl/multiregionccl/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,13 @@ func TestMultiRegionDataDriven(t *testing.T) {
defer log.Scope(t).Close(t)

skip.UnderRace(t, "flaky test")

ctx := context.Background()
datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) {

if strings.Contains(path, "secondary_region") {
skip.UnderStressWithIssue(t, 92235, "flaky test")
}

ds := datadrivenTestState{}
defer ds.cleanup(ctx)
var mu syncutil.Mutex
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

syntax = "proto3";
package cockroach.ccl.storageccl.engineccl.enginepbccl;
option go_package = "enginepbccl";
option go_package = "github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl/enginepbccl";

enum EncryptionType {
// No encryption.
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/storageccl/engineccl/enginepbccl/stats.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

syntax = "proto3";
package cockroach.ccl.storageccl.engineccl.enginepbccl;
option go_package = "enginepbccl";
option go_package = "github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl/enginepbccl";

import "ccl/storageccl/engineccl/enginepbccl/key_registry.proto";

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/utilccl/licenseccl/license.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

syntax = "proto3";
package cockroach.ccl.utilccl.licenseccl;
option go_package = "licenseccl";
option go_package = "github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl";

import "gogoproto/gogo.proto";

Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/cloudpb/external_storage.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

syntax = "proto3";
package cockroach.cloud.cloudpb;
option go_package = "cloudpb";
option go_package = "github.com/cockroachdb/cockroach/pkg/cloud/cloudpb";

import "gogoproto/gogo.proto";

Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/externalconn/connectionpb/connection.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

syntax = "proto3";
package cockroach.cloud.externalconn.connectionpb;
option go_package = "connectionpb";
option go_package = "github.com/cockroachdb/cockroach/pkg/cloud/externalconn/connectionpb";

import "gogoproto/gogo.proto";

Expand Down
2 changes: 1 addition & 1 deletion pkg/clusterversion/cluster_version.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

syntax = "proto3";
package cockroach.clusterversion;
option go_package = "clusterversion";
option go_package = "github.com/cockroachdb/cockroach/pkg/clusterversion";

import "roachpb/metadata.proto";
import "gogoproto/gogo.proto";
Expand Down
8 changes: 8 additions & 0 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,10 @@ const (
// elements.
V23_1_SchemaChangerDeprecatedIndexPredicates

// V23_1AlterSystemPrivilegesAddIndexOnPathAndUsername adds a covering secondary index to
// system.privileges, on the path and username columns.
V23_1AlterSystemPrivilegesAddIndexOnPathAndUsername

// *************************************************
// Step (1): Add new versions here.
// Do not add new versions to a patch release.
Expand Down Expand Up @@ -744,6 +748,10 @@ var rawVersionsSingleton = keyedVersions{
Key: V23_1_SchemaChangerDeprecatedIndexPredicates,
Version: roachpb.Version{Major: 22, Minor: 2, Internal: 50},
},
{
Key: V23_1AlterSystemPrivilegesAddIndexOnPathAndUsername,
Version: roachpb.Version{Major: 22, Minor: 2, Internal: 52},
},

// *************************************************
// Step (2): Add new versions here.
Expand Down
6 changes: 6 additions & 0 deletions pkg/col/coldataext/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ go_library(
srcs = [
"datum_vec.go",
"extended_column_factory.go",
"vec_handler.go",
],
importpath = "github.com/cockroachdb/cockroach/pkg/col/coldataext",
visibility = ["//visibility:public"],
Expand All @@ -20,6 +21,11 @@ go_library(
"//pkg/sql/sem/tree",
"//pkg/sql/types",
"//pkg/util/buildutil",
"//pkg/util/duration",
"//pkg/util/encoding",
"//pkg/util/json",
"//pkg/util/timeutil/pgdate",
"@com_github_cockroachdb_apd_v3//:apd",
"@com_github_cockroachdb_errors//:errors",
],
)
Expand Down
159 changes: 159 additions & 0 deletions pkg/col/coldataext/vec_handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package coldataext

import (
"time"

"github.com/cockroachdb/apd/v3"
"github.com/cockroachdb/cockroach/pkg/col/coldata"
"github.com/cockroachdb/cockroach/pkg/col/typeconv"
"github.com/cockroachdb/cockroach/pkg/sql/colexecerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/encoding"
"github.com/cockroachdb/cockroach/pkg/util/json"
"github.com/cockroachdb/cockroach/pkg/util/timeutil/pgdate"
"github.com/cockroachdb/errors"
)

// MakeVecHandler makes a tree.ValueHandler that stores values to a coldata.Vec.
func MakeVecHandler(vec coldata.Vec) tree.ValueHandler {
v := vecHandler{nulls: vec.Nulls()}
switch vec.CanonicalTypeFamily() {
case types.BoolFamily:
v.bools = vec.Bool()
case types.BytesFamily:
v.bytes = vec.Bytes()
case types.DecimalFamily:
v.decimals = vec.Decimal()
case types.IntFamily:
v.ints = vec.Int64()
case types.FloatFamily:
v.floats = vec.Float64()
case types.TimestampTZFamily:
v.timestamps = vec.Timestamp()
case types.IntervalFamily:
v.intervals = vec.Interval()
case types.JsonFamily:
v.jsons = vec.JSON()
case typeconv.DatumVecCanonicalTypeFamily:
v.datums = vec.Datum()
default:
colexecerror.InternalError(errors.AssertionFailedf("unhandled type %s", vec.Type()))
}
return &v
}

type vecHandler struct {
nulls *coldata.Nulls
bools coldata.Bools
bytes *coldata.Bytes
decimals coldata.Decimals
// TODO(cucaroach): implement small int types
//int16s coldata.Int16s
//int32s coldata.Int32s
ints coldata.Int64s
floats coldata.Float64s
timestamps coldata.Times
intervals coldata.Durations
jsons *coldata.JSONs
datums coldata.DatumVec
row int
}

var _ tree.ValueHandler = (*vecHandler)(nil)

// Reset is used to re-use a batch handler across batches.
func (v *vecHandler) Reset() {
v.row = 0
}

// Len returns the current length of the vector.
func (v *vecHandler) Len() int {
return v.row
}

// Decimal implements tree.ValueHandler interface. It returns a pointer into the
// vec to allow the decimal to be constructed in place which avoids expensive
// copying and temporary allocations.
func (v *vecHandler) Decimal() *apd.Decimal {
d := &v.decimals[v.row]
v.row++
return d
}

// Null implements tree.ValueHandler interface.
func (v *vecHandler) Null() {
v.nulls.SetNull(v.row)
v.row++
}

// String is part of the tree.ValueHandler interface.
func (v *vecHandler) String(s string) {
v.bytes.Set(v.row, encoding.UnsafeConvertStringToBytes(s))
v.row++
}

// Date is part of the tree.ValueHandler interface.
func (v *vecHandler) Date(d pgdate.Date) {
v.ints[v.row] = d.UnixEpochDaysWithOrig()
v.row++
}

// Datum is part of the tree.ValueHandler interface.
func (v *vecHandler) Datum(d tree.Datum) {
v.datums.Set(v.row, d)
v.row++
}

// Bool is part of the tree.ValueHandler interface.
func (v *vecHandler) Bool(b bool) {
v.bools[v.row] = b
v.row++
}

// Bytes is part of the tree.ValueHandler interface.
func (v *vecHandler) Bytes(b []byte) {
v.bytes.Set(v.row, b)
v.row++
}

// Float is part of the tree.ValueHandler interface.
func (v *vecHandler) Float(f float64) {
v.floats[v.row] = f
v.row++
}

// Int is part of the tree.ValueHandler interface.
func (v *vecHandler) Int(i int64) {
v.ints[v.row] = i
v.row++
}

// Duration is part of the tree.ValueHandler interface.
func (v *vecHandler) Duration(d duration.Duration) {
v.intervals[v.row] = d
v.row++
}

// JSON is part of the tree.ValueHandler interface.
func (v *vecHandler) JSON(j json.JSON) {
v.jsons.Set(v.row, j)
v.row++
}

// TimestampTZ is part of the tree.ValueHandler interface.
func (v *vecHandler) TimestampTZ(t time.Time) {
v.timestamps[v.row] = t
v.row++
}
Loading

0 comments on commit 3a532ac

Please sign in to comment.