Merge #59103 #59134

59103: tracing: link child into parent, even if not verbose r=irfansharif a=tbg

Prior to this change, when a child was derived from a local parent, we
would not register the child with the parent. In effect, this meant that
any payloads in the child would not be collected.

Release note: None


59134: kvserver: avoid engine bypass in sideload storage r=jbowens a=tbg

While investigating #59126, I realized we were still going through the
engine for listing the files. I rectified this and as a result,
completely removed the in-mem implementation of the sideloaded storage
that we had in place: instead of it, we simply use an in-mem engine,
which now works like a charm thanks to not bypassing the engine for
listing the file contents (`filepath.Glob` --> `eng.List`).

While I was here, I added some defense in depth: it was unreasonable
that `TruncateTo` was returning an error as a result of failing to
perform best-effort cleanup on its directory.

I am no longer seeing the spurious errors observed in #59126 with
this change.

Touches #59126.
Touches #31913.
Fixes #58948.

Release note: None


Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>

pkg/kv/kvserver/BUILD.bazel

@@ -63,7 +63,6 @@ go_library(
         "replica_send.go",
         "replica_sideload.go",
         "replica_sideload_disk.go",
-        "replica_sideload_inmem.go",
         "replica_split_load.go",
         "replica_sst_snapshot_storage.go",
         "replica_stats.go",

pkg/kv/kvserver/replica_sideload_disk.go

@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/errors/oserror"
 	"golang.org/x/time/rate"
@@ -238,7 +239,8 @@ func (ss *diskSideloadStorage) TruncateTo(
 		// Not worth trying to figure out which one, just try to delete.
 		err := ss.eng.RemoveDir(ss.dir)
 		if err != nil && !oserror.IsNotExist(err) {
-			return bytesFreed, 0, errors.Wrapf(err, "while purging %q", ss.dir)
+			log.Infof(ctx, "unable to remove sideloaded dir %s: %v", ss.dir, err)
+			err = nil // handled
 		}
 	}
 	return bytesFreed, bytesRetained, nil
@@ -247,23 +249,33 @@ func (ss *diskSideloadStorage) TruncateTo(
 func (ss *diskSideloadStorage) forEach(
 	ctx context.Context, visit func(index uint64, filename string) error,
 ) error {
-	matches, err := filepath.Glob(filepath.Join(ss.dir, "i*.t*"))
+	matches, err := ss.eng.List(ss.dir)
+	if oserror.IsNotExist(err) {
+		// Nothing to do.
+		return nil
+	}
 	if err != nil {
 		return err
 	}
 	for _, match := range matches {
+		// List returns a relative path, but we want to deal in absolute paths
+		// because we may pass this back to `eng.{Delete,Stat}`, etc, and those
+		// expect absolute paths.
+		match = filepath.Join(ss.dir, match)
 		base := filepath.Base(match)
+		// Extract `i<log-index>` prefix from file.
 		if len(base) < 1 || base[0] != 'i' {
 			continue
 		}
 		base = base[1:]
 		upToDot := strings.SplitN(base, ".", 2)
 		logIdx, err := strconv.ParseUint(upToDot[0], 10, 64)
 		if err != nil {
-			return errors.Wrapf(err, "while parsing %q during TruncateTo", match)
+			log.Infof(ctx, "unexpected file %s in sideloaded directory %s", match, ss.dir)
+			continue
 		}
 		if err := visit(logIdx, match); err != nil {
-			return errors.Wrapf(err, "matching pattern %q", match)
+			return errors.Wrapf(err, "matching pattern %q on dir %s", match, ss.dir)
 		}
 	}
 	return nil