kvserver,storage: pass Server's ExternalStorage factory to pebble

Release note: none.
Epic: none.

Author: dt

pkg/kv/kvserver/BUILD.bazel

@@ -108,7 +108,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/base",
-        "//pkg/cloud",
         "//pkg/clusterversion",
         "//pkg/config",
         "//pkg/config/zonepb",
@@ -176,7 +175,6 @@ go_library(
         "//pkg/roachpb",
         "//pkg/rpc",
         "//pkg/rpc/nodedialer",
-        "//pkg/security/username",
         "//pkg/server/status",
         "//pkg/server/telemetry",
         "//pkg/settings",
@@ -201,7 +199,6 @@ go_library(
         "//pkg/util/grunning",
         "//pkg/util/hlc",
         "//pkg/util/humanizeutil",
-        "//pkg/util/ioctx",
         "//pkg/util/iterutil",
         "//pkg/util/limit",
         "//pkg/util/log",

pkg/kv/kvserver/app_batch.go

@@ -13,7 +13,6 @@ package kvserver
 import (
 	"context"
 
-	"github.com/cockroachdb/cockroach/pkg/cloud"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
@@ -155,7 +154,6 @@ type postAddEnv struct {
 	eng         storage.Engine
 	sideloaded  logstore.SideloadStorage
 	bulkLimiter *rate.Limiter
-	external    *cloud.ExternalStorageAccessor
 }
 
 func (b *appBatch) runPostAddTriggers(

pkg/kv/kvserver/replica_app_batch.go

@@ -149,7 +149,6 @@ func (b *replicaAppBatch) Stage(
 		eng:         b.r.store.TODOEngine(),
 		sideloaded:  b.r.raftMu.sideloaded,
 		bulkLimiter: b.r.store.limiters.BulkIOWriteRate,
-		external:    b.r.store.cfg.ExternalStorage,
 	}); err != nil {
 		return nil, err
 	}

pkg/kv/kvserver/replica_proposal.go

@@ -16,7 +16,6 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/cloud"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
@@ -27,14 +26,12 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/readsummary/rspb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/uncertainty"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
-	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
-	"github.com/cockroachdb/cockroach/pkg/util/ioctx"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -622,29 +619,26 @@ func addSSTablePreApply(
 	sst kvserverpb.ReplicatedEvalResult_AddSSTable,
 ) bool {
 	if sst.RemoteFilePath != "" {
-		// TODO(dt, bilal, msbutler): Replace this with eng.IngestRemoteFile()
-		log.Warningf(ctx, "EXPERIMENTAL AddSSTABLE REMOTE FILE UNSUPPORTED; downloading %s ("+
-			"with size %d) from %s and adding it whole, ignoring span %s", sst.RemoteFilePath,
-			sst.BackingFileSize, sst.RemoteFileLoc, sst.Span)
-		s, err := env.external.OpenURL(ctx, sst.RemoteFileLoc, username.SQLUsername{})
-		if err != nil {
-			log.Fatalf(ctx, "failed to open remote location %q below raft: %v", sst.RemoteFileLoc, err)
-		}
-		r, _, err := s.ReadFile(ctx, sst.RemoteFilePath, cloud.ReadOptions{})
-		if err != nil {
-			log.Fatalf(ctx, "failed to open remote file path %q in %q below raft: %v", sst.RemoteFilePath, sst.RemoteFileLoc, err)
-		}
-		content, err := ioctx.ReadAll(ctx, r)
-		r.Close(ctx)
-		if err != nil {
-			log.Fatalf(ctx, "failed to read remote file %q in %q below raft: %v", sst.RemoteFilePath, sst.RemoteFileLoc, err)
-		}
-		sst.Data = content
-		sst.CRC32 = util.CRC32(content)
-		log.Infof(ctx, "Unsupported RemoteFile AddSSTABLE downloaded %q, read %d bytes", sst.RemoteFileLoc, len(content))
-		if err := env.sideloaded.Put(ctx, index, term, content); err != nil {
-			log.Fatalf(ctx, "failed to write downloaded remote file %q in %q below raft: %v", sst.RemoteFilePath, sst.RemoteFileLoc, err)
-		}
+		log.Infof(ctx,
+			"EXPERIMENTAL AddSSTABLE EXTERNAL %s (size %d, span %s) from %s",
+			sst.RemoteFilePath,
+			sst.BackingFileSize,
+			sst.Span,
+			sst.RemoteFileLoc,
+		)
+		// TODO(bilal): replace this with the real ingest.
+		/*
+			start := storage.EngineKey{Key: sst.Span.Key}
+			end := storage.EngineKey{Key: sst.Span.EndKey}
+
+			externalFile := pebble.ExternalFile{
+				Locator:         shared.Locator(sst.RemoteFileLoc),
+				ObjName:         sst.RemoteFilePath,
+				Size:            sst.BackingFileSize,
+				SmallestUserKey: start.Encode(),
+				LargestUserKey:  end.Encode(),
+			}*/
+		log.Fatalf(ctx, "Unsupported IngestRemoteFile")
 	}
 	checksum := util.CRC32(sst.Data)
 

pkg/kv/kvserver/store.go

@@ -25,7 +25,6 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
-	"github.com/cockroachdb/cockroach/pkg/cloud"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/config"
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
@@ -1224,8 +1223,6 @@ type StoreConfig struct {
 
 	// RangeLogWriter is used to write entries to the system.rangelog table.
 	RangeLogWriter RangeLogWriter
-
-	ExternalStorage *cloud.ExternalStorageAccessor
 }
 
 // logRangeAndNodeEventsEnabled is used to enable or disable logging range events

pkg/server/config.go

@@ -236,6 +236,7 @@ type BaseConfig struct {
 
 	// SharedStorage is specified to enable disaggregated shared storage.
 	SharedStorage string
+	*cloud.ExternalStorageAccessor
 
 	// StartDiagnosticsReporting starts the asynchronous goroutine that
 	// checks for CockroachDB upgrades and periodically reports
@@ -310,6 +311,7 @@ func (cfg *BaseConfig) SetDefaults(
 	cfg.AmbientCtx.AddLogTag("n", cfg.IDContainer)
 	cfg.Config.InitDefaults()
 	cfg.InitTestingKnobs()
+	cfg.ExternalStorageAccessor = cloud.NewExternalStorageAccessor()
 }
 
 // InitTestingKnobs sets up any testing knobs based on e.g. envvars.
@@ -823,6 +825,7 @@ func (cfg *Config) CreateEngines(ctx context.Context) (Engines, error) {
 			// TODO(radu): move up all remaining settings below so they apply to in-memory stores as well.
 			addCfgOpt(storage.MaxOpenFiles(int(openFileLimitPerStore)))
 			addCfgOpt(storage.MaxWriterConcurrency(2))
+			addCfgOpt(storage.RemoteStorageFactory(cfg.ExternalStorageAccessor))
 			if sharedStorage != nil {
 				addCfgOpt(storage.SharedStorage(sharedStorage))
 			}

pkg/server/config_test.go

@@ -130,6 +130,8 @@ func TestReadEnvironmentVariables(t *testing.T) {
 	cfg.AmbientCtx.Tracer = nil
 	cfgExpected.Tracer = nil
 	cfgExpected.AmbientCtx.Tracer = nil
+	cfg.ExternalStorageAccessor = nil
+	cfgExpected.ExternalStorageAccessor = nil
 	// Temp storage disk monitors will have slightly different names, so we
 	// override them to point to the same one.
 	cfgExpected.TempStorageConfig.Mon = cfg.TempStorageConfig.Mon

pkg/server/server.go

@@ -25,7 +25,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/blobs"
 	"github.com/cockroachdb/cockroach/pkg/blobs/blobspb"
 	"github.com/cockroachdb/cockroach/pkg/build"
-	"github.com/cockroachdb/cockroach/pkg/cloud"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/gossip"
 	"github.com/cockroachdb/cockroach/pkg/inspectz"
@@ -865,7 +864,6 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 		KVFlowHandles:                admissionControl.storesFlowControl,
 		KVFlowHandleMetrics:          admissionControl.kvFlowHandleMetrics,
 		SchedulerLatencyListener:     admissionControl.schedulerLatencyListener,
-		ExternalStorage:              cloud.NewExternalStorageAccessor(),
 	}
 	if storeTestingKnobs := cfg.TestingKnobs.Store; storeTestingKnobs != nil {
 		storeCfg.TestingKnobs = *storeTestingKnobs.(*kvserver.StoreTestingKnobs)
@@ -2059,7 +2057,7 @@ func (s *Server) PreStart(ctx context.Context) error {
 		nil, /* TenantExternalIORecorder */
 		s.registry,
 	)
-	if err := s.node.storeCfg.ExternalStorage.Init(
+	if err := s.cfg.ExternalStorageAccessor.Init(
 		s.externalStorageBuilder.makeExternalStorage, s.externalStorageBuilder.makeExternalStorageFromURI,
 	); err != nil {
 		return err

pkg/storage/BUILD.bazel

@@ -56,6 +56,7 @@ go_library(
         "//pkg/kv/kvserver/diskmap",
         "//pkg/kv/kvserver/uncertainty",
         "//pkg/roachpb",
+        "//pkg/security/username",
         "//pkg/settings",
         "//pkg/settings/cluster",
         "//pkg/sql/catalog/fetchpb",

pkg/storage/open.go

@@ -185,6 +185,14 @@ func SharedStorage(sharedStorage cloud.ExternalStorage) ConfigOption {
 	}
 }
 
+// RemoteStorageFactory enables use of remote storage (experimental).
+func RemoteStorageFactory(accessor *cloud.ExternalStorageAccessor) ConfigOption {
+	return func(cfg *engineConfig) error {
+		cfg.RemoteStorageFactory = accessor
+		return nil
+	}
+}
+
 // MaxConcurrentCompactions configures the maximum number of concurrent
 // compactions an Engine will execute.
 func MaxConcurrentCompactions(n int) ConfigOption {