From c7758193725bd06fda19c862bce1520a107be6ad Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 11 May 2020 15:38:45 -0700 Subject: [PATCH 1/2] channeldb/db: adds optional dry-run abort for migrations --- channeldb/db.go | 21 +++++++++++++++++- channeldb/db_test.go | 10 +++++++++ channeldb/meta_test.go | 48 +++++++++++++++++++++++++++++++++++++++--- channeldb/options.go | 12 +++++++++++ 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/channeldb/db.go b/channeldb/db.go index c2842b181..43ad6cde4 100644 --- a/channeldb/db.go +++ b/channeldb/db.go @@ -25,6 +25,12 @@ const ( dbFilePermission = 0600 ) +var ( + // ErrDryRunMigrationOK signals that a migration executed successful, + // but we intentionally did not commit the result. + ErrDryRunMigrationOK = errors.New("Dry run migration successful") +) + // migration is a function which takes a prior outdated version of the database // instances and mutates the key/bucket structure to arrive at a more // up-to-date version of the database. @@ -145,6 +151,7 @@ type DB struct { dbPath string graph *ChannelGraph clock clock.Clock + dryRun bool } // Open opens an existing channeldb. Any necessary schemas migrations due to @@ -174,6 +181,7 @@ func Open(dbPath string, modifiers ...OptionModifier) (*DB, error) { Backend: bdb, dbPath: dbPath, clock: opts.clock, + dryRun: opts.dryRun, } chanDB.graph = newChannelGraph( chanDB, opts.RejectCacheSize, opts.ChannelCacheSize, @@ -1227,7 +1235,18 @@ func (d *DB) syncVersions(versions []version) error { } meta.DbVersionNumber = latestVersion - return putMeta(meta, tx) + err := putMeta(meta, tx) + if err != nil { + return err + } + + // In dry-run mode, return an error to prevent the transaction + // from committing. + if d.dryRun { + return ErrDryRunMigrationOK + } + + return nil }) } diff --git a/channeldb/db_test.go b/channeldb/db_test.go index e678d2a50..242b70ca4 100644 --- a/channeldb/db_test.go +++ b/channeldb/db_test.go @@ -45,6 +45,16 @@ func TestOpenWithCreate(t *testing.T) { if !fileExists(dbPath) { t.Fatalf("channeldb failed to create data directory") } + + // Now, reopen the same db in dry run migration mode. Since we have not + // applied any migrations, this should ignore the flag and not fail. + cdb, err = Open(dbPath, OptionDryRunMigration(true)) + if err != nil { + t.Fatalf("unable to create channeldb: %v", err) + } + if err := cdb.Close(); err != nil { + t.Fatalf("unable to close channeldb: %v", err) + } } // TestWipe tests that the database wipe operation completes successfully diff --git a/channeldb/meta_test.go b/channeldb/meta_test.go index 1933c0e16..a18932f47 100644 --- a/channeldb/meta_test.go +++ b/channeldb/meta_test.go @@ -12,13 +12,14 @@ import ( // applyMigration is a helper test function that encapsulates the general steps // which are needed to properly check the result of applying migration function. func applyMigration(t *testing.T, beforeMigration, afterMigration func(d *DB), - migrationFunc migration, shouldFail bool) { + migrationFunc migration, shouldFail bool, dryRun bool) { cdb, cleanUp, err := makeTestDB() defer cleanUp() if err != nil { t.Fatal(err) } + cdb.dryRun = dryRun // Create a test node that will be our source node. testNode, err := createTestVertex(cdb) @@ -54,6 +55,9 @@ func applyMigration(t *testing.T, beforeMigration, afterMigration func(d *DB), defer func() { if r := recover(); r != nil { + if dryRun && r != ErrDryRunMigrationOK { + t.Fatalf("expected dry run migration OK") + } err = errors.New(r) } @@ -256,7 +260,8 @@ func TestMigrationWithPanic(t *testing.T) { beforeMigrationFunc, afterMigrationFunc, migrationWithPanic, - true) + true, + false) } // TestMigrationWithFatal asserts that migrations which fail do not modify the @@ -330,7 +335,8 @@ func TestMigrationWithFatal(t *testing.T) { beforeMigrationFunc, afterMigrationFunc, migrationWithFatal, - true) + true, + false) } // TestMigrationWithoutErrors asserts that a successful migration has its @@ -404,6 +410,7 @@ func TestMigrationWithoutErrors(t *testing.T) { beforeMigrationFunc, afterMigrationFunc, migrationWithoutErrors, + false, false) } @@ -446,3 +453,38 @@ func TestMigrationReversion(t *testing.T) { "want: %v, got: %v", ErrDBReversion, err) } } + +// TestMigrationDryRun ensures that opening the database in dry run migration +// mode will fail and not commit the migration. +func TestMigrationDryRun(t *testing.T) { + t.Parallel() + + // Nothing to do, will inspect version number. + beforeMigrationFunc := func(d *DB) {} + + // Check that version of database version is not modified. + afterMigrationFunc := func(d *DB) { + err := kvdb.View(d, func(tx kvdb.ReadTx) error { + meta, err := d.FetchMeta(nil) + if err != nil { + t.Fatal(err) + } + + if meta.DbVersionNumber != 0 { + t.Fatal("dry run migration was not aborted") + } + + return nil + }) + if err != nil { + t.Fatalf("unable to apply after func: %v", err) + } + } + + applyMigration(t, + beforeMigrationFunc, + afterMigrationFunc, + func(kvdb.RwTx) error { return nil }, + true, + true) +} diff --git a/channeldb/options.go b/channeldb/options.go index 90185f2cd..b84dd199d 100644 --- a/channeldb/options.go +++ b/channeldb/options.go @@ -31,6 +31,10 @@ type Options struct { // clock is the time source used by the database. clock clock.Clock + + // dryRun will fail to commit a successful migration when opening the + // database if set to true. + dryRun bool } // DefaultOptions returns an Options populated with default values. @@ -73,3 +77,11 @@ func OptionClock(clock clock.Clock) OptionModifier { o.clock = clock } } + +// OptionDryRunMigration controls whether or not to intentially fail to commit a +// successful migration that occurs when opening the database. +func OptionDryRunMigration(dryRun bool) OptionModifier { + return func(o *Options) { + o.dryRun = dryRun + } +} From 4f6cef83fccb9673ca4f686572d5436ad05255e5 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 11 May 2020 15:38:56 -0700 Subject: [PATCH 2/2] config: adds cli configurable --migration-dry-run --- config.go | 2 ++ lnd.go | 11 ++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/config.go b/config.go index 02aff622f..bc599a160 100644 --- a/config.go +++ b/config.go @@ -340,6 +340,8 @@ type config struct { MaxChannelFeeAllocation float64 `long:"max-channel-fee-allocation" description:"The maximum percentage of total funds that can be allocated to a channel's commitment fee. This only applies for the initiator of the channel. Valid values are within [0.1, 1]."` + DryRunMigration bool `long:"dry-run-migration" description:"If true, lnd will abort committing a migration if it would otherwise have been successful. This leaves the database unmodified, and still compatible with the previously active version of lnd."` + net tor.Net EnableUpfrontShutdown bool `long:"enable-upfront-shutdown" description:"If true, option upfront shutdown script will be enabled. If peers that we open channels with support this feature, we will automatically set the script to which cooperative closes should be paid out to on channel open. This offers the partial protection of a channel peer disconnecting from us if cooperative close is attempted with a different script."` diff --git a/lnd.go b/lnd.go index 98603c39b..291a33cff 100644 --- a/lnd.go +++ b/lnd.go @@ -235,10 +235,15 @@ func Main(lisCfg ListenerCfg) error { channeldb.OptionSetRejectCacheSize(cfg.Caches.RejectCacheSize), channeldb.OptionSetChannelCacheSize(cfg.Caches.ChannelCacheSize), channeldb.OptionSetSyncFreelist(cfg.SyncFreelist), + channeldb.OptionDryRunMigration(cfg.DryRunMigration), ) - if err != nil { - err := fmt.Errorf("unable to open channeldb: %v", err) - ltndLog.Error(err) + switch { + case err == channeldb.ErrDryRunMigrationOK: + ltndLog.Info("%v, exiting", err) + return nil + + case err != nil: + ltndLog.Errorf("Unable to open channeldb: %v", err) return err } defer chanDB.Close()