Skip to content

Commit d7e0f69

Browse files
authored
Merge pull request #8854 from bhandras/invoices-limit-offset-fixup
invoices: fix SQL invoice query pagination
2 parents 71ba355 + b35f060 commit d7e0f69

File tree

8 files changed

+72
-64
lines changed

8 files changed

+72
-64
lines changed

docs/release-notes/release-notes-0.18.2.md

+4
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@
9191

9292
## Testing
9393
## Database
94+
95+
* [Fixed](https://github.com/lightningnetwork/lnd/pull/8854) pagination issues
96+
in SQL invoicedb queries.
97+
9498
## Code Health
9599
## Tooling and Documentation
96100

go.mod

+4-1
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ require (
186186
modernc.org/libc v1.49.3 // indirect
187187
modernc.org/mathutil v1.6.0 // indirect
188188
modernc.org/memory v1.8.0 // indirect
189-
modernc.org/sqlite v1.29.8 // indirect
189+
modernc.org/sqlite v1.29.10 // indirect
190190
modernc.org/strutil v1.2.0 // indirect
191191
modernc.org/token v1.1.0 // indirect
192192
sigs.k8s.io/yaml v1.2.0 // indirect
@@ -203,6 +203,9 @@ replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2
203203
// allows us to specify that as an option.
204204
replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-display v1.30.0-hex-display
205205

206+
// Temporary replace until the next version of sqldb is tagged.
207+
replace github.com/lightningnetwork/lnd/sqldb => ./sqldb
208+
206209
// If you change this please also update .github/pull_request_template.md and
207210
// docs/INSTALL.md.
208211
go 1.21.4

go.sum

+2-6
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,6 @@ github.com/lightningnetwork/lnd/kvdb v1.4.8 h1:xH0a5Vi1yrcZ5BEeF2ba3vlKBRxrL9uYX
456456
github.com/lightningnetwork/lnd/kvdb v1.4.8/go.mod h1:J2diNABOoII9UrMnxXS5w7vZwP7CA1CStrl8MnIrb3A=
457457
github.com/lightningnetwork/lnd/queue v1.1.1 h1:99ovBlpM9B0FRCGYJo6RSFDlt8/vOkQQZznVb18iNMI=
458458
github.com/lightningnetwork/lnd/queue v1.1.1/go.mod h1:7A6nC1Qrm32FHuhx/mi1cieAiBZo5O6l8IBIoQxvkz4=
459-
github.com/lightningnetwork/lnd/sqldb v1.0.2 h1:PfuYzScYMD9/QonKo/QvgsbXfTnH5DfldIimkfdW4Bk=
460-
github.com/lightningnetwork/lnd/sqldb v1.0.2/go.mod h1:V2Xl6JNWLTKE97WJnwfs0d0TYJdIQTqK8/3aAwkd3qI=
461459
github.com/lightningnetwork/lnd/ticker v1.1.1 h1:J/b6N2hibFtC7JLV77ULQp++QLtCwT6ijJlbdiZFbSM=
462460
github.com/lightningnetwork/lnd/ticker v1.1.1/go.mod h1:waPTRAAcwtu7Ji3+3k+u/xH5GHovTsCoSVpho0KDvdA=
463461
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
479477
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
480478
github.com/mattn/go-runewidth v0.0.13 h1:lTGmDsbAYt5DmK6OnoV7EuIF1wEIFAcxld6ypU4OSgU=
481479
github.com/mattn/go-runewidth v0.0.13/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
482-
github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU=
483-
github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
484480
github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU=
485481
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
486482
github.com/miekg/dns v1.1.43 h1:JKfpVSCB84vrAmHzyrsxB5NAr5kLoMXZArPSw7Qlgyg=
@@ -1064,8 +1060,8 @@ modernc.org/opt v0.1.3 h1:3XOZf2yznlhC+ibLltsDGzABUGVx8J6pnFMS3E4dcq4=
10641060
modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0=
10651061
modernc.org/sortutil v1.2.0 h1:jQiD3PfS2REGJNzNCMMaLSp/wdMNieTbKX920Cqdgqc=
10661062
modernc.org/sortutil v1.2.0/go.mod h1:TKU2s7kJMf1AE84OoiGppNHJwvB753OYfNl2WRb++Ss=
1067-
modernc.org/sqlite v1.29.8 h1:nGKglNx9K5v0As+zF0/Gcl1kMkmaU1XynYyq92PbsC8=
1068-
modernc.org/sqlite v1.29.8/go.mod h1:lQPm27iqa4UNZpmr4Aor0MH0HkCLbt1huYDfWylLZFk=
1063+
modernc.org/sqlite v1.29.10 h1:3u93dz83myFnMilBGCOLbr+HjklS6+5rJLx4q86RDAg=
1064+
modernc.org/sqlite v1.29.10/go.mod h1:ItX2a1OVGgNsFh6Dv60JQvGfJfTPHPVpV6DF59akYOA=
10691065
modernc.org/strutil v1.2.0 h1:agBi9dp1I+eOnxXeiZawM8F4LawKv4NzGWSaLfyeNZA=
10701066
modernc.org/strutil v1.2.0/go.mod h1:/mdcBmfOibveCTBxUl5B5l6W+TTH1FXPLHZE6bTosX0=
10711067
modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y=

invoices/invoiceregistry_test.go

-9
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"database/sql"
77
"fmt"
88
"math"
9-
"sync"
109
"testing"
1110
"testing/quick"
1211
"time"
@@ -24,12 +23,6 @@ import (
2423
"github.com/stretchr/testify/require"
2524
)
2625

27-
// sqliteConstructorMu is used to ensure that only one thread can call the
28-
// sqldb.NewTestSqliteDB constructor at a time. This is a temporary workaround
29-
// that can be removed once this race condition in the sqlite repo is resolved:
30-
// https://gitlab.com/cznic/sqlite/-/issues/180
31-
var sqliteConstructorMu sync.Mutex
32-
3326
// TestInvoiceRegistry is a master test which encompasses all tests using an
3427
// InvoiceDB instance. The purpose of this test is to be able to run all tests
3528
// with a custom DB instance, so that we can test the same logic with different
@@ -137,9 +130,7 @@ func TestInvoiceRegistry(t *testing.T) {
137130

138131
var db *sqldb.BaseDB
139132
if sqlite {
140-
sqliteConstructorMu.Lock()
141133
db = sqldb.NewTestSqliteDB(t).BaseDB
142-
sqliteConstructorMu.Unlock()
143134
} else {
144135
db = sqldb.NewTestPostgresDB(t, pgFixture).BaseDB
145136
}

invoices/invoices_test.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,7 @@ func TestInvoices(t *testing.T) {
234234
makeSQLDB := func(t *testing.T, sqlite bool) invpkg.InvoiceDB {
235235
var db *sqldb.BaseDB
236236
if sqlite {
237-
sqliteConstructorMu.Lock()
238237
db = sqldb.NewTestSqliteDB(t).BaseDB
239-
sqliteConstructorMu.Unlock()
240238
} else {
241239
db = sqldb.NewTestPostgresDB(t, pgFixture).BaseDB
242240
}
@@ -249,7 +247,14 @@ func TestInvoices(t *testing.T) {
249247

250248
testClock := clock.NewTestClock(testNow)
251249

252-
return invpkg.NewSQLStore(executor, testClock)
250+
// We'll use a pagination limit of 3 for all tests to ensure
251+
// that we also cover query pagination.
252+
const testPaginationLimit = 3
253+
254+
return invpkg.NewSQLStore(
255+
executor, testClock,
256+
invpkg.WithPaginationLimit(testPaginationLimit),
257+
)
253258
}
254259

255260
for _, test := range testList {

invoices/sql_store.go

+51-40
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import (
2020
)
2121

2222
const (
23-
// queryPaginationLimit is used in the LIMIT clause of the SQL queries
24-
// to limit the number of rows returned.
25-
queryPaginationLimit = 100
23+
// defaultQueryPaginationLimit is used in the LIMIT clause of the SQL
24+
// queries to limit the number of rows returned.
25+
defaultQueryPaginationLimit = 100
2626
)
2727

2828
// SQLInvoiceQueries is an interface that defines the set of operations that can
@@ -152,16 +152,47 @@ type BatchedSQLInvoiceQueries interface {
152152
type SQLStore struct {
153153
db BatchedSQLInvoiceQueries
154154
clock clock.Clock
155+
opts SQLStoreOptions
156+
}
157+
158+
// SQLStoreOptions holds the options for the SQL store.
159+
type SQLStoreOptions struct {
160+
paginationLimit int
161+
}
162+
163+
// defaultSQLStoreOptions returns the default options for the SQL store.
164+
func defaultSQLStoreOptions() SQLStoreOptions {
165+
return SQLStoreOptions{
166+
paginationLimit: defaultQueryPaginationLimit,
167+
}
168+
}
169+
170+
// SQLStoreOption is a functional option that can be used to optionally modify
171+
// the behavior of the SQL store.
172+
type SQLStoreOption func(*SQLStoreOptions)
173+
174+
// WithPaginationLimit sets the pagination limit for the SQL store queries that
175+
// paginate results.
176+
func WithPaginationLimit(limit int) SQLStoreOption {
177+
return func(o *SQLStoreOptions) {
178+
o.paginationLimit = limit
179+
}
155180
}
156181

157182
// NewSQLStore creates a new SQLStore instance given a open
158183
// BatchedSQLInvoiceQueries storage backend.
159184
func NewSQLStore(db BatchedSQLInvoiceQueries,
160-
clock clock.Clock) *SQLStore {
185+
clock clock.Clock, options ...SQLStoreOption) *SQLStore {
186+
187+
opts := defaultSQLStoreOptions()
188+
for _, applyOption := range options {
189+
applyOption(&opts)
190+
}
161191

162192
return &SQLStore{
163193
db: db,
164194
clock: clock,
195+
opts: opts,
165196
}
166197
}
167198

@@ -617,13 +648,11 @@ func (i *SQLStore) FetchPendingInvoices(ctx context.Context) (
617648

618649
readTxOpt := NewSQLInvoiceQueryReadTx()
619650
err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error {
620-
limit := queryPaginationLimit
621-
622651
return queryWithLimit(func(offset int) (int, error) {
623652
params := sqlc.FilterInvoicesParams{
624653
PendingOnly: true,
625654
NumOffset: int32(offset),
626-
NumLimit: int32(limit),
655+
NumLimit: int32(i.opts.paginationLimit),
627656
Reverse: false,
628657
}
629658

@@ -646,7 +675,7 @@ func (i *SQLStore) FetchPendingInvoices(ctx context.Context) (
646675
}
647676

648677
return len(rows), nil
649-
}, limit)
678+
}, i.opts.paginationLimit)
650679
}, func() {
651680
invoices = make(map[lntypes.Hash]Invoice)
652681
})
@@ -660,8 +689,7 @@ func (i *SQLStore) FetchPendingInvoices(ctx context.Context) (
660689

661690
// InvoicesSettledSince can be used by callers to catch up any settled invoices
662691
// they missed within the settled invoice time series. We'll return all known
663-
// settled invoice that have a settle index higher than the passed
664-
// sinceSettleIndex.
692+
// settled invoice that have a settle index higher than the passed idx.
665693
//
666694
// NOTE: The index starts from 1. As a result we enforce that specifying a value
667695
// below the starting index value is a noop.
@@ -676,14 +704,11 @@ func (i *SQLStore) InvoicesSettledSince(ctx context.Context, idx uint64) (
676704

677705
readTxOpt := NewSQLInvoiceQueryReadTx()
678706
err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error {
679-
settleIdx := idx
680-
limit := queryPaginationLimit
681-
682707
err := queryWithLimit(func(offset int) (int, error) {
683708
params := sqlc.FilterInvoicesParams{
684-
SettleIndexGet: sqldb.SQLInt64(settleIdx + 1),
685-
NumLimit: int32(limit),
709+
SettleIndexGet: sqldb.SQLInt64(idx + 1),
686710
NumOffset: int32(offset),
711+
NumLimit: int32(i.opts.paginationLimit),
687712
Reverse: false,
688713
}
689714

@@ -707,10 +732,8 @@ func (i *SQLStore) InvoicesSettledSince(ctx context.Context, idx uint64) (
707732
invoices = append(invoices, *invoice)
708733
}
709734

710-
settleIdx += uint64(limit)
711-
712735
return len(rows), nil
713-
}, limit)
736+
}, i.opts.paginationLimit)
714737
if err != nil {
715738
return err
716739
}
@@ -777,7 +800,7 @@ func (i *SQLStore) InvoicesSettledSince(ctx context.Context, idx uint64) (
777800

778801
// InvoicesAddedSince can be used by callers to seek into the event time series
779802
// of all the invoices added in the database. This method will return all
780-
// invoices with an add index greater than the specified sinceAddIndex.
803+
// invoices with an add index greater than the specified idx.
781804
//
782805
// NOTE: The index starts from 1. As a result we enforce that specifying a value
783806
// below the starting index value is a noop.
@@ -792,14 +815,11 @@ func (i *SQLStore) InvoicesAddedSince(ctx context.Context, idx uint64) (
792815

793816
readTxOpt := NewSQLInvoiceQueryReadTx()
794817
err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error {
795-
addIdx := idx
796-
limit := queryPaginationLimit
797-
798818
return queryWithLimit(func(offset int) (int, error) {
799819
params := sqlc.FilterInvoicesParams{
800-
AddIndexGet: sqldb.SQLInt64(addIdx + 1),
801-
NumLimit: int32(limit),
820+
AddIndexGet: sqldb.SQLInt64(idx + 1),
802821
NumOffset: int32(offset),
822+
NumLimit: int32(i.opts.paginationLimit),
803823
Reverse: false,
804824
}
805825

@@ -821,10 +841,8 @@ func (i *SQLStore) InvoicesAddedSince(ctx context.Context, idx uint64) (
821841
result = append(result, *invoice)
822842
}
823843

824-
addIdx += uint64(limit)
825-
826844
return len(rows), nil
827-
}, limit)
845+
}, i.opts.paginationLimit)
828846
}, func() {
829847
result = nil
830848
})
@@ -851,41 +869,34 @@ func (i *SQLStore) QueryInvoices(ctx context.Context,
851869

852870
readTxOpt := NewSQLInvoiceQueryReadTx()
853871
err := i.db.ExecTx(ctx, &readTxOpt, func(db SQLInvoiceQueries) error {
854-
limit := queryPaginationLimit
855-
856872
return queryWithLimit(func(offset int) (int, error) {
857873
params := sqlc.FilterInvoicesParams{
858874
NumOffset: int32(offset),
859-
NumLimit: int32(limit),
875+
NumLimit: int32(i.opts.paginationLimit),
860876
PendingOnly: q.PendingOnly,
877+
Reverse: q.Reversed,
861878
}
862879

863880
if q.Reversed {
864-
idx := int32(q.IndexOffset)
865-
866881
// If the index offset was not set, we want to
867882
// fetch from the lastest invoice.
868-
if idx == 0 {
883+
if q.IndexOffset == 0 {
869884
params.AddIndexLet = sqldb.SQLInt64(
870885
int64(math.MaxInt64),
871886
)
872887
} else {
873888
// The invoice with index offset id must
874889
// not be included in the results.
875890
params.AddIndexLet = sqldb.SQLInt64(
876-
idx - int32(offset) - 1,
891+
q.IndexOffset - 1,
877892
)
878893
}
879-
880-
params.Reverse = true
881894
} else {
882895
// The invoice with index offset id must not be
883896
// included in the results.
884897
params.AddIndexGet = sqldb.SQLInt64(
885-
q.IndexOffset + uint64(offset) + 1,
898+
q.IndexOffset + 1,
886899
)
887-
888-
params.Reverse = false
889900
}
890901

891902
if q.CreationDateStart != 0 {
@@ -923,7 +934,7 @@ func (i *SQLStore) QueryInvoices(ctx context.Context,
923934
}
924935

925936
return len(rows), nil
926-
}, limit)
937+
}, i.opts.paginationLimit)
927938
}, func() {
928939
invoices = nil
929940
})

sqldb/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ require (
1111
github.com/ory/dockertest/v3 v3.10.0
1212
github.com/stretchr/testify v1.9.0
1313
golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8
14-
modernc.org/sqlite v1.29.8
14+
modernc.org/sqlite v1.29.10
1515
)
1616

1717
require (

sqldb/go.sum

+2-4
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
9292
github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
9393
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
9494
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
95-
github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU=
96-
github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
9795
github.com/mitchellh/mapstructure v1.4.1 h1:CpVNEelQCZBooIPDn+AR3NpivK/TIKU8bDxdASFVQag=
9896
github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
9997
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=
233231
modernc.org/opt v0.1.3/go.mod h1:WdSiB5evDcignE70guQKxYUl14mgWtbClRi5wmkkTX0=
234232
modernc.org/sortutil v1.2.0 h1:jQiD3PfS2REGJNzNCMMaLSp/wdMNieTbKX920Cqdgqc=
235233
modernc.org/sortutil v1.2.0/go.mod h1:TKU2s7kJMf1AE84OoiGppNHJwvB753OYfNl2WRb++Ss=
236-
modernc.org/sqlite v1.29.8 h1:nGKglNx9K5v0As+zF0/Gcl1kMkmaU1XynYyq92PbsC8=
237-
modernc.org/sqlite v1.29.8/go.mod h1:lQPm27iqa4UNZpmr4Aor0MH0HkCLbt1huYDfWylLZFk=
234+
modernc.org/sqlite v1.29.10 h1:3u93dz83myFnMilBGCOLbr+HjklS6+5rJLx4q86RDAg=
235+
modernc.org/sqlite v1.29.10/go.mod h1:ItX2a1OVGgNsFh6Dv60JQvGfJfTPHPVpV6DF59akYOA=
238236
modernc.org/strutil v1.2.0 h1:agBi9dp1I+eOnxXeiZawM8F4LawKv4NzGWSaLfyeNZA=
239237
modernc.org/strutil v1.2.0/go.mod h1:/mdcBmfOibveCTBxUl5B5l6W+TTH1FXPLHZE6bTosX0=
240238
modernc.org/token v1.1.0 h1:Xl7Ap9dKaEs5kLoOQeQmPWevfnk/DM5qcLcYlA8ys6Y=

0 commit comments

Comments
 (0)