Merge cockroachdb#112130 cockroachdb#112624

112130: cli: add job traces for traceable jobs to the debug zip r=adityamaru a=adityamaru This change teaches the debug zip to collect the traces for traceable jobs (backup, restore, import, pcr) that are in a running or reverting state at the time the zip is collected. These traces are dumped in a `/jobs/<jobID>/<timestamp>/trace.zip` file and rely on the existing `tracing/zippr` that is used by `cockroach debug job-trace` to collect the required information. Informs: cockroachdb#111886 Release note (cli change): `cockroach debug zip` will now collect the inflight traces of traceable jobs such as backup, restore, import, c2c and dump them in a `jobs/` subdirectory in the zip. 112624: logictest: disable metamorphic testing for mixed version test r=rafiss a=rafiss We've seen that these tests can get quite slow sometimes. One theory is that this is caused by metamorphic test settings, so this PR disables them so we can determine if it helps with test stability. informs cockroachdb#112621 Release note: None Co-authored-by: adityamaru <[email protected]> Co-authored-by: Rafi Shamim <[email protected]>
herkolategan · Oct 18, 2023 · 2cb7c2a · 2cb7c2a
3 parents 282ff9a + 0506ef5 + 48eceb7
commit 2cb7c2a
Show file tree

Hide file tree

Showing 37 changed files with 371 additions and 134 deletions.
diff --git a/pkg/cli/BUILD.bazel b/pkg/cli/BUILD.bazel
@@ -359,6 +359,7 @@ go_test(
     deps = [
         "//pkg/base",
         "//pkg/build",
+        "//pkg/ccl/backupccl",
         "//pkg/cli/clicfg",
         "//pkg/cli/clienturl",
         "//pkg/cli/clierror",
@@ -374,6 +375,7 @@ go_test(
         "//pkg/gossip",
         "//pkg/jobs",
         "//pkg/jobs/jobspb",
+        "//pkg/jobs/jobstest",
         "//pkg/keys",
         "//pkg/kv",
         "//pkg/kv/kvclient/kvtenant",
@@ -402,6 +404,7 @@ go_test(
         "//pkg/storage",
         "//pkg/testutils",
         "//pkg/testutils/datapathutils",
+        "//pkg/testutils/jobutils",
         "//pkg/testutils/listenerutil",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/skip",

diff --git a/pkg/cli/cliflags/flags.go b/pkg/cli/cliflags/flags.go
@@ -1690,6 +1690,15 @@ require any additional stop-the-world operations to be collected.
 `,
 	}
 
+	ZipIncludeRunningJobTraces = FlagInfo{
+		Name: "include-running-job-traces",
+		Description: `
+Include information about each running, traceable job in jobs/*/*/trace.zip
+files. This involves collecting cluster-wide traces for each running job in the
+cluster.
+`,
+	}
+
 	ZipCPUProfileDuration = FlagInfo{
 		Name: "cpu-profile-duration",
 		Description: `

diff --git a/pkg/cli/clisqlcfg/context.go b/pkg/cli/clisqlcfg/context.go
@@ -184,7 +184,7 @@ func (c *Context) MakeConn(url string) (clisqlclient.Conn, error) {
 	// By default, all connections will use the underlying driver to infer
 	// result types. This should be set back to false for any use case where the
 	// results are only shown for textual display.
-	conn.SetAlwaysInferResultTypes(true)
+	_ = conn.SetAlwaysInferResultTypes(true)
 
 	return conn, nil
 }
@@ -198,7 +198,7 @@ func (c *Context) Run(ctx context.Context, conn clisqlclient.Conn) error {
 	// Anything using a SQL shell (e.g. `cockroach sql` or `demo`), only needs
 	// to show results in text format, so the underlying driver doesn't need to
 	// infer types.
-	conn.SetAlwaysInferResultTypes(false)
+	_ = conn.SetAlwaysInferResultTypes(false)
 
 	// Open the connection to make sure everything is OK before running any
 	// statements. Performs authentication.

diff --git a/pkg/cli/clisqlclient/api.go b/pkg/cli/clisqlclient/api.go
@@ -70,8 +70,9 @@ type Conn interface {
 
 	// SetAlwaysInferResultTypes configures the alwaysInferResultTypes flag, which
 	// determines if the client should use the underlying driver to infer result
-	// types.
-	SetAlwaysInferResultTypes(b bool)
+	// types. It returns a method that can be used to reset the configuration to
+	// its previous value.
+	SetAlwaysInferResultTypes(b bool) func()
 
 	// GetServerMetadata returns details about the CockroachDB node
 	// this connection is connected to.

diff --git a/pkg/cli/clisqlclient/conn.go b/pkg/cli/clisqlclient/conn.go
@@ -164,8 +164,12 @@ func (c *sqlConn) SetMissingPassword(missing bool) {
 }
 
 // SetAlwaysInferResultTypes implements the Conn interface.
-func (c *sqlConn) SetAlwaysInferResultTypes(b bool) {
+func (c *sqlConn) SetAlwaysInferResultTypes(b bool) func() {
+	oldVal := c.alwaysInferResultTypes
 	c.alwaysInferResultTypes = b
+	return func() {
+		c.alwaysInferResultTypes = oldVal
+	}
 }
 
 // EnsureConn (re-)establishes the connection to the server.

diff --git a/pkg/cli/context.go b/pkg/cli/context.go
@@ -363,6 +363,10 @@ type zipContext struct {
 	// latency.
 	includeStacks bool
 
+	// includeRunningJobTraces includes the active traces of each running
+	// Traceable job in individual jobs/*/ranges/trace.zip files.
+	includeRunningJobTraces bool
+
 	// The log/heap/etc files to include.
 	files fileSelection
 }
@@ -381,6 +385,9 @@ func setZipContextDefaults() {
 	// Goroutine stack dumps require a "stop the world" operation on the server side,
 	// which impacts performance and SQL service latency.
 	zipCtx.includeStacks = true
+	// Job traces for running Traceable jobs involves fetching cluster wide traces
+	// for each job.
+	zipCtx.includeRunningJobTraces = false
 	zipCtx.cpuProfDuration = 5 * time.Second
 	zipCtx.concurrency = 15
 

diff --git a/pkg/cli/debug_job_trace_test.go b/pkg/cli/debug_job_trace_test.go
@@ -77,6 +77,7 @@ func (r *traceSpanResumer) CollectProfile(_ context.Context, _ interface{}) erro
 func TestDebugJobTrace(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
+	defer jobs.ResetConstructors()()
 
 	ctx := context.Background()
 	argsFn := func(args *base.TestServerArgs) {

diff --git a/pkg/cli/flags.go b/pkg/cli/flags.go
@@ -702,6 +702,7 @@ func init() {
 		cliflagcfg.IntFlag(f, &zipCtx.concurrency, cliflags.ZipConcurrency)
 		cliflagcfg.BoolFlag(f, &zipCtx.includeRangeInfo, cliflags.ZipIncludeRangeInfo)
 		cliflagcfg.BoolFlag(f, &zipCtx.includeStacks, cliflags.ZipIncludeGoroutineStacks)
+		cliflagcfg.BoolFlag(f, &zipCtx.includeRunningJobTraces, cliflags.ZipIncludeRunningJobTraces)
 	}
 	// List-files + Zip commands.
 	for _, cmd := range []*cobra.Command{debugZipCmd, debugListFilesCmd} {

diff --git a/pkg/cli/testdata/zip/partial1 b/pkg/cli/testdata/zip/partial1
@@ -263,3 +263,4 @@ debug zip --concurrency=1 --cpu-profile-duration=0s /dev/null
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/partial1_excluded b/pkg/cli/testdata/zip/partial1_excluded
@@ -166,3 +166,4 @@ debug zip /dev/null --concurrency=1 --exclude-nodes=2 --cpu-profile-duration=0
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/partial2 b/pkg/cli/testdata/zip/partial2
@@ -165,3 +165,4 @@ debug zip --concurrency=1 --cpu-profile-duration=0 /dev/null
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/testzip b/pkg/cli/testdata/zip/testzip
@@ -126,3 +126,4 @@ debug zip --concurrency=1 --cpu-profile-duration=1s /dev/null
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/testzip_concurrent b/pkg/cli/testdata/zip/testzip_concurrent
@@ -1,5 +1,6 @@
 zip
 ----
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
 [cluster] creating output file /dev/null...
 [cluster] creating output file /dev/null: done
 [cluster] discovering virtual clusters...

diff --git a/pkg/cli/testdata/zip/testzip_exclude_goroutine_stacks b/pkg/cli/testdata/zip/testzip_exclude_goroutine_stacks
@@ -125,4 +125,5 @@ debug zip --concurrency=1 --cpu-profile-duration=1s --include-goroutine-stacks=f
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
 [cluster] NOTE: Omitted node-level goroutine stack dumps from this debug zip bundle. Use the --include-goroutine-stacks flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/testzip_external_process_virtualization b/pkg/cli/testdata/zip/testzip_external_process_virtualization
@@ -161,3 +161,4 @@ debug zip --concurrency=1 --cpu-profile-duration=1s /dev/null
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/testzip_include_goroutine_stacks b/pkg/cli/testdata/zip/testzip_include_goroutine_stacks
@@ -126,3 +126,4 @@ debug zip --concurrency=1 --cpu-profile-duration=1s /dev/null
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/testzip_include_range_info b/pkg/cli/testdata/zip/testzip_include_range_info
@@ -126,3 +126,4 @@ debug zip --concurrency=1 --cpu-profile-duration=1s --include-range-info /dev/nu
 [cluster] pprof summary script... writing binary output: debug/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/testzip_shared_process_virtualization b/pkg/cli/testdata/zip/testzip_shared_process_virtualization
@@ -286,3 +286,4 @@ debug zip --concurrency=1 --cpu-profile-duration=1s /dev/null
 [cluster] pprof summary script... writing binary output: debug/cluster/test-tenant/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/cluster/test-tenant/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/cluster/test-tenant/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/testdata/zip/testzip_shared_process_virtualization_with_default_tenant b/pkg/cli/testdata/zip/testzip_shared_process_virtualization_with_default_tenant
@@ -286,3 +286,4 @@ debug zip --concurrency=1 --cpu-profile-duration=1s /dev/null
 [cluster] pprof summary script... writing binary output: debug/cluster/test-tenant/pprof-summary.sh... done
 [cluster] hot range summary script... writing binary output: debug/cluster/test-tenant/hot-ranges.sh... done
 [cluster] tenant hot range summary script... writing binary output: debug/cluster/test-tenant/hot-ranges-tenant.sh... done
+[cluster] NOTE: Omitted traces of running jobs from this debug zip bundle. Use the --include-running-job-traces flag to enable the fetching of this data.
diff --git a/pkg/cli/zip.go b/pkg/cli/zip.go
@@ -12,6 +12,7 @@ package cli
 
 import (
 	"context"
+	"database/sql/driver"
 	"fmt"
 	"io"
 	"net"
@@ -23,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
 	"github.com/cockroachdb/cockroach/pkg/cli/clisqlclient"
 	"github.com/cockroachdb/cockroach/pkg/cli/clisqlexec"
+	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/server/profiler"
@@ -31,7 +33,11 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
 	"github.com/cockroachdb/cockroach/pkg/util/buildutil"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
+	"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
+	tracezipper "github.com/cockroachdb/cockroach/pkg/util/tracing/zipper"
 	"github.com/cockroachdb/errors"
 	"github.com/jackc/pgconn"
 	"github.com/marusama/semaphore"
@@ -253,7 +259,7 @@ func runDebugZip(cmd *cobra.Command, args []string) (retErr error) {
 			sqlConn, err := makeTenantSQLClient(ctx, "cockroach zip", useSystemDb, tenant.TenantName)
 			// The zip output is sent directly into a text file, so the results should
 			// be scanned into strings.
-			sqlConn.SetAlwaysInferResultTypes(false)
+			_ = sqlConn.SetAlwaysInferResultTypes(false)
 			if err != nil {
 				_ = s.fail(errors.Wrap(err, "unable to open a SQL session. Debug information will be incomplete"))
 			} else {
@@ -344,6 +350,12 @@ done
 		}
 	}
 
+	if !zipCtx.includeRunningJobTraces {
+		zr.info("NOTE: Omitted traces of running jobs from this debug zip bundle." +
+			" Use the --" + cliflags.ZipIncludeRunningJobTraces.Name + " flag to enable the fetching of this" +
+			" data.")
+	}
+
 	if !zipCtx.includeStacks {
 		zr.info("NOTE: Omitted node-level goroutine stack dumps from this debug zip bundle." +
 			" Use the --" + cliflags.ZipIncludeGoroutineStacks.Name + " flag to enable the fetching of this" +
@@ -377,6 +389,97 @@ func maybeAddProfileSuffix(name string) string {
 	return name
 }
 
+type jobTrace struct {
+	jobID   jobspb.JobID
+	traceID tracingpb.TraceID
+}
+
+// dumpTraceableJobTraces collects the traces for some "traceable" jobs that are
+// in a running state. The job types in this list are the ones that have
+// explicitly implemented the TraceableJob interface.
+func (zc *debugZipContext) dumpTraceableJobTraces() error {
+	ctx := context.Background()
+	rows, err := zc.firstNodeSQLConn.Query(ctx,
+		`WITH
+latestprogress AS (
+  SELECT job_id, value
+  FROM system.job_info AS progress
+  WHERE info_key = 'legacy_progress'
+  ORDER BY written desc
+),
+jobpage AS (
+  SELECT id
+  FROM system.jobs@jobs_status_created_idx
+  WHERE (job_type IN ($1, $2, $3, $4)) AND (status IN ($5, $6))
+  ORDER BY id
+)
+SELECT distinct (id), latestprogress.value AS progress
+FROM jobpage AS j
+INNER JOIN latestprogress ON j.id = latestprogress.job_id;`,
+		jobspb.TypeBackup.String(),
+		jobspb.TypeRestore.String(),
+		jobspb.TypeImport.String(),
+		jobspb.TypeReplicationStreamIngestion.String(),
+		"running",
+		"reverting",
+	)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if rows != nil {
+			if err := rows.Close(); err != nil {
+				log.Warningf(ctx, "failed to close with error: %v", err)
+			}
+		}
+	}()
+	vals := make([]driver.Value, 2)
+	jobTraces := make([]jobTrace, 0)
+	for err = rows.Next(vals); err == nil; err = rows.Next(vals) {
+		jobID, ok := vals[0].(int64)
+		if !ok {
+			return errors.New("failed to parse jobID")
+		}
+		progressBytes, ok := vals[1].([]byte)
+		if !ok {
+			return errors.New("failed to parse progress bytes")
+		}
+		progress := &jobspb.Progress{}
+		if err := protoutil.Unmarshal(progressBytes, progress); err != nil {
+			return err
+		}
+		jobTraces = append(jobTraces, jobTrace{jobID: jobspb.JobID(jobID), traceID: progress.TraceID})
+	}
+
+	func() {
+		// Debug zip collection sets this to false since results from the query are
+		// all dumped into txt files. In our case we parse the results of the query
+		// with their respective types and pre-process the information before
+		// dumping into a zip file.
+		reset := zc.firstNodeSQLConn.SetAlwaysInferResultTypes(true)
+		defer reset()
+		for _, jobTrace := range jobTraces {
+			inflightTraceZipper := tracezipper.MakeSQLConnInflightTraceZipper(zc.firstNodeSQLConn.GetDriverConn())
+			jobZip, err := inflightTraceZipper.Zip(ctx, int64(jobTrace.traceID))
+			if err != nil {
+				log.Warningf(ctx, "failed to collect inflight trace zip for job %d: %v", jobTrace.jobID, err)
+				continue
+			}
+
+			ts := timeutil.Now().Format(`20060102150405`)
+			name := fmt.Sprintf("%s/jobs/%d/%s/trace.zip", zc.prefix, jobTrace.jobID, ts)
+			s := zc.clusterPrinter.start("requesting traces for job %d", jobTrace.jobID)
+			if err := zc.z.createRaw(s, name, jobZip); err != nil {
+				log.Warningf(ctx, "failed to write inflight trace zip for job %d to file %s: %v",
+					jobTrace.jobID, name, err)
+				continue
+			}
+		}
+	}()
+
+	return nil
+}
+
 // dumpTableDataForZip runs the specified SQL query and stores the
 // result. Errors encountered while running the SQL query are stored
 // in an error file in the zip file, and dumpTableDataForZip() returns

diff --git a/pkg/cli/zip_cluster_wide.go b/pkg/cli/zip_cluster_wide.go
@@ -188,5 +188,12 @@ func (zc *debugZipContext) collectClusterData(
 		}
 	}
 
+	if zipCtx.includeRunningJobTraces {
+		zc.clusterPrinter.info("collecting the inflight traces for jobs")
+		if err := zc.dumpTraceableJobTraces(); err != nil {
+			return &serverpb.NodesListResponse{}, nil, err
+		}
+	}
+
 	return nodesList, livenessByNodeID, nil
 }