From 17e45670506bc7ecbc8a2bd98b253df0a2f49a8a Mon Sep 17 00:00:00 2001 From: sumeerbhola Date: Mon, 17 Jul 2023 11:45:48 -0400 Subject: [PATCH 1/2] roachtest: delete synctest This test has been skipped since May 2020, because Pebble code is written to crash on an error when writing to the MANIFEST and the WAL, since it is not viable to continue. Additionally, there were problems with hangs due to charybdefs returning EAGAIN (which have since been fixed). We are deleting this now since: - The purpose of this test is to test that Pebble does not lose data due to filesystem errors. It does so in a cruder manner than Pebble's strict MemFS based testing (which came later). Given the existence of the latter, there is no good reason to retain this test. - Working around charybdefs flakiness (see #102492), and Pebble's failstop-on-error behavior is not worth the initial and ongoing maintenance effort. Epic: none Fixes: #48603 Release note: None --- pkg/cli/debug_synctest.go | 5 ++ pkg/cmd/roachtest/tests/BUILD.bazel | 1 - pkg/cmd/roachtest/tests/registry.go | 1 - pkg/cmd/roachtest/tests/synctest.go | 72 ----------------------------- 4 files changed, 5 insertions(+), 74 deletions(-) delete mode 100644 pkg/cmd/roachtest/tests/synctest.go diff --git a/pkg/cli/debug_synctest.go b/pkg/cli/debug_synctest.go index 4ca47b1ff35c..be359c2ee0b0 100644 --- a/pkg/cli/debug_synctest.go +++ b/pkg/cli/debug_synctest.go @@ -30,6 +30,11 @@ import ( "github.com/spf13/cobra" ) +// TODO(sumeer): consider deleting this command, now that it is no longer +// exercised by the synctest roachtest (which has been deleted). We will need +// to confirm that users of CockroachDB are not running it as part of their +// testing. + var debugSyncTestCmd = &cobra.Command{ Use: "synctest ", Short: "Run a log-like workload that can help expose filesystem anomalies", diff --git a/pkg/cmd/roachtest/tests/BUILD.bazel b/pkg/cmd/roachtest/tests/BUILD.bazel index aeccbb0757f0..22f2563bc5e9 100644 --- a/pkg/cmd/roachtest/tests/BUILD.bazel +++ b/pkg/cmd/roachtest/tests/BUILD.bazel @@ -152,7 +152,6 @@ go_library( "sqlsmith.go", "sstable_corruption.go", "status_server.go", - "synctest.go", "sysbench.go", "tlp.go", "tombstones.go", diff --git a/pkg/cmd/roachtest/tests/registry.go b/pkg/cmd/roachtest/tests/registry.go index 9d8af5e64ab8..192301529011 100644 --- a/pkg/cmd/roachtest/tests/registry.go +++ b/pkg/cmd/roachtest/tests/registry.go @@ -133,7 +133,6 @@ func RegisterTests(r registry.Registry) { registerSecure(r) registerSequelize(r) registerSlowDrain(r) - registerSyncTest(r) registerSysbench(r) registerTLP(r) registerTPCC(r) diff --git a/pkg/cmd/roachtest/tests/synctest.go b/pkg/cmd/roachtest/tests/synctest.go deleted file mode 100644 index 8f79ef2f3275..000000000000 --- a/pkg/cmd/roachtest/tests/synctest.go +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright 2018 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 tests - -import ( - "context" - "os" - "path/filepath" - - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec" - "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" -) - -func registerSyncTest(r registry.Registry) { - const nemesisScript = `#!/usr/bin/env bash - -if [[ $1 == "on" ]]; then - charybdefs-nemesis --probability -else - charybdefs-nemesis --clear -fi -` - - r.Add(registry.TestSpec{ - Skip: "#48603: broken on Pebble", - Name: "synctest", - Owner: registry.OwnerStorage, - // This test sets up a custom file system; we don't want the cluster reused. - Cluster: r.MakeClusterSpec(1, spec.ReuseNone()), - Leases: registry.MetamorphicLeases, - Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { - n := c.Node(1) - tmpDir, err := os.MkdirTemp("", "synctest") - if err != nil { - t.Fatal(err) - } - defer func() { - _ = os.RemoveAll(tmpDir) - }() - nemesis := filepath.Join(tmpDir, "nemesis") - - if err := os.WriteFile(nemesis, []byte(nemesisScript), 0755); err != nil { - t.Fatal(err) - } - - c.Put(ctx, t.Cockroach(), "./cockroach") - c.Put(ctx, nemesis, "./nemesis") - c.Run(ctx, n, "chmod +x nemesis") - c.Run(ctx, n, "sudo umount {store-dir}/faulty || true") - c.Run(ctx, n, "mkdir -p {store-dir}/{real,faulty} || true") - t.Status("setting up charybdefs") - - if err := c.Install(ctx, t.L(), n, "charybdefs"); err != nil { - t.Fatal(err) - } - c.Run(ctx, n, "sudo charybdefs {store-dir}/faulty -oallow_other,modules=subdir,subdir={store-dir}/real && chmod 777 {store-dir}/{real,faulty}") - - t.Status("running synctest") - c.Run(ctx, n, "./cockroach debug synctest {store-dir}/faulty ./nemesis") - }, - }) -} From c73932c76c83a73c2ceeb132df648b2e5609d30c Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Mon, 17 Jul 2023 13:05:05 -0700 Subject: [PATCH 2/2] build: use pnpm for cluster-ui auto-publishing The pkg/ui/ tree recently switched to pnpm (from yarn) for package management [1], but the GitHub workflow to automatically publish new versions of pkg/ui/workspaces/cluster-ui wasn't updated to align with that. Use pnpm for dependency management when auto-publishing canary and full-release versions of cluster-ui. Release note: None Epic: None --- .github/workflows/cluster-ui-release-next.yml | 12 ++++++++---- .github/workflows/cluster-ui-release.yml | 13 ++++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/.github/workflows/cluster-ui-release-next.yml b/.github/workflows/cluster-ui-release-next.yml index f81cd0be2a42..83fecdbde526 100644 --- a/.github/workflows/cluster-ui-release-next.yml +++ b/.github/workflows/cluster-ui-release-next.yml @@ -28,14 +28,18 @@ jobs: path: ~/.cache/bazel key: ${{ runner.os }}-bazel-cache + - uses: pnpm/action-setup@v2 + with: + version: 8 + - name: Setup NodeJS uses: actions/setup-node@v3 with: node-version: 16 registry-url: 'https://registry.npmjs.org' always-auth: true - cache: 'yarn' - cache-dependency-path: pkg/ui/workspaces/cluster-ui/yarn.lock + cache: 'pnpm' + cache-dependency-path: 'pkg/ui/pnpm-lock.yaml' env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} @@ -59,10 +63,10 @@ jobs: - name: Build Cluster UI if: steps.version-check.outputs.published == 'no' run: | - yarn install --frozen-lockfile + pnpm install --frozen-lockfile bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client cp ../../../../_bazel/bin/pkg/ui/workspaces/db-console/src/js/protos.* ../db-console/src/js/ - yarn build + pnpm build - name: Create version tag and push if: steps.version-check.outputs.published == 'no' diff --git a/.github/workflows/cluster-ui-release.yml b/.github/workflows/cluster-ui-release.yml index 392af2805031..97ac3f68a4b0 100644 --- a/.github/workflows/cluster-ui-release.yml +++ b/.github/workflows/cluster-ui-release.yml @@ -6,7 +6,6 @@ on: - 'release-*' paths: - 'pkg/ui/workspaces/cluster-ui/**/*.tsx?' - - 'pkg/ui/workspaces/cluster-ui/yarn.lock' - 'pkg/ui/workspaces/cluster-ui/package.json' jobs: @@ -28,14 +27,18 @@ jobs: path: ~/.cache/bazel key: ${{ runner.os }}-bazel-cache + - uses: pnpm/action-setup@v2 + with: + version: 8 + - name: Setup NodeJS uses: actions/setup-node@v3 with: node-version: 16 registry-url: 'https://registry.npmjs.org' always-auth: true - cache: 'yarn' - cache-dependency-path: pkg/ui/workspaces/cluster-ui/yarn.lock + cache: 'pnpm' + cache-dependency-path: 'pkg/ui/pnpm-lock.yaml' env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} @@ -64,10 +67,10 @@ jobs: - name: Build Cluster UI if: steps.version-check.outputs.published == 'no' run: | - yarn install --frozen-lockfile + pnpm install --frozen-lockfile bazel build //pkg/ui/workspaces/db-console/src/js:crdb-protobuf-client cp ../../../../_bazel/bin/pkg/ui/workspaces/db-console/src/js/protos.* ../db-console/src/js/ - yarn build + pnpm build - name: Create version tag and push if: steps.version-check.outputs.published == 'no'