diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 21b33b264..7ebc54037 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -455,6 +455,9 @@ the on going rate we'll permit. flag](https://github.com/lightningnetwork/lnd/pull/9606/) for future compatibility. +* [Establish a base DB version even if it is not yet + tracked](https://github.com/lightningnetwork/lnd/pull/9647). + ## Code Health * A code refactor that [moves all the graph related DB code out of the diff --git a/go.mod b/go.mod index d1351bca0..353afe02e 100644 --- a/go.mod +++ b/go.mod @@ -207,6 +207,10 @@ replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2 // allows us to specify that as an option. replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display +// Temporary replace until https://github.com/lightningnetwork/lnd/pull/9647 is +// merged. +replace github.com/lightningnetwork/lnd/sqldb => ./sqldb + // If you change this please also update docs/INSTALL.md and GO_VERSION in // Makefile (then run `make lint` to see where else it needs to be updated as // well). diff --git a/go.sum b/go.sum index a79d0f443..8f7aacf86 100644 --- a/go.sum +++ b/go.sum @@ -464,8 +464,6 @@ github.com/lightningnetwork/lnd/kvdb v1.4.12 h1:Y0WY5Tbjyjn6eCYh068qkWur5oFtioJl github.com/lightningnetwork/lnd/kvdb v1.4.12/go.mod h1:hx9buNcxsZpZwh8m1sjTQwy2SOeBoWWOZ3RnOQkMsxI= github.com/lightningnetwork/lnd/queue v1.1.1 h1:99ovBlpM9B0FRCGYJo6RSFDlt8/vOkQQZznVb18iNMI= github.com/lightningnetwork/lnd/queue v1.1.1/go.mod h1:7A6nC1Qrm32FHuhx/mi1cieAiBZo5O6l8IBIoQxvkz4= -github.com/lightningnetwork/lnd/sqldb v1.0.7 h1:wQ4DdHY++uwxwth2CHL7s+duGqmMLaoIRBOQCa9HPTk= -github.com/lightningnetwork/lnd/sqldb v1.0.7/go.mod h1:OG09zL/PHPaBJefp4HsPz2YLUJ+zIQHbpgCtLnOx8I4= github.com/lightningnetwork/lnd/ticker v1.1.1 h1:J/b6N2hibFtC7JLV77ULQp++QLtCwT6ijJlbdiZFbSM= github.com/lightningnetwork/lnd/ticker v1.1.1/go.mod h1:waPTRAAcwtu7Ji3+3k+u/xH5GHovTsCoSVpho0KDvdA= github.com/lightningnetwork/lnd/tlv v1.3.0 h1:exS/KCPEgpOgviIttfiXAPaUqw2rHQrnUOpP7HPBPiY= diff --git a/sqldb/migrations.go b/sqldb/migrations.go index f81436733..1eed29828 100644 --- a/sqldb/migrations.go +++ b/sqldb/migrations.go @@ -112,6 +112,15 @@ type MigrationExecutor interface { // order. Migration details are stored in the global variable // migrationConfig. ExecuteMigrations(target MigrationTarget) error + + // GetSchemaVersion returns the current schema version of the database. + GetSchemaVersion() (int, bool, error) + + // SetSchemaVersion sets the schema version of the database. + // + // NOTE: This alters the internal database schema tracker. USE WITH + // CAUTION!!! + SetSchemaVersion(version int, dirty bool) error } var ( @@ -360,6 +369,46 @@ func ApplyMigrations(ctx context.Context, db *BaseDB, } currentVersion = int(version) + } else { + // Since we don't have a version tracked by our own table yet, + // we'll use the schema version reported by sqlc to determine + // the current version. + // + // NOTE: This is safe because the first in-code migration was + // introduced in version 7. This is only possible if the user + // has a schema version <= 4. + var dirty bool + currentVersion, dirty, err = migrator.GetSchemaVersion() + if err != nil { + return err + } + + log.Infof("No database version found, using schema version %d "+ + "(dirty=%v) as base version", currentVersion, dirty) + } + + // Due to an a migration issue in v0.19.0-rc1 we may be at version 2 and + // have a dirty schema due to failing migration 3. If this is indeed the + // case, we need to reset the dirty flag to be able to apply the fixed + // migration. + // NOTE: this could be removed as soon as we drop v0.19.0-beta. + if version == 2 { + schemaVersion, dirty, err := migrator.GetSchemaVersion() + if err != nil { + return err + } + + if schemaVersion == 3 && dirty { + log.Warnf("Schema version %d is dirty. This is "+ + "likely a consequence of a failed migration "+ + "in v0.19.0-rc1. Attempting to recover by "+ + "resetting the dirty flag", schemaVersion) + + err = migrator.SetSchemaVersion(4, false) + if err != nil { + return err + } + } } for _, migration := range migrations { diff --git a/sqldb/migrations_test.go b/sqldb/migrations_test.go index 385840364..304a61189 100644 --- a/sqldb/migrations_test.go +++ b/sqldb/migrations_test.go @@ -452,3 +452,294 @@ func TestCustomMigration(t *testing.T) { }) } } + +// TestSchemaMigrationIdempotency tests that the our schema migrations are +// idempotent. This means that we can apply the migrations multiple times and +// the schema version will always be the same. +func TestSchemaMigrationIdempotency(t *testing.T) { + dropMigrationTrackerEntries := func(t *testing.T, db *BaseDB) { + _, err := db.Exec("DELETE FROM migration_tracker;") + require.NoError(t, err) + } + + lastMigration := migrationConfig[len(migrationConfig)-1] + + t.Run("SQLite", func(t *testing.T) { + // First instantiate the database and run the migrations + // including the custom migrations. + t.Logf("Creating new SQLite DB for testing migrations") + + dbFileName := filepath.Join(t.TempDir(), "tmp.db") + var ( + db *SqliteStore + err error + ) + + // Run the migration 3 times to test that the migrations + // are idempotent. + for i := 0; i < 3; i++ { + db, err = NewSqliteStore(&SqliteConfig{ + SkipMigrations: false, + }, dbFileName) + require.NoError(t, err) + + dbToCleanup := db.DB + t.Cleanup(func() { + require.NoError( + t, dbToCleanup.Close(), + ) + }) + + ctxb := context.Background() + require.NoError( + t, db.ApplyAllMigrations(ctxb, GetMigrations()), + ) + + version, dirty, err := db.GetSchemaVersion() + require.NoError(t, err) + + // Now reset the schema version to 0 and make sure that + // we can apply the migrations again. + require.Equal(t, lastMigration.SchemaVersion, version) + require.False(t, dirty) + + require.NoError( + t, db.SetSchemaVersion( + database.NilVersion, false, + ), + ) + dropMigrationTrackerEntries(t, db.BaseDB) + + // Make sure that we reset the schema version. + version, dirty, err = db.GetSchemaVersion() + require.NoError(t, err) + require.Equal(t, -1, version) + require.False(t, dirty) + } + }) + + t.Run("Postgres", func(t *testing.T) { + // First create a temporary Postgres database to run + // the migrations on. + fixture := NewTestPgFixture( + t, DefaultPostgresFixtureLifetime, + ) + t.Cleanup(func() { + fixture.TearDown(t) + }) + + dbName := randomDBName(t) + + // Next instantiate the database and run the migrations + // including the custom migrations. + t.Logf("Creating new Postgres DB '%s' for testing "+ + "migrations", dbName) + + _, err := fixture.db.ExecContext( + context.Background(), "CREATE DATABASE "+dbName, + ) + require.NoError(t, err) + + cfg := fixture.GetConfig(dbName) + var db *PostgresStore + + // Run the migration 3 times to test that the migrations + // are idempotent. + for i := 0; i < 3; i++ { + cfg.SkipMigrations = false + db, err = NewPostgresStore(cfg) + require.NoError(t, err) + + ctxb := context.Background() + require.NoError( + t, db.ApplyAllMigrations(ctxb, GetMigrations()), + ) + + version, dirty, err := db.GetSchemaVersion() + require.NoError(t, err) + + // Now reset the schema version to 0 and make sure that + // we can apply the migrations again. + require.Equal(t, lastMigration.SchemaVersion, version) + require.False(t, dirty) + + require.NoError( + t, db.SetSchemaVersion( + database.NilVersion, false, + ), + ) + dropMigrationTrackerEntries(t, db.BaseDB) + + // Make sure that we reset the schema version. + version, dirty, err = db.GetSchemaVersion() + require.NoError(t, err) + require.Equal(t, -1, version) + require.False(t, dirty) + } + }) +} + +// TestMigrationBug19RC1 tests a bug that was present in the migration code +// at the v0.19.0-rc1 release. +// The bug was fixed in: https://github.com/lightningnetwork/lnd/pull/9647 +// NOTE: This test may be removed once the final version of 0.19.0 is released. +func TestMigrationSucceedsAfterDirtyStateMigrationFailure19RC1(t *testing.T) { + // setMigrationTrackerVersion is a helper function that + // updates the migration tracker table to a specific version that + // simulates the conditions of the bug. + setMigrationTrackerVersion := func(t *testing.T, db *BaseDB) { + _, err := db.Exec(` + DELETE FROM migration_tracker; + INSERT INTO migration_tracker (version, migration_time) + VALUES (2, CURRENT_TIMESTAMP); + `) + require.NoError(t, err) + } + + const ( + maxSchemaVersionBefore19RC1 = 4 + failingSchemaVersion = 3 + ) + + ctxb := context.Background() + migrations := GetMigrations() + migrations = migrations[:maxSchemaVersionBefore19RC1] + lastMigration := migrations[len(migrations)-1] + + // Make sure that the last migration is the one we expect. + require.Equal( + t, maxSchemaVersionBefore19RC1, lastMigration.SchemaVersion, + ) + + t.Run("SQLite", func(t *testing.T) { + // First instantiate the database and run the migrations + // including the custom migrations. + t.Logf("Creating new SQLite DB for testing migrations") + + dbFileName := filepath.Join(t.TempDir(), "tmp.db") + var ( + db *SqliteStore + err error + ) + + db, err = NewSqliteStore(&SqliteConfig{ + SkipMigrations: false, + }, dbFileName) + require.NoError(t, err) + + dbToCleanup := db.DB + t.Cleanup(func() { + require.NoError(t, dbToCleanup.Close()) + }) + + require.NoError(t, db.ApplyAllMigrations(ctxb, migrations)) + + version, dirty, err := db.GetSchemaVersion() + require.NoError(t, err) + + // Now reset the schema version to 0 and make sure that + // we can apply the migrations again. + require.Equal(t, lastMigration.SchemaVersion, version) + require.False(t, dirty) + + // Set the schema version to the failing version and + // make make the version dirty which essentially tells + // golang-migrate that the migration failed. + require.NoError( + t, db.SetSchemaVersion(failingSchemaVersion, true), + ) + + // Set the migration tracker to the failing version. + setMigrationTrackerVersion(t, db.BaseDB) + + // Close the DB so we can reopen it and apply the + // migrations again. + db.DB.Close() + + db, err = NewSqliteStore(&SqliteConfig{ + SkipMigrations: false, + }, dbFileName) + require.NoError(t, err) + + dbToCleanup2 := db.DB + t.Cleanup(func() { + require.NoError(t, dbToCleanup2.Close()) + }) + + require.NoError(t, db.ApplyAllMigrations(ctxb, migrations)) + + version, dirty, err = db.GetSchemaVersion() + require.NoError(t, err) + require.Equal(t, lastMigration.SchemaVersion, version) + require.False(t, dirty) + }) + + t.Run("Postgres", func(t *testing.T) { + // First create a temporary Postgres database to run + // the migrations on. + fixture := NewTestPgFixture( + t, DefaultPostgresFixtureLifetime, + ) + t.Cleanup(func() { + fixture.TearDown(t) + }) + + dbName := randomDBName(t) + + // Next instantiate the database and run the migrations + // including the custom migrations. + t.Logf("Creating new Postgres DB '%s' for testing "+ + "migrations", dbName) + + _, err := fixture.db.ExecContext( + context.Background(), "CREATE DATABASE "+dbName, + ) + require.NoError(t, err) + + cfg := fixture.GetConfig(dbName) + var db *PostgresStore + + cfg.SkipMigrations = false + db, err = NewPostgresStore(cfg) + require.NoError(t, err) + + require.NoError(t, db.ApplyAllMigrations(ctxb, migrations)) + + version, dirty, err := db.GetSchemaVersion() + require.NoError(t, err) + + // Now reset the schema version to 0 and make sure that + // we can apply the migrations again. + require.Equal(t, lastMigration.SchemaVersion, version) + require.False(t, dirty) + + // Set the schema version to the failing version and + // make make the version dirty which essentially tells + // golang-migrate that the migration failed. + require.NoError( + t, db.SetSchemaVersion(failingSchemaVersion, true), + ) + + // Set the migration tracker to the failing version. + setMigrationTrackerVersion(t, db.BaseDB) + + // Close the DB so we can reopen it and apply the + // migrations again. + db.DB.Close() + + db, err = NewPostgresStore(cfg) + require.NoError(t, err) + + dbToCleanup2 := db.DB + t.Cleanup(func() { + require.NoError(t, dbToCleanup2.Close()) + }) + + require.NoError(t, db.ApplyAllMigrations(ctxb, migrations)) + + version, dirty, err = db.GetSchemaVersion() + require.NoError(t, err) + require.Equal(t, lastMigration.SchemaVersion, version) + require.False(t, dirty) + }) +} diff --git a/sqldb/postgres.go b/sqldb/postgres.go index d271d214e..8cfd3e25b 100644 --- a/sqldb/postgres.go +++ b/sqldb/postgres.go @@ -158,6 +158,10 @@ func (s *PostgresStore) ApplyAllMigrations(ctx context.Context, return ApplyMigrations(ctx, s.BaseDB, s, migrations) } +func errPostgresMigration(err error) error { + return fmt.Errorf("error creating postgres migration: %w", err) +} + // ExecuteMigrations runs migrations for the Postgres database, depending on the // target given, either all migrations or up to a given version. func (s *PostgresStore) ExecuteMigrations(target MigrationTarget) error { @@ -168,7 +172,7 @@ func (s *PostgresStore) ExecuteMigrations(target MigrationTarget) error { driver, err := pgx_migrate.WithInstance(s.DB, &pgx_migrate.Config{}) if err != nil { - return fmt.Errorf("error creating postgres migration: %w", err) + return errPostgresMigration(err) } // Populate the database with our set of schemas based on our embedded @@ -178,3 +182,31 @@ func (s *PostgresStore) ExecuteMigrations(target MigrationTarget) error { postgresFS, driver, "sqlc/migrations", dbName, target, ) } + +// GetSchemaVersion returns the current schema version of the Postgres database. +func (s *PostgresStore) GetSchemaVersion() (int, bool, error) { + driver, err := pgx_migrate.WithInstance(s.DB, &pgx_migrate.Config{}) + if err != nil { + return 0, false, errPostgresMigration(err) + + } + + version, dirty, err := driver.Version() + if err != nil { + return 0, false, err + } + + return version, dirty, nil +} + +// SetSchemaVersion sets the schema version of the Postgres database. +// +// NOTE: This alters the internal database schema tracker. USE WITH CAUTION!!! +func (s *PostgresStore) SetSchemaVersion(version int, dirty bool) error { + driver, err := pgx_migrate.WithInstance(s.DB, &pgx_migrate.Config{}) + if err != nil { + return errPostgresMigration(err) + } + + return driver.SetVersion(version, dirty) +} diff --git a/sqldb/sqlc/migrations/000001_invoices.up.sql b/sqldb/sqlc/migrations/000001_invoices.up.sql index 7f8c653d5..5dca73b34 100644 --- a/sqldb/sqlc/migrations/000001_invoices.up.sql +++ b/sqldb/sqlc/migrations/000001_invoices.up.sql @@ -1,12 +1,14 @@ -- sequences contains all sequences used for invoices. -CREATE TABLE invoice_sequences ( +CREATE TABLE IF NOT EXISTS invoice_sequences ( name TEXT PRIMARY KEY, current_value BIGINT NOT NULL ); -- Initialize a sequence for the settled index used to track invoice settlement -- to remain compatible with the legacy channeldb implementation. -INSERT INTO invoice_sequences(name, current_value) VALUES ('settle_index', 0); +INSERT INTO invoice_sequences (name, current_value) +VALUES ('settle_index', 0) + ON CONFLICT (name) DO NOTHING; -- invoices table contains all the information shared by all the invoice types. CREATE TABLE IF NOT EXISTS invoices ( diff --git a/sqldb/sqlc/migrations/000003_invoice_events.up.sql b/sqldb/sqlc/migrations/000003_invoice_events.up.sql index b9280a9d2..7409f149f 100644 --- a/sqldb/sqlc/migrations/000003_invoice_events.up.sql +++ b/sqldb/sqlc/migrations/000003_invoice_events.up.sql @@ -5,26 +5,31 @@ CREATE TABLE IF NOT EXISTS invoice_event_types( description TEXT NOT NULL ); + -- invoice_event_types defines the different types of invoice events. -INSERT INTO invoice_event_types (id, description) -VALUES +-- Insert events explicitly, checking their existence first. +INSERT INTO invoice_event_types (id, description) +SELECT id, description FROM ( -- invoice_created is the event type used when an invoice is created. - (0, 'invoice_created'), + SELECT 0 AS id, 'invoice_created' AS description UNION ALL -- invoice_canceled is the event type used when an invoice is canceled. - (1, 'invoice_canceled'), + SELECT 1, 'invoice_canceled' UNION ALL -- invoice_settled is the event type used when an invoice is settled. - (2, 'invoice_settled'), + SELECT 2, 'invoice_settled' UNION ALL -- setid_created is the event type used when an AMP sub invoice -- corresponding to the set_id is created. - (3, 'setid_created'), + SELECT 3, 'setid_created' UNION ALL -- setid_canceled is the event type used when an AMP sub invoice -- corresponding to the set_id is canceled. - (4, 'setid_canceled'), - -- setid_settled is the event type used when an AMP sub invoice + SELECT 4, 'setid_canceled' UNION ALL + -- setid_settled is the event type used when an AMP sub invoice -- corresponding to the set_id is settled. - (5, 'setid_settled'); - - + SELECT 5, 'setid_settled' +) AS new_values +WHERE NOT EXISTS ( + SELECT 1 FROM invoice_event_types + WHERE invoice_event_types.id = new_values.id +); -- invoice_events stores all major events related to the node's invoices and -- AMP sub invoices. This table can be used to create a historical view of what -- happened to the node's invoices. diff --git a/sqldb/sqlite.go b/sqldb/sqlite.go index 81e1f26b3..e6a291a17 100644 --- a/sqldb/sqlite.go +++ b/sqldb/sqlite.go @@ -158,6 +158,10 @@ func (s *SqliteStore) ApplyAllMigrations(ctx context.Context, return ApplyMigrations(ctx, s.BaseDB, s, migrations) } +func errSqliteMigration(err error) error { + return fmt.Errorf("error creating sqlite migration: %w", err) +} + // ExecuteMigrations runs migrations for the sqlite database, depending on the // target given, either all migrations or up to a given version. func (s *SqliteStore) ExecuteMigrations(target MigrationTarget) error { @@ -165,7 +169,7 @@ func (s *SqliteStore) ExecuteMigrations(target MigrationTarget) error { s.DB, &sqlite_migrate.Config{}, ) if err != nil { - return fmt.Errorf("error creating sqlite migration: %w", err) + return errSqliteMigration(err) } // Populate the database with our set of schemas based on our embedded @@ -176,6 +180,37 @@ func (s *SqliteStore) ExecuteMigrations(target MigrationTarget) error { ) } +// GetSchemaVersion returns the current schema version of the SQLite database. +func (s *SqliteStore) GetSchemaVersion() (int, bool, error) { + driver, err := sqlite_migrate.WithInstance( + s.DB, &sqlite_migrate.Config{}, + ) + if err != nil { + return 0, false, errSqliteMigration(err) + } + + version, dirty, err := driver.Version() + if err != nil { + return 0, dirty, err + } + + return version, dirty, nil +} + +// SetSchemaVersion sets the schema version of the SQLite database. +// +// NOTE: This alters the internal database schema tracker. USE WITH CAUTION!!! +func (s *SqliteStore) SetSchemaVersion(version int, dirty bool) error { + driver, err := sqlite_migrate.WithInstance( + s.DB, &sqlite_migrate.Config{}, + ) + if err != nil { + return errSqliteMigration(err) + } + + return driver.SetVersion(version, dirty) +} + // NewTestSqliteDB is a helper function that creates an SQLite database for // testing. func NewTestSqliteDB(t *testing.T) *SqliteStore {