Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion invoices/kv_sql_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestMigrationWithChannelDB(t *testing.T) {
// Just some sane defaults for the sqlite config.
const (
timeout = 5 * time.Second
maxConns = 50
maxConns = 2
)

sqliteConfig := &sqlite.Config{
Expand Down
5 changes: 4 additions & 1 deletion kvdb/kvdb_sqlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ const (
// tag is defined. This will allow testing of other database backends.
SqliteBackend = true

testMaxConnections = 50
// Sqlite allows for concurrent reads however writers are limited to
// 1 so we select 2 here to allow for concurrent reads but keep the
// number of connections low to avoid write contention.
testMaxConnections = 2
)

// StartSqliteTestBackend starts a sqlite backed wallet.DB instance
Expand Down
8 changes: 7 additions & 1 deletion kvdb/postgres/fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,18 @@ func getTestDsn(dbName string) string {

var testPostgres *embeddedpostgres.EmbeddedPostgres

// testMaxConnections is the total number of connections to the database server.
const testMaxConnections = 200

// testMaxConnectionPerDatabase is the number of connections per database the
// golang driver will open.
const testMaxConnectionPerDatabase = 10

// 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) {
sqlbase.Init(testMaxConnections)
sqlbase.Init(testMaxConnectionPerDatabase)

postgres := embeddedpostgres.NewDatabase(
embeddedpostgres.DefaultConfig().
Expand All @@ -43,6 +48,7 @@ func StartEmbeddedPostgres() (func() error, error) {
"max_connections": fmt.Sprintf(
"%d", testMaxConnections,
),
"max_pred_locks_per_transaction": "256",
},
),
)
Expand Down
2 changes: 1 addition & 1 deletion lncfg/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const (
SqliteBackend = "sqlite"
DefaultBatchCommitInterval = 500 * time.Millisecond

defaultPostgresMaxConnections = 50
defaultPostgresMaxConnections = 10
defaultSqliteMaxConnections = 2
Comment on lines +42 to 43

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To avoid duplication and ensure consistency, these default values should be sourced from the sqldb package where they are also defined.

Suggested change
defaultPostgresMaxConnections = 10
defaultSqliteMaxConnections = 2
defaultPostgresMaxConnections = sqldb.DefaultMaxConnsPostgres
defaultSqliteMaxConnections = sqldb.DefaultMaxConnsSqlite


defaultSqliteBusyTimeout = 5 * time.Second
Expand Down
2 changes: 1 addition & 1 deletion sample-lnd.conf
Original file line number Diff line number Diff line change
Expand Up @@ -1618,7 +1618,7 @@
; recommended to set a limit that is below the server connection limit.
; Otherwise errors may occur in lnd under high-load conditions.
; Default:
; db.postgres.maxconnections=50
; db.postgres.maxconnections=10
; Example:
; db.postgres.maxconnections=

Expand Down
11 changes: 9 additions & 2 deletions sqldb/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,19 @@ import (
)

const (
// defaultMaxConns is the number of permitted active and idle
// defaultMaxConnsPostgres is the number of permitted active and idle
// connections. We want to limit this so it isn't unlimited. We use the
// same value for the number of idle connections as, this can speed up
// queries given a new connection doesn't need to be established each
// time.
defaultMaxConns = 25
defaultMaxConnsPostgres = 10

// defaultMaxConnsSqlite is the number of permitted active and idle
// connections. We want to limit this to 2 because SQLite allows for
// concurrent reads however writers are limited to 1 so we select 2 here
// to allow for concurrent reads but keep the number of connections low
// to avoid write contention.
defaultMaxConnsSqlite = 2
Comment on lines +10 to +22

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To avoid duplicating these default values in the lncfg package, consider exporting these constants (e.g., DefaultMaxConnsPostgres, DefaultMaxConnsSqlite). The lncfg package can then reference them directly, ensuring consistency and making future changes easier. This change should be propagated to sqldb/postgres.go and sqldb/sqlite.go as well.

Suggested change
// defaultMaxConnsPostgres is the number of permitted active and idle
// connections. We want to limit this so it isn't unlimited. We use the
// same value for the number of idle connections as, this can speed up
// queries given a new connection doesn't need to be established each
// time.
defaultMaxConns = 25
defaultMaxConnsPostgres = 10
// defaultMaxConnsSqlite is the number of permitted active and idle
// connections. We want to limit this to 2 because SQLite allows for
// concurrent reads however writers are limited to 1 so we select 2 here
// to allow for concurrent reads but keep the number of connections low
// to avoid write contention.
defaultMaxConnsSqlite = 2
// DefaultMaxConnsPostgres is the number of permitted active and idle
// connections. We want to limit this so it isn't unlimited. We use the
// same value for the number of idle connections as, this can speed up
// queries given a new connection doesn't need to be established each
// time.
DefaultMaxConnsPostgres = 10
// DefaultMaxConnsSqlite is the number of permitted active and idle
// connections. We want to limit this to 2 because SQLite allows for
// concurrent reads however writers are limited to 1 so we select 2 here
// to allow for concurrent reads but keep the number of connections low
// to avoid write contention.
DefaultMaxConnsSqlite = 2


// connIdleLifetime is the amount of time a connection can be idle.
connIdleLifetime = 5 * time.Minute
Expand Down
2 changes: 1 addition & 1 deletion sqldb/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func NewPostgresStore(cfg *PostgresConfig) (*PostgresStore, error) {
err)
}

maxConns := defaultMaxConns
maxConns := defaultMaxConnsPostgres

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To be consistent with exporting the default values from sqldb/config.go, this should use the exported constant.

Suggested change
maxConns := defaultMaxConnsPostgres
maxConns := DefaultMaxConnsPostgres

if cfg.MaxConnections > 0 {
maxConns = cfg.MaxConnections
}
Expand Down
1 change: 1 addition & 0 deletions sqldb/postgres_fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func NewTestPgFixture(t testing.TB, expiry time.Duration) *TestPgFixture {
"-c", "log_statement=all",
"-c", "log_destination=stderr",
"-c", "max_connections=5000",
"-c", "max_pred_locks_per_transaction=256",
},
}, func(config *docker.HostConfig) {
// Set AutoRemove to true so that stopped container goes away
Expand Down
9 changes: 7 additions & 2 deletions sqldb/sqlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,13 @@ func NewSqliteStore(cfg *SqliteConfig, dbPath string) (*SqliteStore, error) {
err)
}

db.SetMaxOpenConns(defaultMaxConns)
db.SetMaxIdleConns(defaultMaxConns)
maxConns := defaultMaxConnsSqlite

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To be consistent with exporting the default values from sqldb/config.go, this should use the exported constant.

Suggested change
maxConns := defaultMaxConnsSqlite
maxConns := DefaultMaxConnsSqlite

if cfg.MaxConnections > 0 {
maxConns = cfg.MaxConnections
}

db.SetMaxOpenConns(maxConns)
db.SetMaxIdleConns(maxConns)
db.SetConnMaxLifetime(connIdleLifetime)
queries := sqlc.New(db)

Expand Down
Loading