Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
78216: ui: decouple html base; allow development with short builds r=sjbarag,matthewtodd a=dhartunian

Previously, a few small issues prevented the UI development process from
making independent choices from CRDB. First, the base HTML template
contained data available in a global variable. Second, the "short" build
would break this template and keep us from developing on the UI when
we're just tweaking server endpoints.

This PR solves both problems. First, the templated variables are removed
and retrieved a startup in `index.tsx`. The React bootstrap waits for
the JSON response which is saved in a global variable by the JS code
instead of the backend. An attempt was made to remove the need for this
global variable but that changes quite a bit of code. In particular, all
of our links to docs pages rely on the global availability of the
cluster version number. Changing this would require that all docs link
generation functions take the version number as input instead of being
statically generated.

Second, the webpack dev server proxy will now attack a header to its
requests allowing us to notice this on the CRDB server and short circuit
the response on short builds to render the base HTML template as if it
was a "full" build. When the webpack dev server is running and serving
dev assets from memory, this lets us interact with DB Console on the
3000 port and develop using the hot reload workflow. This will shorten
dev feedback loops by making builds faster when backend endpoints are
being tweaked.

Release note: None

83021: cmd/reduce: reduce costfuzz logs r=msirek a=michae2

Add a `-costfuzz` mode to `reduce`, with special handling for logs
produced by the costfuzz roachtest. This mode is similar to `-tlp`.

The logs produced by costfuzz consist of various CREATE TABLE, INSERT,
UPDATE, and DELETE statements followed by three special statements:
1. A SELECT statement, called "control".
2. A SET testing_optimizer_random_cost_seed statement.
3. The same SELECT as (1), this time called "perturbed".

When the results of (1) and (3) do not match, we have a costfuzz
failure.

The `-costfuzz` mode removes these three special statements from the end
of the log, reduces the rest of the log, and then appends the statements
to the reduced log to check if the current reduction is interesting. As
long as the results of (1) and (3) continue to not match, the reduction
is interesting.

Also, make `reduce` run `cockroach demo` with an enterprise license. Now
that we use enterprise features in randgen we sometimes need this.

Informs: #81717

Release note: None

83108: ui: add confirmation modal for reset SQL Stats r=maryliag a=maryliag

Previously, there was not confirmation when the user
clicked on `reset SQL Stats`. This commit introduce
a confirmation modal, with a proper warning about
the data about to be deleted.

Fixes #81867

<img width="591" alt="Screen Shot 2022-06-20 at 4 18 51 PM" src="https://user-images.githubusercontent.com/1017486/174672688-4f983d9c-df8e-4ea6-9c59-6161fe761b09.png">

https://www.loom.com/share/9bd3c6af8f574453ac69e201697601b9

Release note (ui change): Add confirmation modal to `reset SQL Stats`

83130: dev: disable remote cache when building bazel-remote r=rail a=rickystewart

Typically you're building this binary because your cache is not up, but
if your cache is not up you can encounter a lot of these
`Connection refused` errors. Disable the remote cache when building in
this case.

Also disable `nogo` when building `dev`, similarly to save time, and
disable remote caching when building from the top-level `dev` script.

Release note: None

Co-authored-by: David Hartunian <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
5 people committed Jun 21, 2022
5 parents 05e0d0d + 320bdfb + a8ca8e5 + 3be1098 + 90f467c commit 75cf2bb
Show file tree
Hide file tree
Showing 14 changed files with 477 additions and 133 deletions.
2 changes: 1 addition & 1 deletion dev
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fi
if [[ ! -f "$BINARY_PATH" ]]; then
echo "$BINARY_PATH not found, building..."
mkdir -p $BINARY_DIR
bazel build //pkg/cmd/dev --//build/toolchains:nogo_disable_flag
bazel build //pkg/cmd/dev --//build/toolchains:nogo_disable_flag --remote_cache=
cp $(bazel info bazel-bin --//build/toolchains:nogo_disable_flag)/pkg/cmd/dev/dev_/dev $BINARY_PATH
# The Bazel-built binary won't have write permissions.
chmod a+w $BINARY_PATH
Expand Down
5 changes: 3 additions & 2 deletions pkg/cmd/dev/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
crossFlag = "cross"
nogoDisableFlag = "--//build/toolchains:nogo_disable_flag"
geosTarget = "//c-deps:libgeos"
devTarget = "//pkg/cmd/dev:dev"
)

type buildTarget struct {
Expand Down Expand Up @@ -79,7 +80,7 @@ var buildTargetMapping = map[string]string{
"cockroach-oss": "//pkg/cmd/cockroach-oss:cockroach-oss",
"cockroach-short": "//pkg/cmd/cockroach-short:cockroach-short",
"crlfmt": "@com_github_cockroachdb_crlfmt//:crlfmt",
"dev": "//pkg/cmd/dev:dev",
"dev": devTarget,
"docgen": "//pkg/cmd/docgen:docgen",
"docs-issue-gen": "//pkg/cmd/docs-issue-generation:docs-issue-generation",
"execgen": "//pkg/sql/colexec/execgen/cmd/execgen:execgen",
Expand Down Expand Up @@ -404,7 +405,7 @@ func (d *dev) getBasicBuildArgs(
} else {
buildTargets = append(buildTargets, buildTarget{fullName: aliased, kind: "go_binary"})
}
if strings.HasPrefix(aliased, "//") {
if strings.HasPrefix(aliased, "//") && aliased != devTarget {
canDisableNogo = false
}
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/cmd/dev/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ import (
)

const (
bazelRemoteTarget = "@com_github_buchgr_bazel_remote//:bazel-remote"
cacheCleanFlag = "clean"
cacheDownFlag = "down"
cacheResetFlag = "reset"
remoteCacheDisableFlag = "--remote_cache="
bazelRemoteTarget = "@com_github_buchgr_bazel_remote//:bazel-remote"
cacheCleanFlag = "clean"
cacheDownFlag = "down"
cacheResetFlag = "reset"

cachePidFilename = ".dev-cache.pid"
configFilename = "config.yml"
Expand Down Expand Up @@ -136,11 +137,11 @@ func (d *dev) setUpCache(ctx context.Context) (string, error) {

log.Printf("Configuring cache...\n")

err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", "build", bazelRemoteTarget, nogoDisableFlag)
err := d.exec.CommandContextInheritingStdStreams(ctx, "bazel", "build", bazelRemoteTarget, nogoDisableFlag, remoteCacheDisableFlag)
if err != nil {
return "", err
}
bazelRemoteLoc, err := d.exec.CommandContextSilent(ctx, "bazel", "run", bazelRemoteTarget, nogoDisableFlag, "--run_under=//build/bazelutil/whereis")
bazelRemoteLoc, err := d.exec.CommandContextSilent(ctx, "bazel", "run", bazelRemoteTarget, nogoDisableFlag, "--run_under=//build/bazelutil/whereis", remoteCacheDisableFlag)
if err != nil {
return "", err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/reduce/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ go_library(
deps = [
"//pkg/cmd/reduce/reduce",
"//pkg/cmd/reduce/reduce/reducesql",
"//pkg/util/envutil",
"@com_github_cockroachdb_errors//:errors",
],
)
Expand Down
168 changes: 124 additions & 44 deletions pkg/cmd/reduce/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package main

import (
"bytes"
"context"
"flag"
"fmt"
Expand All @@ -27,6 +28,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/cmd/reduce/reduce"
"github.com/cockroachdb/cockroach/pkg/cmd/reduce/reduce/reducesql"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -55,21 +57,26 @@ var (
unknown = flags.Bool("unknown", false, "print unknown types during walk")
workers = flags.Int("goroutines", goroutines, "number of worker goroutines (defaults to NumCPU/3")
chunkReductions = flags.Int("chunk", 0, "number of consecutive chunk reduction failures allowed before halting chunk reduction (default 0)")
tlp = flags.Bool("tlp", false, "last two non-empty lines in file are equivalent queries returning different results")
tlp = flags.Bool("tlp", false, "last two statements in file are equivalent queries returning different results")
costfuzz = flags.Bool("costfuzz", false, "last three statements in file are two identical queries separated by a setting change")
)

// TODO(michae2): Add -costfuzz mode which is aware that the last three
// statements are two identical queries separated by a setting change.

const description = `
The reduce utility attempts to simplify SQL that produces an error in
CockroachDB. The problematic SQL, specified via -file flag, is
repeatedly reduced as long as it produces an error in the CockroachDB
demo that matches the provided -contains regex.
The reduce utility attempts to simplify SQL that produces specific
output in CockroachDB. The problematic SQL, specified via -file flag, is
repeatedly reduced as long as it produces results or errors in cockroach
demo that match the provided -contains regex.
An alternative mode of operation is enabled by specifying -tlp option:
in such case the last two queries in the file must be equivalent and
produce different results.
produce different results. (Note that statements in the file must be
separated by blank lines for -tlp to function correctly.)
Another alternative mode of operation is enabled by the -costfuzz
option: in which case the last three statements in the file must be two
identical queries separated by a setting change, which produce different
results. (Note that statements in the file must be separated by blank
lines for -costfuzz to function correctly.)
The following options are available:
Expand All @@ -91,22 +98,47 @@ func main() {
fmt.Printf("%s: -file must be provided\n\n", os.Args[0])
usage()
}
if *contains == "" && !*tlp {
fmt.Printf("%s: either -contains must be provided or -tlp flag specified\n\n", os.Args[0])
var modes int
if *contains != "" {
modes++
}
if *tlp {
modes++
}
if *costfuzz {
modes++
}

if modes != 1 {
fmt.Printf("%s: exactly one of -contains, -tlp, or -costfuzz must be specified\n\n", os.Args[0])
usage()
}
reducesql.LogUnknown = *unknown
out, err := reduceSQL(*binary, *contains, file, *workers, *verbose, *chunkReductions, *tlp)
out, err := reduceSQL(*binary, *contains, file, *workers, *verbose, *chunkReductions, *tlp, *costfuzz)
if err != nil {
log.Fatal(err)
}
fmt.Println(out)
}

func reduceSQL(
binary, contains string, file *string, workers int, verbose bool, chunkReductions int, tlp bool,
binary, contains string,
file *string,
workers int,
verbose bool,
chunkReductions int,
tlp, costfuzz bool,
) (string, error) {
const tlpFailureError = "TLP_FAILURE"
var settings string
if devLicense, ok := envutil.EnvString("COCKROACH_DEV_LICENSE", 0); ok {
settings += "SET CLUSTER SETTING cluster.organization = 'Cockroach Labs - Production Testing';\n"
settings += fmt.Sprintf("SET CLUSTER SETTING enterprise.license = '%s';\n", devLicense)
}

const (
tlpFailureError = "TLP_FAILURE"
costfuzzSep = "COSTFUZZ_SEP"
)
if tlp {
contains = tlpFailureError
}
Expand All @@ -120,7 +152,7 @@ func reduceSQL(
}

inputString := string(input)
var tlpCheck string
var tlpCostfuzzCheck string

// If TLP check is requested, then we remove the last two queries from the
// input (each query is expected to be delimited by empty lines) which we
Expand All @@ -133,31 +165,14 @@ func reduceSQL(
if tlp {
lines := strings.Split(string(input), "\n")
lineIdx := len(lines) - 1
// findPreviousQuery return the query preceding lineIdx without a
// semicolon. Queries are expected to be delimited with empty lines.
findPreviousQuery := func() string {
// Skip empty lines.
for lines[lineIdx] == "" {
lineIdx--
}
lastQueryLineIdx := lineIdx
// Now skip over all lines comprising the query.
for lines[lineIdx] != "" {
lineIdx--
}
// lineIdx right now points at an empty line before the query.
query := strings.Join(lines[lineIdx+1:lastQueryLineIdx+1], " ")
// Remove the semicolon.
return query[:len(query)-1]
}
partitioned := findPreviousQuery()
unpartitioned := findPreviousQuery()
partitioned, lineIdx := findPreviousQuery(lines, lineIdx)
unpartitioned, lineIdx := findPreviousQuery(lines, lineIdx)
inputString = strings.Join(lines[:lineIdx], "\n")
// tlpCheck is a query that will result in an error with tlpFailureError
// error message when unpartitioned and partitioned queries return
// different results (which is the case when there are rows in one
// We make tlpCostfuzzCheck a query that will result in an error with
// tlpFailureError error message when unpartitioned and partitioned queries
// return different results (which is the case when there are rows in one
// result set that are not present in the other).
tlpCheck = fmt.Sprintf(`
tlpCostfuzzCheck = fmt.Sprintf(`
SELECT CASE
WHEN
(SELECT count(*) FROM ((%[1]s) EXCEPT ALL (%[2]s))) != 0
Expand All @@ -168,6 +183,35 @@ SELECT CASE
END;`, unpartitioned, partitioned, tlpFailureError)
}

// If costfuzz mode is requested, then we remove the last three statements
// from the input (statements are expected to be delimited by empty lines)
// which we then save for the costfuzz check.
if costfuzz {
lines := strings.Split(string(input), "\n")
lineIdx := len(lines) - 1
perturb, lineIdx := findPreviousQuery(lines, lineIdx)
setting, lineIdx := findPreviousQuery(lines, lineIdx)
control, lineIdx := findPreviousQuery(lines, lineIdx)
inputString = strings.Join(lines[:lineIdx], "\n")
// We make tlpCostfuzzCheck the original three control / setting / perturbed
// statements, surrounded by sentinel statements.
tlpCostfuzzCheck = fmt.Sprintf(`
SELECT '%[1]s';
%[2]s;
SELECT '%[1]s';
%[3]s;
SELECT '%[1]s';
%[4]s;
SELECT '%[1]s';
`, costfuzzSep, control, setting, perturb)
}

// Pretty print the input so the file size comparison is useful.
inputSQL, err := reducesql.Pretty(inputString)
if err != nil {
Expand All @@ -178,12 +222,12 @@ SELECT CASE
if verbose {
logger = log.New(os.Stderr, "", 0)
logger.Printf("input SQL pretty printed, %d bytes -> %d bytes\n", len(input), len(inputSQL))
if tlp {
prettyTLPCheck, err := reducesql.Pretty(tlpCheck)
if tlp || costfuzz {
prettyTLPCheck, err := reducesql.Pretty(tlpCostfuzzCheck)
if err != nil {
return "", err
}
logger.Printf("\nTLP check query:\n%s\n\n", prettyTLPCheck)
logger.Printf("\nCheck query:\n%s\n\n", prettyTLPCheck)
}
}

Expand All @@ -200,14 +244,15 @@ SELECT CASE
"--empty",
"--disable-demo-license",
"--set=errexit=false",
"--format=tsv",
)
cmd.Env = []string{"COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING", "true"}
sql = settings + sql
if !strings.HasSuffix(sql, ";") {
sql += ";"
}
// If -tlp was not specified, this is a noop, if it was specified, then
// we append the special TLP check query.
sql += tlpCheck
// If neither -tlp nor -costfuzz were specified, this is a noop.
sql += tlpCostfuzzCheck
cmd.Stdin = strings.NewReader(sql)
out, err := cmd.CombinedOutput()
switch {
Expand All @@ -218,6 +263,23 @@ SELECT CASE
case errors.HasType(err, (*os.PathError)(nil)):
log.Fatal(err)
}
if costfuzz {
parts := bytes.Split(out, []byte(costfuzzSep))
if len(parts) != 5 {
if verbose {
logOriginalHint = func() {
logger.Printf("could not divide output into 5 parts")
}
}
return false, logOriginalHint
}
if verbose {
logOriginalHint = func() {
logger.Printf("control and perturbed query results were the same: \n%v\n\n%v\n", string(parts[1]), string(parts[3]))
}
}
return !bytes.Equal(parts[1], parts[3]), logOriginalHint
}
if verbose {
logOriginalHint = func() {
logger.Printf("output did not match regex %s:\n\n%s", contains, string(out))
Expand All @@ -237,3 +299,21 @@ SELECT CASE
)
return out, err
}

// findPreviousQuery return the query preceding lineIdx without a semicolon.
// Queries are expected to be delimited with empty lines.
func findPreviousQuery(lines []string, lineIdx int) (string, int) {
// Skip empty lines.
for lines[lineIdx] == "" {
lineIdx--
}
lastQueryLineIdx := lineIdx
// Now skip over all lines comprising the query.
for lines[lineIdx] != "" {
lineIdx--
}
// lineIdx right now points at an empty line before the query.
query := strings.Join(lines[lineIdx+1:lastQueryLineIdx+1], " ")
// Remove the semicolon.
return query[:len(query)-1], lineIdx
}
Loading

0 comments on commit 75cf2bb

Please sign in to comment.