From c7e773132b1a68263ca0d8694679aeb8e8d7f936 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 17 Jun 2025 20:39:12 +0200 Subject: [PATCH] channeldb: update optional migration code for multiple versions --- channeldb/db.go | 88 +++++++++++++++++++++++++----------------- channeldb/meta.go | 52 +++++++++++++++++++++---- channeldb/meta_test.go | 56 +++++++++++++++++---------- channeldb/options.go | 19 ++++++--- 4 files changed, 147 insertions(+), 68 deletions(-) diff --git a/channeldb/db.go b/channeldb/db.go index 63915bc1b..412f4eaae 100644 --- a/channeldb/db.go +++ b/channeldb/db.go @@ -308,7 +308,7 @@ var ( // to determine its state. optionalVersions = []optionalVersion{ { - name: "prune revocation log", + name: "prune_revocation_log", migration: func(db kvdb.Backend, cfg MigrationConfig) error { @@ -1751,50 +1751,68 @@ func (d *DB) applyOptionalVersions(cfg OptionalMiragtionConfig) error { Versions: make(map[uint64]string), } } else { - return err + return fmt.Errorf("unable to fetch optional "+ + "meta: %w", err) } } - log.Infof("Checking for optional update: prune_revocation_log=%v, "+ - "db_version=%s", cfg.PruneRevocationLog, om) - - // Exit early if the optional migration is not specified. - if !cfg.PruneRevocationLog { - return nil - } - - // Exit early if the optional migration has already been applied. - if _, ok := om.Versions[0]; ok { - return nil - } - - // Get the optional version. - version := optionalVersions[0] - log.Infof("Performing database optional migration: %s", version.name) - + // migrationCfg is the parent configuration which implements the config + // interfaces of all the single optional migrations. migrationCfg := &MigrationConfigImpl{ migration30.MigrateRevLogConfigImpl{ NoAmountData: d.noRevLogAmtData, }, } - // Migrate the data. - if err := version.migration(d, migrationCfg); err != nil { - log.Errorf("Unable to apply optional migration: %s, error: %v", - version.name, err) - return err - } + log.Infof("Applying %d optional migrations", len(optionalVersions)) - // Update the optional meta. Notice that unlike the mandatory db - // migrations where we perform the migration and updating meta in a - // single db transaction, we use different transactions here. Even when - // the following update is failed, we should be fine here as we would - // re-run the optional migration again, which is a noop, during next - // startup. - om.Versions[0] = version.name - if err := d.putOptionalMeta(om); err != nil { - log.Errorf("Unable to update optional meta: %v", err) - return err + // Apply the optional migrations if requested. + for number, version := range optionalVersions { + log.Infof("Checking for optional update: name=%v", version.name) + + // Exit early if the optional migration is not specified. + if !cfg.MigrationFlags[number] { + log.Debugf("Skipping optional migration: name=%s as "+ + "it is not specified in the config", + version.name) + + continue + } + + // Exit early if the optional migration has already been + // applied. + if _, ok := om.Versions[uint64(number)]; ok { + log.Debugf("Skipping optional migration: name=%s as "+ + "it has already been applied", version.name) + + continue + } + + log.Infof("Performing database optional migration: %s", + version.name) + + // Call the migration function for the specific optional + // migration. + if err := version.migration(d, migrationCfg); err != nil { + log.Errorf("Unable to apply optional migration: %s, "+ + "error: %v", version.name, err) + return err + } + + // Update the optional meta. Notice that unlike the mandatory db + // migrations where we perform the migration and updating meta + // in a single db transaction, we use different transactions + // here. Even when the following update is failed, we should be + // fine here as we would re-run the optional migration again, + // which is a noop, during next startup. + om.Versions[uint64(number)] = version.name + if err := d.putOptionalMeta(om); err != nil { + log.Errorf("Unable to update optional meta: %v", err) + return err + } + + log.Infof("Successfully applied optional migration: %s", + version.name) } return nil diff --git a/channeldb/meta.go b/channeldb/meta.go index a32ab81ed..127acf51f 100644 --- a/channeldb/meta.go +++ b/channeldb/meta.go @@ -4,6 +4,8 @@ import ( "bytes" "errors" "fmt" + "slices" + "strings" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/tlv" @@ -30,6 +32,10 @@ var ( // ErrMarkerNotPresent is the error that is returned if the queried // marker is not present in the given database. ErrMarkerNotPresent = errors.New("marker not present") + + // ErrInvalidOptionalVersion is the error that is returned if the + // optional version persisted in the database is invalid. + ErrInvalidOptionalVersion = errors.New("invalid optional version") ) // Meta structure holds the database meta information. @@ -104,15 +110,28 @@ type OptionalMeta struct { Versions map[uint64]string } +// String returns a string representation of the optional meta. func (om *OptionalMeta) String() string { - s := "" - for index, name := range om.Versions { - s += fmt.Sprintf("%d: %s", index, name) + if len(om.Versions) == 0 { + return "empty" } - if s == "" { - s = "empty" + + // Create a slice of indices to sort + indices := make([]uint64, 0, len(om.Versions)) + for index := range om.Versions { + indices = append(indices, index) } - return s + + // Sort the indices in ascending order. + slices.Sort(indices) + + // Create the string parts in sorted order. + parts := make([]string, len(indices)) + for i, index := range indices { + parts[i] = fmt.Sprintf("%d: %s", index, om.Versions[index]) + } + + return strings.Join(parts, ", ") } // fetchOptionalMeta reads the optional meta from the database. @@ -146,7 +165,20 @@ func (d *DB) fetchOptionalMeta() (*OptionalMeta, error) { if err != nil { return err } - om.Versions[version] = optionalVersions[i].name + + // This check would not allow to downgrade LND software + // to a version with an optional migration when an + // optional migration not known to the current version + // has already been applied. + if version >= uint64(len(optionalVersions)) { + return fmt.Errorf("optional version read "+ + "from db is %d, but only optional "+ + "migrations up to %d are known: %w", + version, len(optionalVersions)-1, + ErrInvalidOptionalVersion) + } + + om.Versions[version] = optionalVersions[version].name } return nil @@ -174,8 +206,12 @@ func (d *DB) putOptionalMeta(om *OptionalMeta) error { return err } - // Write the version indexes. + // Write the version indexes of the single migrations. for v := range om.Versions { + if v >= uint64(len(optionalVersions)) { + return ErrInvalidOptionalVersion + } + err := tlv.WriteVarInt(&b, v, &[8]byte{}) if err != nil { return err diff --git a/channeldb/meta_test.go b/channeldb/meta_test.go index 97f6f0489..563deeee2 100644 --- a/channeldb/meta_test.go +++ b/channeldb/meta_test.go @@ -506,29 +506,37 @@ func TestOptionalMeta(t *testing.T) { om1, err := db.fetchOptionalMeta() require.NoError(t, err, "error getting optional meta") require.Equal(t, om, om1, "unexpected empty versions") - require.Equal(t, "0: prune revocation log", om.String()) + require.Equal(t, "0: prune_revocation_log", om.String()) } // TestApplyOptionalVersions checks that the optional migration is applied as // expected based on the config. +// +// NOTE: Cannot be run in parallel because we alter the optionalVersions +// global variable which could be used by other tests. func TestApplyOptionalVersions(t *testing.T) { - t.Parallel() - db, err := MakeTestDB(t) require.NoError(t, err) - // Overwrite the migration function so we can count how many times the - // migration has happened. - migrateCount := 0 - optionalVersions[0].migration = func(_ kvdb.Backend, - _ MigrationConfig) error { + // migrateCount is the number of migrations that have been run. It + // counts the number of times a migration function is called. + var migrateCount int - migrateCount++ - return nil + // Modify all migrations to track their execution. + for i := range optionalVersions { + optionalVersions[i].migration = func(_ kvdb.Backend, + _ MigrationConfig) error { + + migrateCount++ + + return nil + } } - // Test that when the flag is false, no migration happens. - cfg := OptionalMiragtionConfig{} + // All migrations are disabled by default. + cfg := NewOptionalMiragtionConfig() + + // Run the optional migrations. err = db.applyOptionalVersions(cfg) require.NoError(t, err, "failed to apply optional migration") require.Equal(t, 0, migrateCount, "expected no migration") @@ -536,13 +544,18 @@ func TestApplyOptionalVersions(t *testing.T) { // Check the optional meta is not updated. om, err := db.fetchOptionalMeta() require.NoError(t, err, "error getting optional meta") - require.Empty(t, om.Versions, "expected empty versions") - // Test that when specified, the optional migration is applied. - cfg.PruneRevocationLog = true + // Enable all optional migrations. + for i := range cfg.MigrationFlags { + cfg.MigrationFlags[i] = true + } + err = db.applyOptionalVersions(cfg) require.NoError(t, err, "failed to apply optional migration") - require.Equal(t, 1, migrateCount, "expected migration") + require.Equal( + t, len(optionalVersions), migrateCount, + "expected all migrations to be run", + ) // Fetch the updated optional meta. om, err = db.fetchOptionalMeta() @@ -556,12 +569,15 @@ func TestApplyOptionalVersions(t *testing.T) { } require.Equal(t, omExpected, om, "unexpected empty versions") - // Test that though specified, the optional migration is not run since - // it's already been applied. - cfg.PruneRevocationLog = true + // We make sure running the migrations again does not call the + // migrations again because the meta data should signal that they have + // already been run. err = db.applyOptionalVersions(cfg) require.NoError(t, err, "failed to apply optional migration") - require.Equal(t, 1, migrateCount, "expected no migration") + require.Equal( + t, len(optionalVersions), migrateCount, + "expected all migrations to be run", + ) } // TestFetchMeta tests that the FetchMeta returns the latest DB version for a diff --git a/channeldb/options.go b/channeldb/options.go index 6e631e2cb..8c3f9c734 100644 --- a/channeldb/options.go +++ b/channeldb/options.go @@ -25,9 +25,18 @@ const ( // OptionalMiragtionConfig defines the flags used to signal whether a // particular migration needs to be applied. type OptionalMiragtionConfig struct { - // PruneRevocationLog specifies that the revocation log migration needs - // to be applied. - PruneRevocationLog bool + // MigrationFlags is an array of booleans indicating which optional + // migrations should be run. The index in the array corresponds to the + // migration number in optionalVersions. + MigrationFlags []bool +} + +// NewOptionalMiragtionConfig creates a new OptionalMiragtionConfig with the +// default migration flags. +func NewOptionalMiragtionConfig() OptionalMiragtionConfig { + return OptionalMiragtionConfig{ + MigrationFlags: make([]bool, len(optionalVersions)), + } } // Options holds parameters for tuning and customizing a channeldb.DB. @@ -62,7 +71,7 @@ type Options struct { // DefaultOptions returns an Options populated with default values. func DefaultOptions() Options { return Options{ - OptionalMiragtionConfig: OptionalMiragtionConfig{}, + OptionalMiragtionConfig: NewOptionalMiragtionConfig(), NoMigration: false, clock: clock.NewDefaultClock(), } @@ -124,6 +133,6 @@ func OptionStoreFinalHtlcResolutions( // revocation logs needs to be applied or not. func OptionPruneRevocationLog(prune bool) OptionModifier { return func(o *Options) { - o.OptionalMiragtionConfig.PruneRevocationLog = prune + o.OptionalMiragtionConfig.MigrationFlags[0] = prune } }