Skip to content

Commit

Permalink
settingswatcher: version guard support for clusters bootstrapped at o…
Browse files Browse the repository at this point in the history 
…ld versions

When a cluster is bootstrapping, the sql server is initialized before
the cluster version is populated in the DB. Previously, the version
guard utility was unable to handle this state if the version is older
than the maxVersion used to initialize the version guard. Now, the
versionGuard handles this bootstrapping state by falling back on the
in-memory cluster version.

Part of cockroachdb#94843

Release note: none
  • Loading branch information
@jeffswenson
jeffswenson committed Mar 13, 2023
1 parent 9ed78bf commit 506a3ef
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 33 deletions.
6 changes: 5 additions & 1 deletion pkg/server/settingswatcher/settings_watcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ func (s *SettingsWatcher) GetStorageClusterVersion() clusterversion.ClusterVersi
return s.mu.storageClusterVersion
}

// errVersionSettingNotFound is returned by GetClusterVersionFromStorage if the
// 'version' setting is not present in the system.settings table.
var errVersionSettingNotFound = errors.New("got nil value for tenant cluster version row")

// GetClusterVersionFromStorage reads the cluster version from the storage via
// the given transaction.
func (s *SettingsWatcher) GetClusterVersionFromStorage(
Expand All @@ -453,7 +457,7 @@ func (s *SettingsWatcher) GetClusterVersionFromStorage(
return clusterversion.ClusterVersion{}, err
}
if row.Value == nil {
return clusterversion.ClusterVersion{}, errors.New("got nil value for tenant cluster version row")
return clusterversion.ClusterVersion{}, errVersionSettingNotFound
}
_, val, _, err := s.dec.DecodeRow(roachpb.KeyValue{Key: row.Key, Value: *row.Value}, nil /* alloc */)
if err != nil {
Expand Down
49 changes: 38 additions & 11 deletions pkg/server/settingswatcher/version_guard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
)

// VersionGuard is a utility for checking the cluster version in a transaction.
Expand All @@ -33,30 +35,55 @@ import (
// ...
// }
type VersionGuard struct {
maxGateIsActive bool
txnVersion clusterversion.ClusterVersion
activeVersion clusterversion.ClusterVersion
}

// MakeVersionGuard constructs a version guard for the transaction.
func (s *SettingsWatcher) MakeVersionGuard(
ctx context.Context, txn *kv.Txn, maxGate clusterversion.Key,
) (VersionGuard, error) {
if s.settings.Version.IsActive(ctx, maxGate) {
activeVersion := s.settings.Version.ActiveVersion(ctx)
if activeVersion.IsActive(maxGate) {
return VersionGuard{activeVersion: activeVersion}, nil
}

txnVersion, err := s.GetClusterVersionFromStorage(ctx, txn)
if errors.Is(err, errVersionSettingNotFound) {
// The version setting is set via the upgrade job. Since there are some
// permanent upgrades that run unconditionally when a cluster is
// created, the version setting is populated during the cluster boot
// strap process.
//
// The case where a setting is old and the version is missing is
// uncommon and mostly shows up during tests. Usually clusters are
// bootstrapped at the binary version, so a new cluster will hit the
// fast path of the version guard since the active version is the most
// recent version gate.
//
// However, during testing the sql server may be bootstrapped at an old
// cluster version and hits the slow path because the cluster version
// is behind the maxGate version. In this case we treat the in-memory
// version as the active setting.
//
// Using the in-memory version is safe from race conditions because the
// transaction did read the missing value from the system.settings
// table and will get retried if the setting changes.
log.Ops.Warningf(ctx, "the 'version' setting was not found in the system.setting table using in-memory settings %v", activeVersion)
return VersionGuard{
maxGateIsActive: true,
activeVersion: activeVersion,
}, nil
}
txnVersion, err := s.GetClusterVersionFromStorage(ctx, txn)
if err != nil {
return VersionGuard{}, err
}

return VersionGuard{
txnVersion: txnVersion,
}, err
activeVersion: txnVersion,
}, nil
}

// IsActive returns true if the transaction should treat the version guard as
// active.
func (v *VersionGuard) IsActive(version clusterversion.Key) bool {
if v.maxGateIsActive {
return true
}
return v.txnVersion.IsActive(version)
return v.activeVersion.IsActive(version)
}
68 changes: 47 additions & 21 deletions pkg/server/settingswatcher/version_guard_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,42 +35,64 @@ func TestVersionGuard(t *testing.T) {

type testCase struct {
name string
storageVersion clusterversion.Key
storageVersion *clusterversion.Key
settingsVersion clusterversion.Key
checkVersions map[clusterversion.Key]bool
}

initialVersion := clusterversion.V22_2
startVersion := clusterversion.V23_1Start
maxVersion := clusterversion.V23_1

tests := []testCase{
{
name: "bootstrap-old-version",
storageVersion: nil,
settingsVersion: initialVersion,
checkVersions: map[clusterversion.Key]bool{
initialVersion: true,
startVersion: false,
maxVersion: false,
},
},
{
name: "bootstrap-current-version",
storageVersion: nil,
settingsVersion: maxVersion,
checkVersions: map[clusterversion.Key]bool{
initialVersion: true,
startVersion: true,
maxVersion: true,
},
},
{
name: "unfinalized",
storageVersion: initialVersion,
storageVersion: &initialVersion,
settingsVersion: initialVersion,
checkVersions: map[clusterversion.Key]bool{
initialVersion: true,
clusterversion.V23_1Start: false,
maxVersion: false,
initialVersion: true,
startVersion: false,
maxVersion: false,
},
},
{
name: "mid-finalize",
storageVersion: clusterversion.V23_1Start,
storageVersion: &startVersion,
settingsVersion: initialVersion,
checkVersions: map[clusterversion.Key]bool{
initialVersion: true,
clusterversion.V23_1Start: true,
maxVersion: false,
initialVersion: true,
startVersion: true,
maxVersion: false,
},
},
{
name: "finalized",
storageVersion: maxVersion,
storageVersion: &maxVersion,
settingsVersion: maxVersion,
checkVersions: map[clusterversion.Key]bool{
initialVersion: true,
clusterversion.V23_1Start: true,
maxVersion: true,
initialVersion: true,
startVersion: true,
maxVersion: true,
},
},
{
Expand All @@ -80,7 +102,7 @@ func TestVersionGuard(t *testing.T) {
// version behind the settings version should not exist in
// production.
name: "verify-optimization",
storageVersion: initialVersion,
storageVersion: &initialVersion,
settingsVersion: maxVersion,
checkVersions: map[clusterversion.Key]bool{
initialVersion: true,
Expand All @@ -101,14 +123,18 @@ func TestVersionGuard(t *testing.T) {
settingVersion := clusterversion.ClusterVersion{Version: clusterversion.ByKey(test.settingsVersion)}
require.NoError(t, settings.Version.SetActiveVersion(ctx, settingVersion))

storageVersion := clusterversion.ClusterVersion{Version: clusterversion.ByKey(test.storageVersion)}
marshaledVersion, err := protoutil.Marshal(&storageVersion)
if test.storageVersion == nil {
tDB.Exec(t, `DELETE FROM system.settings WHERE name = 'version'`)
} else {
storageVersion := clusterversion.ClusterVersion{Version: clusterversion.ByKey(*test.storageVersion)}
marshaledVersion, err := protoutil.Marshal(&storageVersion)
require.NoError(t, err)
tDB.Exec(t, `
UPSERT INTO system.settings (name, value, "lastUpdated", "valueType")
VALUES ('version', $1, now(), 'm')`,
marshaledVersion)

require.NoError(t, err)
tDB.Exec(t, `
UPDATE system.settings
SET value = $1
WHERE name = 'version'`, marshaledVersion)
}

watcher := settingswatcher.New(nil, s.Codec(), settings, nil, nil, nil)
require.NoError(t, kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
Expand Down

0 comments on commit 506a3ef

Please sign in to comment.