From f8e7dea25fa2fa52fe8cd5ce057bf26423605c04 Mon Sep 17 00:00:00 2001
From: Josh Imhoff <josh@cockroachlabs.com>
Date: Wed, 2 Aug 2023 18:34:49 -0400
Subject: [PATCH] storage: fix overwrite RemoteStorage factory bug

As of this commit, if both cfg.SharedStorage and cfg.RemoteStorageFactory are
set, CRDB uses cfg.SharedStorage. Note that eventually we will enable using
both at the same time, but with the abstractions available today, that is not
easy.

We prefer cfg.SharedStorage, since the Locator -> Storage mapping contained
in it is needed for CRDB to function properly.

Release note: None.
Epic: None.
---
 pkg/storage/pebble.go | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/pkg/storage/pebble.go b/pkg/storage/pebble.go
index be726c051a5c..63705f2264d1 100644
--- a/pkg/storage/pebble.go
+++ b/pkg/storage/pebble.go
@@ -1148,6 +1148,13 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) {
 	opts.EventListener = &el
 	p.wrappedIntentWriter = wrapIntentWriter(p)
 
+	// If both cfg.SharedStorage and cfg.RemoteStorageFactory are set, CRDB uses
+	// cfg.SharedStorage. Note that eventually we will enable using both at the
+	// same time, but we don't have the right abstractions in place to do that
+	// today.
+	//
+	// We prefer cfg.SharedStorage, since the Locator -> Storage mapping contained
+	// in it is needed for CRDB to function properly.
 	if cfg.SharedStorage != nil {
 		esWrapper := &externalStorageWrapper{p: p, es: cfg.SharedStorage, ctx: ctx}
 		opts.Experimental.RemoteStorage = remote.MakeSimpleFactory(map[remote.Locator]remote.Storage{
@@ -1155,10 +1162,10 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) {
 		})
 		opts.Experimental.CreateOnShared = true
 		opts.Experimental.CreateOnSharedLocator = ""
-	}
-
-	if cfg.RemoteStorageFactory != nil {
-		opts.Experimental.RemoteStorage = remoteStorageAdaptor{p: p, ctx: ctx, factory: cfg.RemoteStorageFactory}
+	} else {
+		if cfg.RemoteStorageFactory != nil {
+			opts.Experimental.RemoteStorage = remoteStorageAdaptor{p: p, ctx: ctx, factory: cfg.RemoteStorageFactory}
+		}
 	}
 
 	// Read the current store cluster version.