storage: fix setting shared creator ID

The `AtLeast` version check was incorrect and we weren't setting the
Creator ID.

The code is improved to also set the Creator ID when the version is
bumped.

We are missing unit tests for shared storage but there is currently no
way to use a test / in-memory shared store; we will add tests when we
address this.

Epic: none
Release note: None

pkg/roachpb/version.go

@@ -49,9 +49,9 @@ func (v Version) LessEq(otherV Version) bool {
 	return v.Equal(otherV) || v.Less(otherV)
 }
 
-// AtLeast returns true if the receiver is greater than or requal to the parameter.
+// AtLeast returns true if the receiver is greater than or equal to the parameter.
 func (v Version) AtLeast(otherV Version) bool {
-	return !otherV.Less(v)
+	return !v.Less(otherV)
 }
 
 // String implements the fmt.Stringer interface.

pkg/roachpb/version_test.go

@@ -16,7 +16,7 @@ import (
 	"github.com/kr/pretty"
 )
 
-func TestVersionLess(t *testing.T) {
+func TestVersionCmp(t *testing.T) {
 	v := func(major, minor, patch, internal int32) Version {
 		return Version{
 			Major:    major,
@@ -51,6 +51,12 @@ func TestVersionLess(t *testing.T) {
 			if a, e := test.v1.Less(test.v2), test.less; a != e {
 				t.Errorf("expected %s < %s? %t; got %t", pretty.Sprint(test.v1), pretty.Sprint(test.v2), e, a)
 			}
+			if a, e := test.v1.Equal(test.v2), test.v1 == test.v2; a != e {
+				t.Errorf("expected %s = %s? %t; got %t", pretty.Sprint(test.v1), pretty.Sprint(test.v2), e, a)
+			}
+			if a, e := test.v1.AtLeast(test.v2), test.v1 == test.v2 || !test.less; a != e {
+				t.Errorf("expected %s >= %s? %t; got %t", pretty.Sprint(test.v1), pretty.Sprint(test.v2), e, a)
+			}
 		})
 	}
 }

pkg/storage/pebble.go

@@ -843,14 +843,10 @@ func (p *Pebble) SetStoreID(ctx context.Context, storeID int32) error {
 	if p == nil {
 		return nil
 	}
-	if p.storeIDPebbleLog != nil {
-		p.storeIDPebbleLog.Set(ctx, storeID)
-	}
+	p.storeIDPebbleLog.Set(ctx, storeID)
 	// Note that SetCreatorID only does something if shared storage is configured
 	// in the pebble options. The version gate protects against accidentally
 	// setting the creator ID on an older store.
-	// TODO(radu): we don't yet have a complete story about how we will transition
-	// an existing store to use shared storage.
 	if storeID != base.TempStoreID && p.minVersion.AtLeast(clusterversion.ByKey(clusterversion.V23_1SetPebbleCreatorID)) {
 		if err := p.db.SetCreatorID(uint64(storeID)); err != nil {
 			return err
@@ -1999,6 +1995,15 @@ func (p *Pebble) SetMinVersion(version roachpb.Version) error {
 		return err
 	}
 
+	// Set the shared object creator ID if the version is high enough. See SetStoreID().
+	if version.AtLeast(clusterversion.ByKey(clusterversion.V23_1SetPebbleCreatorID)) {
+		if storeID := p.storeIDPebbleLog.Get(); storeID != 0 && storeID != base.TempStoreID {
+			if err := p.db.SetCreatorID(uint64(storeID)); err != nil {
+				return err
+			}
+		}
+	}
+
 	// Pebble has a concept of format major versions, similar to cluster
 	// versions. Backwards incompatible changes to Pebble's on-disk
 	// format are gated behind new format major versions. Bumping the