diff --git a/docs/release-notes/release-notes-0.18.2.md b/docs/release-notes/release-notes-0.18.2.md index e2216bc700..6d44635e4a 100644 --- a/docs/release-notes/release-notes-0.18.2.md +++ b/docs/release-notes/release-notes-0.18.2.md @@ -91,6 +91,10 @@ ## Testing ## Database + +* [Fixed](https://github.com/lightningnetwork/lnd/pull/8854) pagination issues + in SQL invoicedb queries. + ## Code Health ## Tooling and Documentation diff --git a/go.mod b/go.mod index 60b6c5e61b..e0bcd34502 100644 --- a/go.mod +++ b/go.mod @@ -186,7 +186,7 @@ require ( modernc.org/libc v1.49.3 // indirect modernc.org/mathutil v1.6.0 // indirect modernc.org/memory v1.8.0 // indirect - modernc.org/sqlite v1.29.8 // indirect + modernc.org/sqlite v1.29.10 // indirect modernc.org/strutil v1.2.0 // indirect modernc.org/token v1.1.0 // indirect sigs.k8s.io/yaml v1.2.0 // indirect @@ -203,6 +203,9 @@ 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 the next version of sqldb is tagged. +replace github.com/lightningnetwork/lnd/sqldb => ./sqldb + // If you change this please also update .github/pull_request_template.md and // docs/INSTALL.md. go 1.21.4 diff --git a/go.sum b/go.sum index 280c4f2845..5fe81d3507 100644 --- a/go.sum +++ b/go.sum @@ -456,8 +456,6 @@ github.com/lightningnetwork/lnd/kvdb v1.4.8 h1:xH0a5Vi1yrcZ5BEeF2ba3vlKBRxrL9uYX github.com/lightningnetwork/lnd/kvdb v1.4.8/go.mod h1:J2diNABOoII9UrMnxXS5w7vZwP7CA1CStrl8MnIrb3A= 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.2 h1:PfuYzScYMD9/QonKo/QvgsbXfTnH5DfldIimkfdW4Bk= -github.com/lightningnetwork/lnd/sqldb v1.0.2/go.mod h1:V2Xl6JNWLTKE97WJnwfs0d0TYJdIQTqK8/3aAwkd3qI= 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.2.3 h1:If5ibokA/UoCBGuCKaY6Vn2SJU0l9uAbehCnhTZjEP8= @@ -479,8 +477,6 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-runewidth v0.0.13 h1:lTGmDsbAYt5DmK6OnoV7EuIF1wEIFAcxld6ypU4OSgU= github.com/mattn/go-runewidth v0.0.13/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= -github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU= -github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/miekg/dns v1.1.43 h1:JKfpVSCB84vrAmHzyrsxB5NAr5kLoMXZArPSw7Qlgyg= @@ -1064,8 +1060,8 @@ modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4= modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0= modernc.org/sortutil v1.2.0 h1:jQiD3PfS2REGJNzNCMMaLSp/wdMNieTbKX920Cqdgqc= modernc.org/sortutil v1.2.0/go.mod h1:TKU2s7kJMf1AE84OoiGppNHJwvB753OYfNl2WRb++Ss= -modernc.org/sqlite v1.29.8 h1:nGKglNx9K5v0As+zF0/Gcl1kMkmaU1XynYyq92PbsC8= -modernc.org/sqlite v1.29.8/go.mod h1:lQPm27iqa4UNZpmr4Aor0MH0HkCLbt1huYDfWylLZFk= +modernc.org/sqlite v1.29.10 h1:3u93dz83myFnMilBGCOLbr+HjklS6+5rJLx4q86RDAg= +modernc.org/sqlite v1.29.10/go.mod h1:ItX2a1OVGgNsFh6Dv60JQvGfJfTPHPVpV6DF59akYOA= modernc.org/strutil v1.2.0 h1:agBi9dp1I+eOnxXeiZawM8F4LawKv4NzGWSaLfyeNZA= modernc.org/strutil v1.2.0/go.mod h1:/mdcBmfOibveCTBxUl5B5l6W+TTH1FXPLHZE6bTosX0= modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y= diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 4a76d58aae..b0c0195227 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -6,7 +6,6 @@ import ( "database/sql" "fmt" "math" - "sync" "testing" "testing/quick" "time" @@ -24,12 +23,6 @@ import ( "github.com/stretchr/testify/require" ) -// sqliteConstructorMu is used to ensure that only one thread can call the -// sqldb.NewTestSqliteDB constructor at a time. This is a temporary workaround -// that can be removed once this race condition in the sqlite repo is resolved: -// https://gitlab.com/cznic/sqlite/-/issues/180 -var sqliteConstructorMu sync.Mutex - // TestInvoiceRegistry is a master test which encompasses all tests using an // InvoiceDB instance. The purpose of this test is to be able to run all tests // with a custom DB instance, so that we can test the same logic with different @@ -137,9 +130,7 @@ func TestInvoiceRegistry(t *testing.T) { var db *sqldb.BaseDB if sqlite { - sqliteConstructorMu.Lock() db = sqldb.NewTestSqliteDB(t).BaseDB - sqliteConstructorMu.Unlock() } else { db = sqldb.NewTestPostgresDB(t, pgFixture).BaseDB } diff --git a/invoices/invoices_test.go b/invoices/invoices_test.go index 71da385714..4e30b0ac96 100644 --- a/invoices/invoices_test.go +++ b/invoices/invoices_test.go @@ -234,9 +234,7 @@ func TestInvoices(t *testing.T) { makeSQLDB := func(t *testing.T, sqlite bool) invpkg.InvoiceDB { var db *sqldb.BaseDB if sqlite { - sqliteConstructorMu.Lock() db = sqldb.NewTestSqliteDB(t).BaseDB - sqliteConstructorMu.Unlock() } else { db = sqldb.NewTestPostgresDB(t, pgFixture).BaseDB } @@ -249,7 +247,14 @@ func TestInvoices(t *testing.T) { testClock := clock.NewTestClock(testNow) - return invpkg.NewSQLStore(executor, testClock) + // We'll use a pagination limit of 3 for all tests to ensure + // that we also cover query pagination. + const testPaginationLimit = 3 + + return invpkg.NewSQLStore( + executor, testClock, + invpkg.WithPaginationLimit(testPaginationLimit), + ) } for _, test := range testList { diff --git a/invoices/sql_store.go b/invoices/sql_store.go index bcf3c1f892..21254ddef5 100644 --- a/invoices/sql_store.go +++ b/invoices/sql_store.go @@ -20,9 +20,9 @@ import ( ) const ( - // queryPaginationLimit is used in the LIMIT clause of the SQL queries - // to limit the number of rows returned. - queryPaginationLimit = 100 + // defaultQueryPaginationLimit is used in the LIMIT clause of the SQL + // queries to limit the number of rows returned. + defaultQueryPaginationLimit = 100 ) // SQLInvoiceQueries is an interface that defines the set of operations that can @@ -152,16 +152,47 @@ type BatchedSQLInvoiceQueries interface { type SQLStore struct { db BatchedSQLInvoiceQueries clock clock.Clock + opts SQLStoreOptions +} + +// SQLStoreOptions holds the options for the SQL store. +type SQLStoreOptions struct { + paginationLimit int +} + +// defaultSQLStoreOptions returns the default options for the SQL store. +func defaultSQLStoreOptions() SQLStoreOptions { + return SQLStoreOptions{ + paginationLimit: defaultQueryPaginationLimit, + } +} + +// SQLStoreOption is a functional option that can be used to optionally modify +// the behavior of the SQL store. +type SQLStoreOption func(*SQLStoreOptions) + +// WithPaginationLimit sets the pagination limit for the SQL store queries that +// paginate results. +func WithPaginationLimit(limit int) SQLStoreOption { + return func(o *SQLStoreOptions) { + o.paginationLimit = limit + } } // NewSQLStore creates a new SQLStore instance given a open // BatchedSQLInvoiceQueries storage backend. func NewSQLStore(db BatchedSQLInvoiceQueries, - clock clock.Clock) *SQLStore { + clock clock.Clock, options ...SQLStoreOption) *SQLStore { + + opts := defaultSQLStoreOptions() + for _, applyOption := range options { + applyOption(&opts) + } return &SQLStore{ db: db, clock: clock, + opts: opts, } } @@ -617,13 +648,11 @@ func (i *SQLStore) FetchPendingInvoices(ctx context.Context) ( readTxOpt := NewSQLInvoiceQueryReadTx() err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error { - limit := queryPaginationLimit - return queryWithLimit(func(offset int) (int, error) { params := sqlc.FilterInvoicesParams{ PendingOnly: true, NumOffset: int32(offset), - NumLimit: int32(limit), + NumLimit: int32(i.opts.paginationLimit), Reverse: false, } @@ -646,7 +675,7 @@ func (i *SQLStore) FetchPendingInvoices(ctx context.Context) ( } return len(rows), nil - }, limit) + }, i.opts.paginationLimit) }, func() { invoices = make(map[lntypes.Hash]Invoice) }) @@ -660,8 +689,7 @@ func (i *SQLStore) FetchPendingInvoices(ctx context.Context) ( // InvoicesSettledSince can be used by callers to catch up any settled invoices // they missed within the settled invoice time series. We'll return all known -// settled invoice that have a settle index higher than the passed -// sinceSettleIndex. +// settled invoice that have a settle index higher than the passed idx. // // NOTE: The index starts from 1. As a result we enforce that specifying a value // below the starting index value is a noop. @@ -676,14 +704,11 @@ func (i *SQLStore) InvoicesSettledSince(ctx context.Context, idx uint64) ( readTxOpt := NewSQLInvoiceQueryReadTx() err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error { - settleIdx := idx - limit := queryPaginationLimit - err := queryWithLimit(func(offset int) (int, error) { params := sqlc.FilterInvoicesParams{ - SettleIndexGet: sqldb.SQLInt64(settleIdx + 1), - NumLimit: int32(limit), + SettleIndexGet: sqldb.SQLInt64(idx + 1), NumOffset: int32(offset), + NumLimit: int32(i.opts.paginationLimit), Reverse: false, } @@ -707,10 +732,8 @@ func (i *SQLStore) InvoicesSettledSince(ctx context.Context, idx uint64) ( invoices = append(invoices, *invoice) } - settleIdx += uint64(limit) - return len(rows), nil - }, limit) + }, i.opts.paginationLimit) if err != nil { return err } @@ -777,7 +800,7 @@ func (i *SQLStore) InvoicesSettledSince(ctx context.Context, idx uint64) ( // InvoicesAddedSince can be used by callers to seek into the event time series // of all the invoices added in the database. This method will return all -// invoices with an add index greater than the specified sinceAddIndex. +// invoices with an add index greater than the specified idx. // // NOTE: The index starts from 1. As a result we enforce that specifying a value // below the starting index value is a noop. @@ -792,14 +815,11 @@ func (i *SQLStore) InvoicesAddedSince(ctx context.Context, idx uint64) ( readTxOpt := NewSQLInvoiceQueryReadTx() err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error { - addIdx := idx - limit := queryPaginationLimit - return queryWithLimit(func(offset int) (int, error) { params := sqlc.FilterInvoicesParams{ - AddIndexGet: sqldb.SQLInt64(addIdx + 1), - NumLimit: int32(limit), + AddIndexGet: sqldb.SQLInt64(idx + 1), NumOffset: int32(offset), + NumLimit: int32(i.opts.paginationLimit), Reverse: false, } @@ -821,10 +841,8 @@ func (i *SQLStore) InvoicesAddedSince(ctx context.Context, idx uint64) ( result = append(result, *invoice) } - addIdx += uint64(limit) - return len(rows), nil - }, limit) + }, i.opts.paginationLimit) }, func() { result = nil }) @@ -851,21 +869,18 @@ func (i *SQLStore) QueryInvoices(ctx context.Context, readTxOpt := NewSQLInvoiceQueryReadTx() err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error { - limit := queryPaginationLimit - return queryWithLimit(func(offset int) (int, error) { params := sqlc.FilterInvoicesParams{ NumOffset: int32(offset), - NumLimit: int32(limit), + NumLimit: int32(i.opts.paginationLimit), PendingOnly: q.PendingOnly, + Reverse: q.Reversed, } if q.Reversed { - idx := int32(q.IndexOffset) - // If the index offset was not set, we want to // fetch from the lastest invoice. - if idx == 0 { + if q.IndexOffset == 0 { params.AddIndexLet = sqldb.SQLInt64( int64(math.MaxInt64), ) @@ -873,19 +888,15 @@ func (i *SQLStore) QueryInvoices(ctx context.Context, // The invoice with index offset id must // not be included in the results. params.AddIndexLet = sqldb.SQLInt64( - idx - int32(offset) - 1, + q.IndexOffset - 1, ) } - - params.Reverse = true } else { // The invoice with index offset id must not be // included in the results. params.AddIndexGet = sqldb.SQLInt64( - q.IndexOffset + uint64(offset) + 1, + q.IndexOffset + 1, ) - - params.Reverse = false } if q.CreationDateStart != 0 { @@ -923,7 +934,7 @@ func (i *SQLStore) QueryInvoices(ctx context.Context, } return len(rows), nil - }, limit) + }, i.opts.paginationLimit) }, func() { invoices = nil }) diff --git a/sqldb/go.mod b/sqldb/go.mod index fcf6c3ee9e..51792d2e0b 100644 --- a/sqldb/go.mod +++ b/sqldb/go.mod @@ -11,7 +11,7 @@ require ( github.com/ory/dockertest/v3 v3.10.0 github.com/stretchr/testify v1.9.0 golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8 - modernc.org/sqlite v1.29.8 + modernc.org/sqlite v1.29.10 ) require ( diff --git a/sqldb/go.sum b/sqldb/go.sum index 2d92b7667f..140eb99967 100644 --- a/sqldb/go.sum +++ b/sqldb/go.sum @@ -92,8 +92,6 @@ github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU= -github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/mitchellh/mapstructure v1.4.1 h1:CpVNEelQCZBooIPDn+AR3NpivK/TIKU8bDxdASFVQag= github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/moby/sys/mountinfo v0.5.0/go.mod h1:3bMD3Rg+zkqx8MRYPi7Pyb0Ie97QEBmdxbhnCLlSvSU= @@ -233,8 +231,8 @@ modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4= modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0= modernc.org/sortutil v1.2.0 h1:jQiD3PfS2REGJNzNCMMaLSp/wdMNieTbKX920Cqdgqc= modernc.org/sortutil v1.2.0/go.mod h1:TKU2s7kJMf1AE84OoiGppNHJwvB753OYfNl2WRb++Ss= -modernc.org/sqlite v1.29.8 h1:nGKglNx9K5v0As+zF0/Gcl1kMkmaU1XynYyq92PbsC8= -modernc.org/sqlite v1.29.8/go.mod h1:lQPm27iqa4UNZpmr4Aor0MH0HkCLbt1huYDfWylLZFk= +modernc.org/sqlite v1.29.10 h1:3u93dz83myFnMilBGCOLbr+HjklS6+5rJLx4q86RDAg= +modernc.org/sqlite v1.29.10/go.mod h1:ItX2a1OVGgNsFh6Dv60JQvGfJfTPHPVpV6DF59akYOA= modernc.org/strutil v1.2.0 h1:agBi9dp1I+eOnxXeiZawM8F4LawKv4NzGWSaLfyeNZA= modernc.org/strutil v1.2.0/go.mod h1:/mdcBmfOibveCTBxUl5B5l6W+TTH1FXPLHZE6bTosX0= modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y=