Skip to content

Commit

Permalink
Merge #95353 #98169
Browse files Browse the repository at this point in the history 
95353: bazel: refactor dev roachprod-bench-wrapper command to dev test-binaries r=srosenberg a=herkolategan

This intends to decouple generating test binaries and running the roachprod microbenchmarks command. Rather than having a strong coupling this command will now just build and archive the portable test binaries that can then be used by any other commands in the future.

The command also no longer builds the libraries or the roachprod microbenchmarks binary (formerly roachprod-bench). After this change `dev test-binaries` will only generate an archive of portable test binaries. This archive can then be used with the roachprod microbenchmarks command, possibly amongst other things, to run microbenchmarks.

See also: #90958

Release note: None

98169: copy: add test with 1PC optimization disabled r=otan a=rafiss

This makes sure everything in the COPY is still committed even if the insert_fast_path is disabled.

Epic: None
Release note: None

Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
@herkolategan @rafiss
3 people committed Mar 7, 2023
3 parents b84f10c + 9d9236c + 725e75e commit 76d6719
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 119 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/dev/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ go_library(
"lint.go",
"main.go",
"merge_test_xmls.go",
"roachprod_bench.go",
"roachprod_stress.go",
"test.go",
"test_binaries.go",
"testlogic.go",
"ui.go",
"util.go",
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/dev.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ Typical usage:
makeTestCmd(ret.test),
makeUICmd(&ret),
makeRoachprodStressCmd(ret.roachprodStress),
makeRoachprodBenchCmd(ret.roachprodBench),
makeTestBinariesCmd(ret.testBinaries),
)

// Add all the shared flags.
Expand Down
153 changes: 36 additions & 117 deletions pkg/cmd/dev/roachprod_bench.go → pkg/cmd/dev/test_binaries.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@ package main

import (
"context"
"crypto/sha256"
"encoding/hex"
"fmt"
"io"
"log"
"os"
"path"
"path/filepath"
"strings"
"text/template"
Expand All @@ -30,10 +27,9 @@ import (
)

const (
benchArgsFlag = "bench-args"
compressFlag = "compress"
buildHashFlag = "build-hash"
outputFlag = "output"
batchSizeFlag = "batch-size"
artifactsDir = "artifacts"
)

type packageTemplateData struct {
Expand All @@ -56,8 +52,8 @@ type runTemplateData struct {

const stageOutputTemplateScript = `
mkdir -p #{.StageDir#}
cp -Lr #{.BazelOutputDir#}/#{.RunfilesDirName#} #{.StageDir#}/#{.RunfilesDirName#}
cp -Lr #{.BazelOutputDir#}/#{.TargetBinName#} #{.StageDir#}/#{.TargetBinName#}
ln -s #{.BazelOutputDir#}/#{.RunfilesDirName#} #{.StageDir#}/#{.RunfilesDirName#}
ln -s #{.BazelOutputDir#}/#{.TargetBinName#} #{.StageDir#}/#{.TargetBinName#}
echo #{shesc .RunScript#} > #{.StageDir#}/run.sh
chmod +x #{.StageDir#}/run.sh
Expand All @@ -69,7 +65,9 @@ const packageTemplateScript = `
echo Packaging staged build output...
cd #{.ArtifactsDir#}
tar -chf bin.tar pkg
rm -rf pkg
bazel clean
`

const runTemplateScript = `
Expand All @@ -81,53 +79,39 @@ ln -f -s ./#{.RunfilesDirName#}/com_github_cockroachdb_cockroach/#{.PackageDir#}
./#{.TargetBinName#} "$@"
`

func makeRoachprodBenchCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Command {
roachprodBenchCmd := &cobra.Command{
Use: "roachprod-bench-wrapper <pkg>",
Short: "invokes pkg/cmd/roachprod-bench to parallelize execution of specified microbenchmarks",
Long: "invokes pkg/cmd/roachprod-bench to parallelize execution of specified microbenchmarks",
Example: `dev roachprod-bench-wrapper ./pkg/sql/importer --cluster my_cluster`,
func makeTestBinariesCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Command {
testBinariesCmd := &cobra.Command{
Use: "test-binaries <pkg>",
Short: "builds portable test binaries supplied with an executable run script for each package",
Long: "builds portable test binaries supplied with an executable run script for each package",
Example: `dev test-binaries ./pkg/sql/... --output=test_binaries.tar.gz`,
Args: cobra.MinimumNArgs(1),
RunE: runE,
}
roachprodBenchCmd.Flags().String(benchArgsFlag, "", "additional arguments to pass to roachbench")
roachprodBenchCmd.Flags().String(volumeFlag, "bzlhome", "the Docker volume to use as the container home directory (only used for cross builds)")
roachprodBenchCmd.Flags().String(clusterFlag, "", "the name of the cluster (must be set)")
roachprodBenchCmd.Flags().String(buildHashFlag, "", "override the build hash for the build output")
roachprodBenchCmd.Flags().Bool(raceFlag, false, "run tests using race builds")
roachprodBenchCmd.Flags().Bool(compressFlag, true, "compress the output of the benchmarks binaries")
roachprodBenchCmd.Flags().Int(batchSizeFlag, 128, "the number of packages to build per batch")
roachprodBenchCmd.Flags().String(crossFlag, "crosslinux", "the cross build target to use")
return roachprodBenchCmd
testBinariesCmd.Flags().String(volumeFlag, "bzlhome", "the Docker volume to use as the container home directory (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")
testBinariesCmd.Flags().String(crossFlag, "crosslinux", "the cross build target to use")
return testBinariesCmd
}

func (d *dev) roachprodBench(cmd *cobra.Command, commandLine []string) error {
func (d *dev) testBinaries(cmd *cobra.Command, commandLine []string) error {
ctx := cmd.Context()
var (
cluster = mustGetFlagString(cmd, clusterFlag)
volume = mustGetFlagString(cmd, volumeFlag)
benchArgs = mustGetFlagString(cmd, benchArgsFlag)
output = mustGetFlagString(cmd, outputFlag)
race = mustGetFlagBool(cmd, raceFlag)
compress = mustGetFlagBool(cmd, compressFlag)
batchSize = mustGetConstrainedFlagInt(cmd, batchSizeFlag, func(v int) error {
if v <= 0 {
return fmt.Errorf("%s must be greater than zero", batchSizeFlag)
}
return nil
})
crossConfig = mustGetFlagString(cmd, crossFlag)
buildHash = mustGetFlagString(cmd, buildHashFlag)
)

workspace, err := d.getWorkspace(ctx)
if err != nil {
return err
}

if cluster == "" {
return fmt.Errorf("must provide --cluster (e.g., `roachprod create $USER-bench -n 8 --gce-machine-type=n2d-standard-8 --local-ssd=true`)")
}
packages, testArgs := splitArgsAtDash(cmd, commandLine)
packages, _ := splitArgsAtDash(cmd, commandLine)
if len(packages) != 1 {
return fmt.Errorf("expected a single benchmark target specification; e.g., ./pkg/util/... or ./pkg/cmd/dev")
}
Expand Down Expand Up @@ -161,34 +145,16 @@ func (d *dev) roachprodBench(cmd *cobra.Command, commandLine []string) error {
return fmt.Errorf("no test targets found for package %s", pkg)
}

// Generate a unique build hash for the given targets and git revision.
if buildHash == "" {
stdout, cmdErr := d.exec.CommandContextSilent(ctx, "git", "rev-parse", "HEAD")
if cmdErr != nil {
return cmdErr
}
gitRevision := strings.TrimSpace(string(stdout))
buildHashSHA := sha256.New()
buildHashSHA.Write([]byte(gitRevision))
for _, target := range testTargets {
buildHashSHA.Write([]byte(target))
}
buildHash = hex.EncodeToString(buildHashSHA.Sum(nil)[:])[:8]
if output == "" {
return fmt.Errorf("must provide --output")
}

// Assume that we need LibGEOS.
if err = d.buildGeos(ctx, volume); err != nil {
return err
compress := strings.HasSuffix(output, ".tar.gz")
if !compress && !strings.HasSuffix(output, ".tar") {
return fmt.Errorf("output must have the extension .tar or .tar.gz")
}
binTar := strings.TrimSuffix(output, ".gz")

// Clear any old artifacts.
outputPrefix := fmt.Sprintf("roachbench/%s", buildHash)
binTar := fmt.Sprintf("artifacts/%s/bin.tar", outputPrefix)
_, _ = d.os.Remove(binTar), d.os.Remove(binTar+".gz")

// Build the test targets.
var manifest strings.Builder
artifactsDir := fmt.Sprintf("/artifacts/%s", outputPrefix)
dockerArgs, err := d.getDockerRunArgs(ctx, volume, false)
if err != nil {
return err
Expand All @@ -210,15 +176,14 @@ func (d *dev) roachprodBench(cmd *cobra.Command, commandLine []string) error {
if err != nil {
return err
}
}

// Add package name to the manifest file.
for _, target := range targetBatch {
targetDir := targetToDir(target)
manifest.WriteString(fmt.Sprintf("%s\n", targetDir))
}
err = d.packageTestBinaries(ctx, dockerArgs, artifactsDir)
if err != nil {
return err
}

err = d.packageTestArtifacts(ctx, dockerArgs, artifactsDir)
err = os.Rename(filepath.Join(artifactsDir, "bin.tar"), binTar)
if err != nil {
return err
}
Expand All @@ -232,44 +197,7 @@ func (d *dev) roachprodBench(cmd *cobra.Command, commandLine []string) error {
}
}

err = os.WriteFile(fmt.Sprintf("artifacts/%s/roachbench.manifest", outputPrefix), []byte(manifest.String()), 0644)
if err != nil {
return err
}

// Build roachprod-bench.
args, roachprodBenchTarget, err := d.getBasicBuildArgs(ctx, []string{"//pkg/cmd/roachprod-bench"})
if err != nil {
return err
}
if _, err = d.exec.CommandContextSilent(ctx, "bazel", args...); err != nil {
return err
}
if err = d.stageArtifacts(ctx, roachprodBenchTarget); err != nil {
return err
}

libdir := path.Join(workspace, "artifacts", "libgeos", "lib")
if err = d.os.MkdirAll(libdir); err != nil {
return err
}
for _, libWithExt := range []string{"libgeos.so", "libgeos_c.so"} {
src := filepath.Join(workspace, "artifacts", libWithExt)
dst := filepath.Join(libdir, libWithExt)
if err = d.os.CopyFile(src, dst); err != nil {
return err
}
if err = d.os.Remove(src); err != nil {
return err
}
}

roachprodBenchArgs := []string{fmt.Sprintf("./artifacts/%s", outputPrefix), "-cluster", cluster, "-libdir", libdir}
roachprodBenchArgs = append(roachprodBenchArgs, strings.Fields(benchArgs)...)
roachprodBenchArgs = append(roachprodBenchArgs, "--")
roachprodBenchArgs = append(roachprodBenchArgs, testArgs...)
log.Printf("Running roachprod-bench with args: %s", strings.Join(roachprodBenchArgs, " "))
return d.exec.CommandContextInheritingStdStreams(ctx, filepath.Join(workspace, "bin", "roachprod-bench"), roachprodBenchArgs...)
return nil
}

func (d *dev) crossBuildTests(
Expand All @@ -280,7 +208,7 @@ func (d *dev) crossBuildTests(
crossConfig string,
artifactsDir string,
) error {
bazelArgs = append(bazelArgs, fmt.Sprintf("--config=%s", crossConfig), "--config=ci")
bazelArgs = append(bazelArgs, fmt.Sprintf("--config=%s", crossConfig), "--crdb_test_off")
configArgs := getConfigArgs(bazelArgs)
stageOutputTemplate, err := createTemplate("stage", stageOutputTemplateScript)
if err != nil {
Expand All @@ -291,8 +219,7 @@ func (d *dev) crossBuildTests(
return err
}

// Construct a script that builds the binaries and copies them
// to the appropriate location in /artifacts.
// Construct a script that builds the binaries in a temporary directory.
var script strings.Builder
var bazelBinSet bool
script.WriteString("set -euxo pipefail\n")
Expand Down Expand Up @@ -336,7 +263,7 @@ func (d *dev) crossBuildTests(
return err
}

func (d *dev) packageTestArtifacts(
func (d *dev) packageTestBinaries(
ctx context.Context, dockerArgs []string, artifactsDir string,
) error {
var packageBuf strings.Builder
Expand All @@ -353,14 +280,6 @@ func (d *dev) packageTestArtifacts(
return err
}

func (d *dev) buildGeos(ctx context.Context, volume string) error {
crossArgs, targets, err := d.getBasicBuildArgs(ctx, []string{"//c-deps:libgeos"})
if err != nil {
return err
}
return d.crossBuild(ctx, crossArgs, targets, "crosslinux", volume)
}

func createTemplate(name string, script string) (*template.Template, error) {
return template.New(name).
Funcs(template.FuncMap{"shesc": func(i interface{}) string {
Expand Down
15 changes: 15 additions & 0 deletions pkg/sql/copy/copy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,21 @@ func TestCopyFromTransaction(t *testing.T) {
},
func(f1, f2 driver.Value) bool { return !decEq(f1, f2) },
},
{
"implicit_non_atomic_no_1pc",
"COPY lineitem FROM STDIN WITH CSV DELIMITER '|';",
[]string{fmt.Sprintf(csvData, 1), fmt.Sprintf(csvData, 2)},
func(tconn clisqlclient.Conn, f func(tconn clisqlclient.Conn)) {
err := tconn.Exec(ctx, "SET enable_insert_fast_path = false")
require.NoError(t, err)
err = tconn.Exec(ctx, "SET copy_from_atomic_enabled = false")
require.NoError(t, err)
orig := sql.SetCopyFromBatchSize(1)
defer sql.SetCopyFromBatchSize(orig)
f(tconn)
},
func(f1, f2 driver.Value) bool { return !decEq(f1, f2) },
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 76d6719

Please sign in to comment.