Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
101931: dev: add flag for extra docker args r=srosenberg,rickystewart a=herkolategan

In order to support git alternates an additional volume must be supplied to the
bazel support docker. The TeamCity script already supplies a utility method
called `run_bazel` which supplies the volume to support git alternates.

This change allows for extra arguments to be passed via a command line argument
to the docker invocation that is utilised for cross builds by the `dev` utility.

Resolves: #101829

102200: sql,clusterversion: add a cluster version for partially visible indexes r=yuzefovich a=rytaft

Fixes #101976

Release note: None

102296: sql: remove redundant column from crdb_internal.*_distsql_flows r=yuzefovich a=yuzefovich

This commit removes now redundant `status` column from `crdb_internal.cluster_distsql_flows` and
`crdb_internal.node_distsql_flows` virtual tables. We no longer have queueing of the remote flows in place, so all remote flows always have 'running' status.

Epic: None

Release note: None

102333: release: increase bincheck timeout r=celiala a=rail

Increase overall timeout for bincheck in order to be less flaky running the tests on GitHub runners.

Epic: none
Release note: None

102354: ui: enable initial data fetch in insecure mode r=zachlite a=zachlite

Previously, `alertDataSync`, the function that populates missing cluster info, did not consider insecure mode when assessing a user's login state.

Because the 23.1 test cluster is running in insecure mode, the UI is noticeably missing it's cluster ID.

Resolves #102350
Epic: none
Release note: None

Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: zachlite <[email protected]>
  • Loading branch information
6 people committed Apr 26, 2023
6 parents a947128 + 8eecff7 + 94b5a80 + dcc4533 + e054ffd + b64c170 commit bc5e6fc
Show file tree
Hide file tree
Showing 31 changed files with 280 additions and 116 deletions.
2 changes: 1 addition & 1 deletion build/release/bincheck/bincheck
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ echo ""
"$cockroach" start-single-node --insecure --listening-url-file="$urlfile" --enterprise-encryption=path=cockroach-data,key=aes-128.key,old-key=plain --pid-file="$pidfile" &

trap "kill -9 $! || true" EXIT
for i in {0..3}
for i in {0..8}
do
[[ -f "$urlfile" ]] && break
backoff=$((2 ** i))
Expand Down
2 changes: 1 addition & 1 deletion dev
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fi
set -euo pipefail

# Bump this counter to force rebuilding `dev` on all machines.
DEV_VERSION=71
DEV_VERSION=72

THIS_DIR=$(cd "$(dirname "$0")" && pwd)
BINARY_DIR=$THIS_DIR/bin/dev-versions
Expand Down
2 changes: 0 additions & 2 deletions docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -2701,7 +2701,6 @@ Info contains an information about a single DistSQL remote flow.
| ----- | ---- | ----- | ----------- | -------------- |
| node_id | [int32](#cockroach.server.serverpb.ListDistSQLFlowsResponse-int32) | | NodeID is the node on which this remote flow is either running or queued. | [reserved](#support-status) |
| timestamp | [google.protobuf.Timestamp](#cockroach.server.serverpb.ListDistSQLFlowsResponse-google.protobuf.Timestamp) | | Timestamp must be in the UTC timezone. | [reserved](#support-status) |
| status | [DistSQLRemoteFlows.Status](#cockroach.server.serverpb.ListDistSQLFlowsResponse-cockroach.server.serverpb.DistSQLRemoteFlows.Status) | | Status is the current status of this remote flow. TODO(yuzefovich): remove this in 23.2. | [reserved](#support-status) |
| stmt | [string](#cockroach.server.serverpb.ListDistSQLFlowsResponse-string) | | Stmt is the SQL statement for which this flow is executing. | [reserved](#support-status) |


Expand Down Expand Up @@ -2795,7 +2794,6 @@ Info contains an information about a single DistSQL remote flow.
| ----- | ---- | ----- | ----------- | -------------- |
| node_id | [int32](#cockroach.server.serverpb.ListDistSQLFlowsResponse-int32) | | NodeID is the node on which this remote flow is either running or queued. | [reserved](#support-status) |
| timestamp | [google.protobuf.Timestamp](#cockroach.server.serverpb.ListDistSQLFlowsResponse-google.protobuf.Timestamp) | | Timestamp must be in the UTC timezone. | [reserved](#support-status) |
| status | [DistSQLRemoteFlows.Status](#cockroach.server.serverpb.ListDistSQLFlowsResponse-cockroach.server.serverpb.DistSQLRemoteFlows.Status) | | Status is the current status of this remote flow. TODO(yuzefovich): remove this in 23.2. | [reserved](#support-status) |
| stmt | [string](#cockroach.server.serverpb.ListDistSQLFlowsResponse-string) | | Stmt is the SQL statement for which this flow is executing. | [reserved](#support-status) |


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 @@ -294,4 +294,4 @@ trace.opentelemetry.collector string address of an OpenTelemetry trace collecto
trace.snapshot.rate duration 0s if non-zero, interval at which background trace snapshots are captured tenant-rw
trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https://<ui>/#/debug/tracez tenant-rw
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. tenant-rw
version version 1000023.1-4 set the active cluster version in the format '<major>.<minor>' tenant-rw
version version 1000023.1-6 set the active cluster version in the format '<major>.<minor>' tenant-rw
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,6 @@
<tr><td><div id="setting-trace-snapshot-rate" class="anchored"><code>trace.snapshot.rate</code></div></td><td>duration</td><td><code>0s</code></td><td>if non-zero, interval at which background trace snapshots are captured</td><td>Serverless/Dedicated/Self-Hosted</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><td>Serverless/Dedicated/Self-Hosted</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><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.1-4</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.1-6</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
</tbody>
</table>
2 changes: 0 additions & 2 deletions pkg/cli/zip_table_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ var zipInternalTablesPerCluster = DebugZipTableRegistry{
"flow_id",
"node_id",
"since",
"status",
"crdb_internal.hide_sql_constants(stmt) as stmt",
},
},
Expand Down Expand Up @@ -624,7 +623,6 @@ var zipInternalTablesPerNode = DebugZipTableRegistry{
"node_id",
"stmt",
"since",
"status",
"crdb_internal.hide_sql_constants(stmt) as stmt",
},
},
Expand Down
24 changes: 16 additions & 8 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,14 +538,18 @@ const (
// the process of upgrading from previous supported releases to 23.2.
V23_2Start

// V23_2TTLAllowDescPK is the version where TTL tables can have descending
// primary keys.
V23_2TTLAllowDescPK

// V23_2_PartiallyVisibleIndexes is the version where partially visible
// indexes are enabled.
V23_2_PartiallyVisibleIndexes

// *************************************************
// Step 1b: Add new version for 23.2 development here.
// Do not add new versions to a patch release.
// *************************************************

// V23_2TTLAllowDescPK is the version where TTL tables can have descending
// primary keys.
V23_2TTLAllowDescPK
)

func (k Key) String() string {
Expand Down Expand Up @@ -940,15 +944,19 @@ var rawVersionsSingleton = keyedVersions{
Key: V23_2Start,
Version: roachpb.Version{Major: 23, Minor: 1, Internal: 2},
},
{
Key: V23_2TTLAllowDescPK,
Version: roachpb.Version{Major: 23, Minor: 1, Internal: 4},
},
{
Key: V23_2_PartiallyVisibleIndexes,
Version: roachpb.Version{Major: 23, Minor: 1, Internal: 6},
},

// *************************************************
// Step 2b: Add new version gates for 23.2 development here.
// Do not add new versions to a patch release.
// *************************************************
{
Key: V23_2TTLAllowDescPK,
Version: roachpb.Version{Major: 23, Minor: 1, Internal: 4},
},
}

// developmentBranch must true on the main development branch but
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/acceptance.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (d *dev) acceptance(cmd *cobra.Command, commandLine []string) error {
return err
}
volume := mustGetFlagString(cmd, volumeFlag)
err = d.crossBuild(ctx, crossArgs, targets, "crosslinux", volume)
err = d.crossBuild(ctx, crossArgs, targets, "crosslinux", volume, nil)
if err != nil {
return err
}
Expand Down
13 changes: 10 additions & 3 deletions pkg/cmd/dev/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func makeBuildCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Com
buildCmd.Flags().String(volumeFlag, "bzlhome", "the Docker volume to use as the container home directory (only used for cross builds)")
buildCmd.Flags().String(crossFlag, "", "cross-compiles using the builder image (options: linux, linuxarm, macos, macosarm, windows)")
buildCmd.Flags().Lookup(crossFlag).NoOptDefVal = "linux"
buildCmd.Flags().StringArray(dockerArgsFlag, []string{}, "additional arguments to pass to Docker (only used for cross builds)")
addCommonBuildFlags(buildCmd)
return buildCmd
}
Expand Down Expand Up @@ -145,6 +146,7 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
targets, additionalBazelArgs := splitArgsAtDash(cmd, commandLine)
ctx := cmd.Context()
cross := mustGetFlagString(cmd, crossFlag)
dockerArgs := mustGetFlagStringArray(cmd, dockerArgsFlag)

// Set up dev cache unless it's disabled via the environment variable or the
// testing knob.
Expand Down Expand Up @@ -185,15 +187,20 @@ func (d *dev) build(cmd *cobra.Command, commandLine []string) error {
}
volume := mustGetFlagString(cmd, volumeFlag)
cross = "cross" + cross
return d.crossBuild(ctx, args, buildTargets, cross, volume)
return d.crossBuild(ctx, args, buildTargets, cross, volume, dockerArgs)
}

func (d *dev) crossBuild(
ctx context.Context, bazelArgs []string, targets []buildTarget, crossConfig string, volume string,
ctx context.Context,
bazelArgs []string,
targets []buildTarget,
crossConfig string,
volume string,
dockerArgs []string,
) error {
bazelArgs = append(bazelArgs, fmt.Sprintf("--config=%s", crossConfig), "--config=ci")
configArgs := getConfigArgs(bazelArgs)
dockerArgs, err := d.getDockerRunArgs(ctx, volume, false)
dockerArgs, err := d.getDockerRunArgs(ctx, volume, false, dockerArgs)
if err != nil {
return err
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/cmd/dev/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
)

const volumeFlag = "volume"
const dockerArgsFlag = "docker-args"

// MakeBuilderCmd constructs the subcommand used to run
func makeBuilderCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Command {
Expand All @@ -44,7 +45,7 @@ func (d *dev) builder(cmd *cobra.Command, extraArgs []string) error {
if len(extraArgs) == 0 {
tty = true
}
args, err := d.getDockerRunArgs(ctx, volume, tty)
args, err := d.getDockerRunArgs(ctx, volume, tty, nil)
args = append(args, extraArgs...)
if err != nil {
return err
Expand All @@ -56,7 +57,7 @@ func (d *dev) builder(cmd *cobra.Command, extraArgs []string) error {
}

func (d *dev) getDockerRunArgs(
ctx context.Context, volume string, tty bool,
ctx context.Context, volume string, tty bool, extraArgs []string,
) (args []string, err error) {
err = d.ensureBinaryInPath("docker")
if err != nil {
Expand Down Expand Up @@ -137,6 +138,7 @@ func (d *dev) getDockerRunArgs(
// is authoritative. This can result in writes to the actual underlying
// filesystem to be lost, but it's a cache so we don't care about that.
args = append(args, "-v", volume+":/home/roach:delegated")
args = append(args, extraArgs...)
args = append(args, "-u", fmt.Sprintf("%s:%s", uid, gid))
args = append(args, bazelImage)
return
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (d *dev) compose(cmd *cobra.Command, _ []string) error {
return err
}
volume := mustGetFlagString(cmd, volumeFlag)
err = d.crossBuild(ctx, crossArgs, targets, "crosslinux", volume)
err = d.crossBuild(ctx, crossArgs, targets, "crosslinux", volume, nil)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/roachprod_stress.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (d *dev) roachprodStress(cmd *cobra.Command, commandLine []string) error {
if race {
crossArgs = append(crossArgs, "--config=race")
}
err = d.crossBuild(ctx, crossArgs, targets, "crosslinux", volume)
err = d.crossBuild(ctx, crossArgs, targets, "crosslinux", volume, nil)
if err != nil {
return err
}
Expand Down
12 changes: 7 additions & 5 deletions pkg/cmd/dev/test_binaries.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func makeTestBinariesCmd(runE func(cmd *cobra.Command, args []string) error) *co
RunE: runE,
}
testBinariesCmd.Flags().String(volumeFlag, "bzlhome", "the Docker volume to use as the container home directory (only used for cross builds)")
testBinariesCmd.Flags().StringArray(dockerArgsFlag, []string{}, "additional arguments to pass to Docker (only used for cross builds)")
testBinariesCmd.Flags().String(outputFlag, "bin/test_binaries.tar.gz", "the file output path of the archived test binaries")
testBinariesCmd.Flags().Bool(raceFlag, false, "produce race builds")
testBinariesCmd.Flags().Int(batchSizeFlag, 128, "the number of packages to build per batch")
Expand All @@ -99,10 +100,11 @@ func makeTestBinariesCmd(runE func(cmd *cobra.Command, args []string) error) *co
func (d *dev) testBinaries(cmd *cobra.Command, commandLine []string) error {
ctx := cmd.Context()
var (
volume = mustGetFlagString(cmd, volumeFlag)
output = mustGetFlagString(cmd, outputFlag)
race = mustGetFlagBool(cmd, raceFlag)
batchSize = mustGetConstrainedFlagInt(cmd, batchSizeFlag, func(v int) error {
extraDockerArgs = mustGetFlagStringArray(cmd, dockerArgsFlag)
volume = mustGetFlagString(cmd, volumeFlag)
output = mustGetFlagString(cmd, outputFlag)
race = mustGetFlagBool(cmd, raceFlag)
batchSize = mustGetConstrainedFlagInt(cmd, batchSizeFlag, func(v int) error {
if v <= 0 {
return fmt.Errorf("%s must be greater than zero", batchSizeFlag)
}
Expand Down Expand Up @@ -155,7 +157,7 @@ func (d *dev) testBinaries(cmd *cobra.Command, commandLine []string) error {
}
binTar := strings.TrimSuffix(output, ".gz")

dockerArgs, err := d.getDockerRunArgs(ctx, volume, false)
dockerArgs, err := d.getDockerRunArgs(ctx, volume, false, extraDockerArgs)
if err != nil {
return err
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/cmd/dev/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ func mustGetFlagString(cmd *cobra.Command, name string) string {
return val
}

func mustGetFlagStringArray(cmd *cobra.Command, name string) []string {
val, err := cmd.Flags().GetStringArray(name)
if err != nil {
log.Fatalf("unexpected error: %v", err)
}
return val
}

func mustGetFlagStringSlice(cmd *cobra.Command, name string) []string {
val, err := cmd.Flags().GetStringSlice(name)
if err != nil {
Expand Down
9 changes: 1 addition & 8 deletions pkg/server/serverpb/status.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1136,11 +1136,6 @@ message ListDistSQLFlowsRequest {}
// same physical plan. The gateway node that initiated the execution of the plan
// isn't included.
message DistSQLRemoteFlows {
enum Status {
RUNNING = 0;
QUEUED = 1;
}

// Info contains an information about a single DistSQL remote flow.
message Info {
// NodeID is the node on which this remote flow is either running or queued.
Expand All @@ -1152,9 +1147,7 @@ message DistSQLRemoteFlows {
(gogoproto.nullable) = false, (gogoproto.stdtime) = true
];

// Status is the current status of this remote flow.
// TODO(yuzefovich): remove this in 23.2.
Status status = 3;
reserved 3;

// Stmt is the SQL statement for which this flow is executing.
string stmt = 4;
Expand Down
1 change: 0 additions & 1 deletion pkg/server/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,6 @@ func (b *baseStatusServer) ListLocalDistSQLFlows(
Infos: []serverpb.DistSQLRemoteFlows_Info{{
NodeID: nodeIDOrZero,
Timestamp: f.Timestamp,
Status: serverpb.DistSQLRemoteFlows_RUNNING,
Stmt: f.StatementSQL,
}},
})
Expand Down
Loading

0 comments on commit bc5e6fc

Please sign in to comment.