Merge pull request #10565 from ziggie1984/fix-sqlite-config-defaults

sqldb+lncfg: wire up unused SQLite config fields

lncfg/db.go

@@ -40,9 +40,6 @@ const (
 	DefaultBatchCommitInterval = 500 * time.Millisecond
 
 	defaultPostgresMaxConnections = 50
-	defaultSqliteMaxConnections   = 2
-
-	defaultSqliteBusyTimeout = 5 * time.Second
 
 	// NSChannelDB is the namespace name that we use for the combined graph
 	// and channel state DB.
@@ -126,8 +123,8 @@ func DefaultDB() *DB {
 			QueryConfig:            *sqldb.DefaultPostgresConfig(),
 		},
 		Sqlite: &sqldb.SqliteConfig{
-			MaxConnections: defaultSqliteMaxConnections,
-			BusyTimeout:    defaultSqliteBusyTimeout,
+			MaxConnections: sqldb.DefaultSqliteMaxConns,
+			BusyTimeout:    sqldb.DefaultSqliteBusyTimeout,
 			QueryConfig:    *sqldb.DefaultSQLiteConfig(),
 		},
 		UseNativeSQL:           false,
@@ -207,7 +204,7 @@ func (db *DB) Init(ctx context.Context, dbPath string) error {
 		sqlbase.Init(db.Postgres.MaxConnections)
 
 	case db.Backend == SqliteBackend:
-		sqlbase.Init(db.Sqlite.MaxConnections)
+		sqlbase.Init(db.Sqlite.MaxConns())
 	}
 
 	return nil

sqldb/config.go

@@ -31,6 +31,40 @@ type SqliteConfig struct {
 	QueryConfig    `group:"query" namespace:"query"`
 }
 
+const (
+	// DefaultSqliteMaxConns is the default number of maximum open
+	// connections for SQLite. SQLite only supports a single writer, so a
+	// low default reduces contention on the busy_timeout and limits
+	// resource usage, especially on mobile.
+	DefaultSqliteMaxConns = 2
+
+	// DefaultSqliteBusyTimeout is the default busy_timeout value used
+	// when no BusyTimeout is configured.
+	DefaultSqliteBusyTimeout = 5 * time.Second
+)
+
+// busyTimeoutMs returns the busy_timeout value in milliseconds. If
+// BusyTimeout is not set, it returns the default value.
+func (s *SqliteConfig) busyTimeoutMs() int64 {
+	if s.BusyTimeout > 0 {
+		return s.BusyTimeout.Milliseconds()
+	}
+
+	return DefaultSqliteBusyTimeout.Milliseconds()
+}
+
+// MaxConns returns the effective maximum number of open connections. If
+// MaxConnections is not set, it returns a default of 2. This low default is
+// chosen because SQLite only supports a single writer, which helps reduce
+// contention and resource usage.
+func (s *SqliteConfig) MaxConns() int {
+	if s.MaxConnections > 0 {
+		return s.MaxConnections
+	}
+
+	return DefaultSqliteMaxConns
+}
+
 // Validate checks that the SqliteConfig values are valid.
 func (p *SqliteConfig) Validate() error {
 	if err := p.QueryConfig.Validate(true); err != nil {

sqldb/sqlite.go

@@ -70,7 +70,7 @@ func NewSqliteStore(cfg *SqliteConfig, dbPath string) (*SqliteStore, error) {
 		},
 		{
 			name:  "busy_timeout",
-			value: "5000",
+			value: fmt.Sprintf("%d", cfg.busyTimeoutMs()),
 		},
 		{
 			// With the WAL mode, this ensures that we also do an
@@ -100,6 +100,12 @@ func NewSqliteStore(cfg *SqliteConfig, dbPath string) (*SqliteStore, error) {
 		)
 	}
 
+	// Then we add any user specified pragma options. Note that these can
+	// be of the form: "key=value", "key(N)" or "key".
+	for _, option := range cfg.PragmaOptions {
+		sqliteOptions.Add(sqliteOptionPrefix, option)
+	}
+
 	// Construct the DSN which is just the database file name, appended
 	// with the series of pragma options as a query URL string. For more
 	// details on the formatting here, see the modernc.org/sqlite docs:
@@ -130,8 +136,8 @@ func NewSqliteStore(cfg *SqliteConfig, dbPath string) (*SqliteStore, error) {
 			err)
 	}
 
-	db.SetMaxOpenConns(defaultMaxConns)
-	db.SetMaxIdleConns(defaultMaxConns)
+	db.SetMaxOpenConns(cfg.MaxConns())
+	db.SetMaxIdleConns(cfg.MaxConns())
 	db.SetConnMaxLifetime(connIdleLifetime)
 	queries := sqlc.New(db)
 

sqldb/sqlite_bench_test.go

@@ -0,0 +1,204 @@
+//go:build !js && !(windows && (arm || 386)) && !(linux && (ppc64 || mips || mipsle || mips64))
+
+package sqldb
+
+import (
+	"context"
+	"database/sql"
+	"fmt"
+	"path/filepath"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/lightningnetwork/lnd/sqldb/sqlc"
+	"github.com/stretchr/testify/require"
+)
+
+// BenchmarkSqliteMaxConns benchmarks sequential reads against a SQLite
+// database with varying MaxConnections settings.
+//
+// Run with:
+//
+//	go test -bench=BenchmarkSqliteMaxConns -benchmem -run=^$ ./sqldb/
+func BenchmarkSqliteMaxConns(b *testing.B) {
+	const numInvoices = 500
+
+	// connCounts contains the MaxConnections values we want to compare.
+	// 0 means "use the library default" (currently 2 for SQLite).
+	connCounts := []int{1, 2, 4, 8, 16, 0}
+
+	// Build a fresh SQLite database that will be shared across all
+	// sub-benchmarks. We insert a fixed set of invoices once and then
+	// execute read-only queries from multiple goroutines.
+	dbFileName := filepath.Join(b.TempDir(), "bench.db")
+
+	// Open the store once with migrations so the schema is in place.
+	setupStore, err := NewSqliteStore(&SqliteConfig{
+		SkipMigrations: false,
+	}, dbFileName)
+	require.NoError(b, err)
+
+	require.NoError(b, setupStore.ApplyAllMigrations(
+		context.Background(), GetMigrations(),
+	))
+
+	ctx := context.Background()
+
+	// Insert test invoices. We use a predictable hash per invoice so we
+	// can look them up deterministically during the benchmark.
+	hashes := make([][]byte, numInvoices)
+	for i := range numInvoices {
+		hash := make([]byte, 32)
+		hash[0] = byte(i)
+		hash[1] = byte(i >> 8)
+		hashes[i] = hash
+
+		_, err := setupStore.InsertInvoice(
+			ctx, sqlc.InsertInvoiceParams{
+				Hash:               hash,
+				PaymentAddr:        hash,
+				PaymentRequestHash: hash,
+				Expiry:             3600,
+				CreatedAt:          time.Now(),
+			},
+		)
+		require.NoError(b, err)
+	}
+
+	require.NoError(b, setupStore.DB.Close())
+
+	for _, maxConns := range connCounts {
+		name := fmt.Sprintf("MaxConns=%d", maxConns)
+		if maxConns == 0 {
+			name = fmt.Sprintf("MaxConns=default(%d)",
+				DefaultSqliteMaxConns)
+		}
+
+		b.Run(name, func(b *testing.B) {
+			store, err := NewSqliteStore(
+				&SqliteConfig{
+					SkipMigrations: true,
+					MaxConnections: maxConns,
+				}, dbFileName,
+			)
+			require.NoError(b, err)
+
+			b.Cleanup(func() {
+				require.NoError(b, store.DB.Close())
+			})
+
+			var i int
+			for b.Loop() {
+				hash := hashes[i%numInvoices]
+				i++
+
+				_, err := store.GetInvoice(
+					ctx,
+					sqlc.GetInvoiceParams{
+						Hash: hash,
+					},
+				)
+				if err != nil {
+					require.ErrorIs(b, err, sql.ErrNoRows)
+				}
+			}
+		})
+	}
+}
+
+// BenchmarkSqliteMaxConnsConcurrentReads measures aggregate read throughput
+// for a fixed level of goroutine concurrency to complement the sequential
+// benchmark above. Each iteration launches a fixed number of goroutines that
+// all issue reads simultaneously, directly stressing the connection pool.
+func BenchmarkSqliteMaxConnsConcurrentReads(b *testing.B) {
+	const (
+		numInvoices = 500
+		goroutines  = 16
+	)
+
+	connCounts := []int{1, 2, 4, 8, 16, 0}
+
+	dbFileName := filepath.Join(b.TempDir(), "bench_conc.db")
+
+	setupStore, err := NewSqliteStore(&SqliteConfig{
+		SkipMigrations: false,
+	}, dbFileName)
+	require.NoError(b, err)
+
+	require.NoError(b, setupStore.ApplyAllMigrations(
+		context.Background(), GetMigrations(),
+	))
+
+	ctx := context.Background()
+
+	hashes := make([][]byte, numInvoices)
+	for i := range numInvoices {
+		hash := make([]byte, 32)
+		hash[0] = byte(i)
+		hash[1] = byte(i >> 8)
+		hashes[i] = hash
+
+		_, err := setupStore.InsertInvoice(
+			ctx, sqlc.InsertInvoiceParams{
+				Hash:               hash,
+				PaymentAddr:        hash,
+				PaymentRequestHash: hash,
+				Expiry:             3600,
+				CreatedAt:          time.Now(),
+			},
+		)
+		require.NoError(b, err)
+	}
+
+	require.NoError(b, setupStore.DB.Close())
+
+	for _, maxConns := range connCounts {
+		name := fmt.Sprintf("MaxConns=%d", maxConns)
+		if maxConns == 0 {
+			name = fmt.Sprintf("MaxConns=default(%d)",
+				DefaultSqliteMaxConns)
+		}
+
+		b.Run(name, func(b *testing.B) {
+			store, err := NewSqliteStore(
+				&SqliteConfig{
+					SkipMigrations: true,
+					MaxConnections: maxConns,
+				}, dbFileName,
+			)
+			require.NoError(b, err)
+
+			b.Cleanup(func() {
+				require.NoError(b, store.DB.Close())
+			})
+
+			for b.Loop() {
+				var wg sync.WaitGroup
+				wg.Add(goroutines)
+
+				for g := range goroutines {
+					go func() {
+						defer wg.Done()
+
+						hash := hashes[g%numInvoices]
+						_, err := store.GetInvoice(
+							ctx,
+							sqlc.GetInvoiceParams{
+								Hash: hash,
+							},
+						)
+						if err != nil &&
+							err != sql.ErrNoRows {
+
+							b.Errorf("GetInvoice:"+
+								" %v", err)
+						}
+					}()
+				}
+
+				wg.Wait()
+			}
+		})
+	}
+}