Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
84244: backupinfo: add tracing spans r=adityamaru a=stevendanna

We lost one of the tracing spans we added for checkpointing during a
refactor. While I was here, I added tracing spans to many of the
public methods and a couple of the private methods that are called
repeatedly by public methods.

For public methods, I added spans to methods that did substantial work
with external storage, excluding those that primarily just called
other methods that have had spans added.

Release note: None

Co-authored-by: Steven Danna <danna@cockroachlabs.com>
  • Loading branch information
craig[bot] and stevendanna committed Jul 13, 2022
2 parents 4d0cec8 + 0e7d90b commit 5ded4fc
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/ccl/backupccl/backupinfo/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ go_library(
"//pkg/util/protoutil",
"//pkg/util/syncutil",
"//pkg/util/timeutil",
"//pkg/util/tracing",
"@com_github_cockroachdb_errors//:errors",
],
)
Expand Down
43 changes: 43 additions & 0 deletions pkg/ccl/backupccl/backupinfo/manifest_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -187,6 +188,9 @@ func ReadBackupCheckpointManifest(
filename string,
encryption *jobspb.BackupEncryptionOptions,
) (backuppb.BackupManifest, int64, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.ReadBackupCheckpointManifest")
defer sp.Finish()

checkpointFile, err := readLatestCheckpointFile(ctx, exportStore, filename)
if err != nil {
return backuppb.BackupManifest{}, 0, err
Expand Down Expand Up @@ -216,6 +220,9 @@ func ReadBackupManifest(
filename string,
encryption *jobspb.BackupEncryptionOptions,
) (backuppb.BackupManifest, int64, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.ReadBackupManifest")
defer sp.Finish()

manifestFile, err := exportStore.ReadFile(ctx, filename)
if err != nil {
return backuppb.BackupManifest{}, 0, err
Expand Down Expand Up @@ -250,6 +257,9 @@ func readManifest(
manifestReader ioctx.ReadCloserCtx,
checksumReader ioctx.ReadCloserCtx,
) (backuppb.BackupManifest, int64, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.readManifest")
defer sp.Finish()

descBytes, err := mon.ReadAll(ctx, manifestReader, mem)
if err != nil {
return backuppb.BackupManifest{}, 0, err
Expand Down Expand Up @@ -345,6 +355,9 @@ func readBackupPartitionDescriptor(
filename string,
encryption *jobspb.BackupEncryptionOptions,
) (backuppb.BackupPartitionDescriptor, int64, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.readBackupPartitionDescriptor")
defer sp.Finish()

r, err := exportStore.ReadFile(ctx, filename)
if err != nil {
return backuppb.BackupPartitionDescriptor{}, 0, err
Expand Down Expand Up @@ -405,6 +418,9 @@ func readTableStatistics(
filename string,
encryption *jobspb.BackupEncryptionOptions,
) (*backuppb.StatsTable, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.readTableStatistics")
defer sp.Finish()

r, err := exportStore.ReadFile(ctx, filename)
if err != nil {
return nil, err
Expand Down Expand Up @@ -440,6 +456,9 @@ func GetStatisticsFromBackup(
encryption *jobspb.BackupEncryptionOptions,
backup backuppb.BackupManifest,
) ([]*stats.TableStatisticProto, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.GetStatisticsFromBackup")
defer sp.Finish()

// This part deals with pre-20.2 stats format where backup statistics
// are stored as a field in backup manifests instead of in their
// individual files.
Expand Down Expand Up @@ -472,6 +491,9 @@ func WriteBackupManifest(
encryption *jobspb.BackupEncryptionOptions,
desc *backuppb.BackupManifest,
) error {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.WriteBackupManifest")
defer sp.Finish()

sort.Sort(BackupFileDescriptors(desc.Files))

descBuf, err := protoutil.Marshal(desc)
Expand Down Expand Up @@ -533,6 +555,9 @@ func WriteBackupPartitionDescriptor(
encryption *jobspb.BackupEncryptionOptions,
desc *backuppb.BackupPartitionDescriptor,
) error {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.WriteBackupPartitionDescriptor")
defer sp.Finish()

descBuf, err := protoutil.Marshal(desc)
if err != nil {
return err
Expand Down Expand Up @@ -565,6 +590,9 @@ func WriteTableStatistics(
encryption *jobspb.BackupEncryptionOptions,
stats *backuppb.StatsTable,
) error {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.WriteTableStatistics")
defer sp.Finish()

statsBuf, err := protoutil.Marshal(stats)
if err != nil {
return err
Expand Down Expand Up @@ -596,6 +624,9 @@ func LoadBackupManifests(
makeExternalStorageFromURI cloud.ExternalStorageFromURIFactory,
encryption *jobspb.BackupEncryptionOptions,
) ([]backuppb.BackupManifest, int64, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.LoadBackupManifests")
defer sp.Finish()

backupManifests := make([]backuppb.BackupManifest, len(uris))
var reserved int64
defer func() {
Expand Down Expand Up @@ -636,6 +667,9 @@ func GetLocalityInfo(
encryption *jobspb.BackupEncryptionOptions,
prefix string,
) (jobspb.RestoreDetails_BackupLocalityInfo, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.GetLocalityInfo")
defer sp.Finish()

var info jobspb.RestoreDetails_BackupLocalityInfo
// Now get the list of expected partial per-store backup manifest filenames
// and attempt to find them.
Expand Down Expand Up @@ -856,6 +890,9 @@ func SanitizeLocalityKV(kv string) string {
func CheckForPreviousBackup(
ctx context.Context, exportStore cloud.ExternalStorage, defaultURI string,
) error {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.CheckForPreviousBackup")
defer sp.Finish()

redactedURI := backuputils.RedactURIForErrorMessage(defaultURI)
r, err := exportStore.ReadFile(ctx, backupbase.BackupManifestName)
if err == nil {
Expand Down Expand Up @@ -906,6 +943,9 @@ func WriteBackupManifestCheckpoint(
execCfg *sql.ExecutorConfig,
user username.SQLUsername,
) error {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.WriteBackupManifestCheckpoint")
defer sp.Finish()

defaultStore, err := execCfg.DistSQLSrv.ExternalStorageFromURI(ctx, storageURI, user)
if err != nil {
return err
Expand Down Expand Up @@ -1088,6 +1128,9 @@ func FetchPreviousBackups(
encryptionParams jobspb.BackupEncryptionOptions,
kmsEnv cloud.KMSEnv,
) ([]backuppb.BackupManifest, *jobspb.BackupEncryptionOptions, int64, error) {
ctx, sp := tracing.ChildSpan(ctx, "backupinfo.FetchPreviousBackups")
defer sp.Finish()

if len(prevBackupURIs) == 0 {
return nil, nil, 0, nil
}
Expand Down

0 comments on commit 5ded4fc

Please sign in to comment.