Merge pull request #10163 from ellemouton/graphMigFlipSwitch

multi: switch on graph SQL migration

Author: ellemouton

config_builder.go

@@ -70,6 +70,10 @@ const (
 	// invoiceMigration is the version of the migration that will be used to
 	// migrate invoices from the kvdb to the sql database.
 	invoiceMigration = 7
+
+	// graphMigration is the version number for the graph migration
+	// that migrates the KV graph to the native SQL schema.
+	graphMigration = 10
 )
 
 // GrpcRegistrar is an interface that must be satisfied by an external subserver
@@ -1091,6 +1095,11 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 	if d.cfg.DB.UseNativeSQL {
 		migrations := sqldb.GetMigrations()
 
+		queryCfg := &d.cfg.DB.Sqlite.QueryConfig
+		if d.cfg.DB.Backend == lncfg.PostgresBackend {
+			queryCfg = &d.cfg.DB.Postgres.QueryConfig
+		}
+
 		// If the user has not explicitly disabled the SQL invoice
 		// migration, attach the custom migration function to invoice
 		// migration (version 7). Even if this custom migration is
@@ -1115,17 +1124,42 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 				d.logger.Debugf("Setting invoice bucket " +
 					"tombstone")
 
-				return dbs.ChanStateDB.SetInvoiceBucketTombstone() //nolint:ll
+				//nolint:ll
+				return dbs.ChanStateDB.SetInvoiceBucketTombstone()
+			}
+
+			graphMig := func(tx *sqlc.Queries) error {
+				cfg := &graphdb.SQLStoreConfig{
+					//nolint:ll
+					ChainHash: *d.cfg.ActiveNetParams.GenesisHash,
+					QueryCfg:  queryCfg,
+				}
+				err := graphdb.MigrateGraphToSQL(
+					ctx, cfg, dbs.ChanStateDB.Backend, tx,
+				)
+				if err != nil {
+					return fmt.Errorf("failed to migrate "+
+						"graph to SQL: %w", err)
+				}
+
+				return nil
 			}
 
 			// Make sure we attach the custom migration function to
 			// the correct migration version.
 			for i := 0; i < len(migrations); i++ {
 				version := migrations[i].Version
-				if version == invoiceMigration {
+				switch version {
+				case invoiceMigration:
 					migrations[i].MigrationFn = invoiceMig
 
 					continue
+				case graphMigration:
+					migrations[i].MigrationFn = graphMig
+
+					continue
+
+				default:
 				}
 
 				migFn, ok := d.getSQLMigration(
@@ -1167,8 +1201,18 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 
 		dbs.InvoiceDB = sqlInvoiceDB
 
-		graphStore, err = d.getGraphStore(
-			baseDB, databaseBackends.GraphDB, graphDBOptions...,
+		graphExecutor := sqldb.NewTransactionExecutor(
+			baseDB, func(tx *sql.Tx) graphdb.SQLQueries {
+				return baseDB.WithTx(tx)
+			},
+		)
+
+		graphStore, err = graphdb.NewSQLStore(
+			&graphdb.SQLStoreConfig{
+				ChainHash: *d.cfg.ActiveNetParams.GenesisHash,
+				QueryCfg:  queryCfg,
+			},
+			graphExecutor, graphDBOptions...,
 		)
 		if err != nil {
 			err = fmt.Errorf("unable to get graph store: %w", err)

config_prod.go

@@ -5,25 +5,14 @@ package lnd
 import (
 	"context"
 
-	graphdb "github.com/lightningnetwork/lnd/graph/db"
 	"github.com/lightningnetwork/lnd/kvdb"
-	"github.com/lightningnetwork/lnd/sqldb"
 	"github.com/lightningnetwork/lnd/sqldb/sqlc"
 )
 
 // RunTestSQLMigration is a build tag that indicates whether the test_native_sql
 // build tag is set.
 var RunTestSQLMigration = false
 
-// getGraphStore returns a graphdb.V1Store backed by a graphdb.KVStore
-// implementation.
-func (d *DefaultDatabaseBuilder) getGraphStore(_ *sqldb.BaseDB,
-	kvBackend kvdb.Backend,
-	opts ...graphdb.StoreOptionModifier) (graphdb.V1Store, error) {
-
-	return graphdb.NewKVStore(kvBackend, opts...)
-}
-
 // getSQLMigration returns a migration function for the given version.
 //
 // NOTE: this is a no-op for the production build since all migrations that are

config_test_native_sql.go

@@ -4,79 +4,24 @@ package lnd
 
 import (
 	"context"
-	"database/sql"
-	"fmt"
 
-	graphdb "github.com/lightningnetwork/lnd/graph/db"
 	"github.com/lightningnetwork/lnd/kvdb"
-	"github.com/lightningnetwork/lnd/lncfg"
-	"github.com/lightningnetwork/lnd/sqldb"
 	"github.com/lightningnetwork/lnd/sqldb/sqlc"
 )
 
 // RunTestSQLMigration is a build tag that indicates whether the test_native_sql
 // build tag is set.
 var RunTestSQLMigration = true
 
-// getGraphStore returns a graphdb.V1Store backed by a graphdb.SQLStore
-// implementation.
-func (d *DefaultDatabaseBuilder) getGraphStore(baseDB *sqldb.BaseDB,
-	_ kvdb.Backend, opts ...graphdb.StoreOptionModifier) (graphdb.V1Store,
-	error) {
-
-	graphExecutor := sqldb.NewTransactionExecutor(
-		baseDB, func(tx *sql.Tx) graphdb.SQLQueries {
-			return baseDB.WithTx(tx)
-		},
-	)
-
-	queryConfig := d.cfg.DB.Sqlite.QueryConfig
-	if d.cfg.DB.Backend == lncfg.PostgresBackend {
-		queryConfig = d.cfg.DB.Postgres.QueryConfig
-	}
-
-	return graphdb.NewSQLStore(
-		&graphdb.SQLStoreConfig{
-			ChainHash: *d.cfg.ActiveNetParams.GenesisHash,
-			QueryCfg:  &queryConfig,
-		},
-		graphExecutor, opts...,
-	)
-}
-
-// graphSQLMigration is the version number for the graph migration
-// that migrates the KV graph to the native SQL schema.
-const graphSQLMigration = 10
-
 // getSQLMigration returns a migration function for the given version.
-func (d *DefaultDatabaseBuilder) getSQLMigration(ctx context.Context,
-	version int, kvBackend kvdb.Backend) (func(tx *sqlc.Queries) error,
+func (d *DefaultDatabaseBuilder) getSQLMigration(_ context.Context,
+	version int, _ kvdb.Backend) (func(tx *sqlc.Queries) error,
 	bool) {
 
-	cfg := &graphdb.SQLStoreConfig{
-		ChainHash: *d.cfg.ActiveNetParams.GenesisHash,
-		QueryCfg:  &d.cfg.DB.Sqlite.QueryConfig,
-	}
-	if d.cfg.DB.Backend == lncfg.PostgresBackend {
-		cfg.QueryCfg = &d.cfg.DB.Postgres.QueryConfig
-	}
-
 	switch version {
-	case graphSQLMigration:
-		return func(tx *sqlc.Queries) error {
-			err := graphdb.MigrateGraphToSQL(
-				ctx, cfg, kvBackend, tx,
-			)
-			if err != nil {
-				return fmt.Errorf("failed to migrate graph "+
-					"to SQL: %w", err)
-			}
-
-			return nil
-		}, true
+	default:
+		// No version was matched, so we return false to indicate that
+		// no migration is known for the given version.
+		return nil, false
 	}
-
-	// No version was matched, so we return false to indicate that no
-	// migration is known for the given version.
-	return nil, false
 }

docs/release-notes/release-notes-0.20.0.md

@@ -248,6 +248,11 @@ reader of a payment request.
 * Add missing [sql index](https://github.com/lightningnetwork/lnd/pull/10155)
   for settled invoices to increase query speed.
 
+* [Migrate the KV graph store to native 
+  SQL](https://github.com/lightningnetwork/lnd/pull/10163). For this migration 
+  to take place, the db backend must already be either `postgres` or `sqlite` 
+  and the `--use-native-sql` flag must be set.
+
 ## Code Health
 
 ## Tooling and Documentation

itest/lnd_graph_migration_test.go

@@ -5,7 +5,6 @@ import (
 	"database/sql"
 	"net"
 
-	"github.com/lightningnetwork/lnd"
 	graphdb "github.com/lightningnetwork/lnd/graph/db"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/node"
@@ -17,10 +16,6 @@ import (
 // testGraphMigration tests that the graph migration from the old KV store to
 // the new native SQL store works as expected.
 func testGraphMigration(ht *lntest.HarnessTest) {
-	if !lnd.RunTestSQLMigration {
-		ht.Skip("not running with test_native_sql tag")
-	}
-
 	ctx := ht.Context()
 	alice := ht.NewNodeWithCoins("Alice", nil)
 

sqldb/migrations.go

@@ -78,6 +78,20 @@ var (
 			Version:       8,
 			SchemaVersion: 7,
 		},
+		{
+			Name:          "000008_graph",
+			Version:       9,
+			SchemaVersion: 8,
+		},
+		{
+			Name:          "kv_graph_migration",
+			Version:       10,
+			SchemaVersion: 8,
+			// A migration function may be attached to this
+			// migration to migrate KV graph to the native SQL
+			// schema. This is optional and can be disabled by the
+			// user if necessary.
+		},
 	}, migrationAdditions...)
 
 	// ErrMigrationMismatch is returned when a migrated record does not

sqldb/migrations_dev.go

@@ -2,19 +2,4 @@
 
 package sqldb
 
-var migrationAdditions = []MigrationConfig{
-	{
-		Name:          "000008_graph",
-		Version:       9,
-		SchemaVersion: 8,
-	},
-	{
-		Name:          "kv_graph_migration",
-		Version:       10,
-		SchemaVersion: 8,
-		// A migration function may be attached to this
-		// migration to migrate KV graph to the native SQL
-		// schema. This is optional and can be disabled by the
-		// user if necessary.
-	},
-}
+var migrationAdditions []MigrationConfig