From bd291286f73a4686f6d2e3880ee44152d2c03e36 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 27 Sep 2021 11:57:23 +0200 Subject: [PATCH 1/3] kvdb/postgres: convert all types of panic data to error Previously the assumption existed that panic data was already an error. If that wasn't the case for example because panic("string") was used, the transaction wasn't unlocked. --- kvdb/postgres/db.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/kvdb/postgres/db.go b/kvdb/postgres/db.go index 0db46b6df..3276fb557 100644 --- a/kvdb/postgres/db.go +++ b/kvdb/postgres/db.go @@ -151,8 +151,15 @@ func (db *db) getPrefixedTableName(table string) string { func catchPanic(f func() error) (err error) { defer func() { if r := recover(); r != nil { - err = r.(error) - log.Criticalf("Caught unhandled error: %v", err) + log.Criticalf("Caught unhandled error: %v", r) + + switch data := r.(type) { + case error: + err = data + + default: + err = errors.New(fmt.Sprintf("%v", data)) + } } }() From 7c048efa2157b9fb0930fa65264e3b00ca538b3e Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 27 Sep 2021 11:18:58 +0200 Subject: [PATCH 2/3] kvdb/postgres/test: single instance embedded postgres database Prepare for parallel tests that use a postgres backend. We don't want a high number of embedded postgres instances running simultaneously. --- kvdb/postgres/db_test.go | 22 +++--- kvdb/postgres/fixture.go | 149 ++++++++++++++++++++++++--------------- kvdb/postgres_test.go | 13 ++-- 3 files changed, 116 insertions(+), 68 deletions(-) diff --git a/kvdb/postgres/db_test.go b/kvdb/postgres/db_test.go index faf853dd2..dab6bf447 100644 --- a/kvdb/postgres/db_test.go +++ b/kvdb/postgres/db_test.go @@ -14,15 +14,19 @@ import ( // TestInterface performs all interfaces tests for this database driver. func TestInterface(t *testing.T) { - f := NewFixture(t) - defer f.Cleanup() + stop, err := StartEmbeddedPostgres() + require.NoError(t, err) + defer stop() + + f, err := NewFixture("") + require.NoError(t, err) // dbType is the database type name for this driver. const dbType = "postgres" ctx := context.Background() cfg := &Config{ - Dsn: testDsn, + Dsn: f.Dsn, } walletdbtest.TestInterface(t, dbType, ctx, cfg, prefix) @@ -30,17 +34,19 @@ func TestInterface(t *testing.T) { // TestPanic tests recovery from panic conditions. func TestPanic(t *testing.T) { - f := NewFixture(t) - defer f.Cleanup() + stop, err := StartEmbeddedPostgres() + require.NoError(t, err) + defer stop() - d := f.NewBackend() + f, err := NewFixture("") + require.NoError(t, err) - err := d.(*db).Update(func(tx walletdb.ReadWriteTx) error { + err = f.Db.(*db).Update(func(tx walletdb.ReadWriteTx) error { bucket, err := tx.CreateTopLevelBucket([]byte("test")) require.NoError(t, err) // Stop database server. - f.Cleanup() + stop() // Keep trying to get data until Get panics because the // connection is lost. diff --git a/kvdb/postgres/fixture.go b/kvdb/postgres/fixture.go index 8b3ecae73..929563220 100644 --- a/kvdb/postgres/fixture.go +++ b/kvdb/postgres/fixture.go @@ -4,87 +4,118 @@ package postgres import ( "context" + "crypto/rand" "database/sql" - "testing" + "encoding/hex" + "fmt" + "strings" "github.com/btcsuite/btcwallet/walletdb" embeddedpostgres "github.com/fergusstrange/embedded-postgres" - "github.com/stretchr/testify/require" ) const ( - testDsn = "postgres://postgres:postgres@localhost:9876/postgres?sslmode=disable" - prefix = "test" + testDsnTemplate = "postgres://postgres:postgres@localhost:9876/%v?sslmode=disable" + prefix = "test" ) -func clearTestDb(t *testing.T) { - dbConn, err := sql.Open("pgx", testDsn) - require.NoError(t, err) - - _, err = dbConn.ExecContext(context.Background(), "DROP SCHEMA IF EXISTS public CASCADE;") - require.NoError(t, err) +func getTestDsn(dbName string) string { + return fmt.Sprintf(testDsnTemplate, dbName) } -func openTestDb(t *testing.T) *db { - clearTestDb(t) +var testPostgres *embeddedpostgres.EmbeddedPostgres - db, err := newPostgresBackend( - context.Background(), - &Config{ - Dsn: testDsn, - }, - prefix, - ) - require.NoError(t, err) - - return db -} - -type fixture struct { - t *testing.T - tempDir string - postgres *embeddedpostgres.EmbeddedPostgres -} - -func NewFixture(t *testing.T) *fixture { +// StartEmbeddedPostgres starts an embedded postgres instance. This only needs +// to be done once, because NewFixture will create random new databases on every +// call. It returns a stop closure that stops the database if called. +func StartEmbeddedPostgres() (func() error, error) { postgres := embeddedpostgres.NewDatabase( embeddedpostgres.DefaultConfig(). Port(9876)) err := postgres.Start() - require.NoError(t, err) + if err != nil { + return nil, err + } + + testPostgres = postgres + + return testPostgres.Stop, nil +} + +// NewFixture returns a new postgres test database. The database name is +// randomly generated. +func NewFixture(dbName string) (*fixture, error) { + if dbName == "" { + // Create random database name. + randBytes := make([]byte, 8) + _, err := rand.Read(randBytes) + if err != nil { + return nil, err + } + + dbName = "test_" + hex.EncodeToString(randBytes) + } + + // Create database if it doesn't exist yet. + dbConn, err := sql.Open("pgx", getTestDsn("postgres")) + if err != nil { + return nil, err + } + defer dbConn.Close() + + _, err = dbConn.ExecContext( + context.Background(), "CREATE DATABASE "+dbName, + ) + if err != nil && !strings.Contains(err.Error(), "already exists") { + return nil, err + } + + // Open database + dsn := getTestDsn(dbName) + db, err := newPostgresBackend( + context.Background(), + &Config{ + Dsn: dsn, + }, + prefix, + ) + if err != nil { + return nil, err + } return &fixture{ - t: t, - postgres: postgres, + Dsn: dsn, + Db: db, + }, nil +} + +type fixture struct { + Dsn string + Db walletdb.DB +} + +// Dump returns the raw contents of the database. +func (b *fixture) Dump() (map[string]interface{}, error) { + dbConn, err := sql.Open("pgx", b.Dsn) + if err != nil { + return nil, err } -} - -func (b *fixture) Cleanup() { - b.postgres.Stop() -} - -func (b *fixture) NewBackend() walletdb.DB { - clearTestDb(b.t) - db := openTestDb(b.t) - - return db -} - -func (b *fixture) Dump() map[string]interface{} { - dbConn, err := sql.Open("pgx", testDsn) - require.NoError(b.t, err) rows, err := dbConn.Query( "SELECT tablename FROM pg_catalog.pg_tables WHERE schemaname='public'", ) - require.NoError(b.t, err) + if err != nil { + return nil, err + } var tables []string for rows.Next() { var table string err := rows.Scan(&table) - require.NoError(b.t, err) + if err != nil { + return nil, err + } tables = append(tables, table) } @@ -93,10 +124,14 @@ func (b *fixture) Dump() map[string]interface{} { for _, table := range tables { rows, err := dbConn.Query("SELECT * FROM " + table) - require.NoError(b.t, err) + if err != nil { + return nil, err + } cols, err := rows.Columns() - require.NoError(b.t, err) + if err != nil { + return nil, err + } colCount := len(cols) var tableRows []map[string]interface{} @@ -108,7 +143,9 @@ func (b *fixture) Dump() map[string]interface{} { } err := rows.Scan(valuePtrs...) - require.NoError(b.t, err) + if err != nil { + return nil, err + } tableData := make(map[string]interface{}) for i, v := range values { @@ -127,5 +164,5 @@ func (b *fixture) Dump() map[string]interface{} { result[table] = tableRows } - return result + return result, nil } diff --git a/kvdb/postgres_test.go b/kvdb/postgres_test.go index a880ceea7..445f71b73 100644 --- a/kvdb/postgres_test.go +++ b/kvdb/postgres_test.go @@ -13,8 +13,9 @@ import ( type m = map[string]interface{} func TestPostgres(t *testing.T) { - f := postgres.NewFixture(t) - defer f.Cleanup() + stop, err := postgres.StartEmbeddedPostgres() + require.NoError(t, err) + defer stop() tests := []struct { name string @@ -173,10 +174,14 @@ func TestPostgres(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - test.test(t, f.NewBackend()) + f, err := postgres.NewFixture("") + require.NoError(t, err) + + test.test(t, f.Db) if test.expectedDb != nil { - dump := f.Dump() + dump, err := f.Dump() + require.NoError(t, err) require.Equal(t, test.expectedDb, dump) } }) From d997bbf6b3a2853f19358f05f9e19ddceacb037d Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 16 Jul 2021 18:07:30 +0200 Subject: [PATCH 3/3] channeldb/test: test with postgres --- .github/workflows/main.yml | 3 +- channeldb/db_test.go | 5 ++++ channeldb/setup_test.go | 11 +++++++ contractcourt/setup_test.go | 11 +++++++ docs/release-notes/release-notes-0.14.0.md | 2 ++ kvdb/backend.go | 21 +++++++++++-- kvdb/kvdb_no_postgres.go | 19 ++++++++++++ kvdb/kvdb_postgres.go | 15 ++++++++++ kvdb/postgres/fixture.go | 4 +++ kvdb/postgres/fixture_interface.go | 8 +++++ kvdb/test_utils.go | 34 ++++++++++++++++++++++ routing/setup_test.go | 11 +++++++ sweep/setup_test.go | 11 +++++++ 13 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 channeldb/setup_test.go create mode 100644 contractcourt/setup_test.go create mode 100644 kvdb/kvdb_no_postgres.go create mode 100644 kvdb/kvdb_postgres.go create mode 100644 kvdb/postgres/fixture_interface.go create mode 100644 kvdb/test_utils.go create mode 100644 routing/setup_test.go create mode 100644 sweep/setup_test.go diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6f35225ab..81331b740 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -206,7 +206,8 @@ jobs: matrix: unit_type: - btcd unit-cover - - unit tags="kvdb_etcd kvdb_postgres" + - unit tags="kvdb_etcd" + - unit tags="kvdb_postgres" - travis-race steps: - name: git checkout diff --git a/channeldb/db_test.go b/channeldb/db_test.go index 5731c03a8..c0b9bd79c 100644 --- a/channeldb/db_test.go +++ b/channeldb/db_test.go @@ -25,6 +25,11 @@ import ( func TestOpenWithCreate(t *testing.T) { t.Parallel() + // Checking for db file existence is not possible with postgres. + if kvdb.PostgresBackend { + t.Skip() + } + // First, create a temporary directory to be used for the duration of // this test. tempDirName, err := ioutil.TempDir("", "channeldb") diff --git a/channeldb/setup_test.go b/channeldb/setup_test.go new file mode 100644 index 000000000..91d3bfb47 --- /dev/null +++ b/channeldb/setup_test.go @@ -0,0 +1,11 @@ +package channeldb + +import ( + "testing" + + "github.com/lightningnetwork/lnd/kvdb" +) + +func TestMain(m *testing.M) { + kvdb.RunTests(m) +} diff --git a/contractcourt/setup_test.go b/contractcourt/setup_test.go new file mode 100644 index 000000000..f9925ce04 --- /dev/null +++ b/contractcourt/setup_test.go @@ -0,0 +1,11 @@ +package contractcourt + +import ( + "testing" + + "github.com/lightningnetwork/lnd/kvdb" +) + +func TestMain(m *testing.M) { + kvdb.RunTests(m) +} diff --git a/docs/release-notes/release-notes-0.14.0.md b/docs/release-notes/release-notes-0.14.0.md index eb013ee94..4e782686e 100644 --- a/docs/release-notes/release-notes-0.14.0.md +++ b/docs/release-notes/release-notes-0.14.0.md @@ -460,6 +460,8 @@ messages directly. There is no routing/path finding involved. * [Fixed flake that occurred with the switch dust forwarding test under the data race unit build.](https://github.com/lightningnetwork/lnd/pull/5828) +* [Run channeldb tests on postgres](https://github.com/lightningnetwork/lnd/pull/5550) + ## Database * [Ensure single writer for legacy diff --git a/kvdb/backend.go b/kvdb/backend.go index 014260b2f..7d4cbc36f 100644 --- a/kvdb/backend.go +++ b/kvdb/backend.go @@ -5,7 +5,9 @@ package kvdb import ( "context" + "crypto/sha256" "encoding/binary" + "encoding/hex" "fmt" "io/ioutil" "os" @@ -245,7 +247,20 @@ func updateLastCompactionDate(dbFile string) error { func GetTestBackend(path, name string) (Backend, func(), error) { empty := func() {} - if TestBackend == BoltBackendName { + switch { + case PostgresBackend: + key := filepath.Join(path, name) + keyHash := sha256.Sum256([]byte(key)) + + f, err := NewPostgresFixture("test_" + hex.EncodeToString(keyHash[:])) + if err != nil { + return nil, func() {}, err + } + return f.DB(), func() { + _ = f.DB().Close() + }, nil + + case TestBackend == BoltBackendName: db, err := GetBoltBackend(&BoltBackendConfig{ DBPath: path, DBFileName: name, @@ -256,7 +271,8 @@ func GetTestBackend(path, name string) (Backend, func(), error) { return nil, nil, err } return db, empty, nil - } else if TestBackend == EtcdBackendName { + + case TestBackend == EtcdBackendName: etcdConfig, cancel, err := StartEtcdTestBackend(path, 0, 0, "") if err != nil { return nil, empty, err @@ -265,6 +281,7 @@ func GetTestBackend(path, name string) (Backend, func(), error) { EtcdBackendName, context.TODO(), etcdConfig, ) return backend, cancel, err + } return nil, nil, fmt.Errorf("unknown backend") diff --git a/kvdb/kvdb_no_postgres.go b/kvdb/kvdb_no_postgres.go new file mode 100644 index 000000000..d581c2b55 --- /dev/null +++ b/kvdb/kvdb_no_postgres.go @@ -0,0 +1,19 @@ +// +build !kvdb_postgres + +package kvdb + +import ( + "errors" + + "github.com/lightningnetwork/lnd/kvdb/postgres" +) + +const PostgresBackend = false + +func NewPostgresFixture(dbName string) (postgres.Fixture, error) { + return nil, errors.New("postgres backend not available") +} + +func StartEmbeddedPostgres() (func() error, error) { + return nil, errors.New("postgres backend not available") +} diff --git a/kvdb/kvdb_postgres.go b/kvdb/kvdb_postgres.go new file mode 100644 index 000000000..023a51840 --- /dev/null +++ b/kvdb/kvdb_postgres.go @@ -0,0 +1,15 @@ +// +build kvdb_postgres + +package kvdb + +import "github.com/lightningnetwork/lnd/kvdb/postgres" + +const PostgresBackend = true + +func NewPostgresFixture(dbName string) (postgres.Fixture, error) { + return postgres.NewFixture(dbName) +} + +func StartEmbeddedPostgres() (func() error, error) { + return postgres.StartEmbeddedPostgres() +} diff --git a/kvdb/postgres/fixture.go b/kvdb/postgres/fixture.go index 929563220..9257f49aa 100644 --- a/kvdb/postgres/fixture.go +++ b/kvdb/postgres/fixture.go @@ -95,6 +95,10 @@ type fixture struct { Db walletdb.DB } +func (b *fixture) DB() walletdb.DB { + return b.Db +} + // Dump returns the raw contents of the database. func (b *fixture) Dump() (map[string]interface{}, error) { dbConn, err := sql.Open("pgx", b.Dsn) diff --git a/kvdb/postgres/fixture_interface.go b/kvdb/postgres/fixture_interface.go new file mode 100644 index 000000000..7a1f8b9e9 --- /dev/null +++ b/kvdb/postgres/fixture_interface.go @@ -0,0 +1,8 @@ +package postgres + +import "github.com/btcsuite/btcwallet/walletdb" + +type Fixture interface { + DB() walletdb.DB + Dump() (map[string]interface{}, error) +} diff --git a/kvdb/test_utils.go b/kvdb/test_utils.go new file mode 100644 index 000000000..adc1156e7 --- /dev/null +++ b/kvdb/test_utils.go @@ -0,0 +1,34 @@ +package kvdb + +import ( + "fmt" + "os" + "testing" +) + +// RunTests is a helper function to run the tests in a package with +// initialization and tear-down of a test kvdb backend. +func RunTests(m *testing.M) { + var close func() error + if PostgresBackend { + var err error + close, err = StartEmbeddedPostgres() + if err != nil { + fmt.Printf("Error: %v\n", err) + os.Exit(1) + } + } + + // os.Exit() does not respect defer statements + code := m.Run() + + if close != nil { + err := close() + if err != nil { + fmt.Printf("Error: %v\n", err) + } + } + + os.Exit(code) + +} diff --git a/routing/setup_test.go b/routing/setup_test.go new file mode 100644 index 000000000..544fc3885 --- /dev/null +++ b/routing/setup_test.go @@ -0,0 +1,11 @@ +package routing + +import ( + "testing" + + "github.com/lightningnetwork/lnd/kvdb" +) + +func TestMain(m *testing.M) { + kvdb.RunTests(m) +} diff --git a/sweep/setup_test.go b/sweep/setup_test.go new file mode 100644 index 000000000..e2d22ce3b --- /dev/null +++ b/sweep/setup_test.go @@ -0,0 +1,11 @@ +package sweep + +import ( + "testing" + + "github.com/lightningnetwork/lnd/kvdb" +) + +func TestMain(m *testing.M) { + kvdb.RunTests(m) +}