From 8ce1ba7bf5a6afccda0e8f21bc400ee7d57214f7 Mon Sep 17 00:00:00 2001 From: sumeerbhola Date: Mon, 17 Jul 2023 11:45:48 -0400 Subject: [PATCH] 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/synctest.go | 72 ----------------------------- 3 files changed, 5 insertions(+), 73 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/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") - }, - }) -}