kvserver: avoid engine bypass in sideload storage

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. This makes sense, as in this test the sideloaded storage
is set up with an in-memory engine. The old code, which read from
actual disk, would try to glob `auxiliary/...` (i.e. some path relative
to the cwd - oops), which would return no results. As a result, the
sideloaded storage would try to invoke `eng.Remove("auxiliary/...")`
which the engine would refuse, since there would actually be (in-mem)
files stored in the engine. This all resolves now that everyone is
looking at the same files.

It's possible that this fixes the issue with BenchmarkUserFile, as
previously errors from TruncateTo could block the raft log queue,
which in turn could at least slow down the test, but it's hard to
say for sure.

Touches #59126.
Touches #31913.

Release note: None

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