multi: introduce a batch filter writer #274

ellemouton · 2023-05-05T13:41:26Z

This PR adds a generic BatchWriter which can be used to batch write items to a db.
It persists the batch once the size reaches a certain threshold or if a ticker goes off
if at least one items is in the batch.

This BatchWriter is then used by the GetCFilter method to persist filters that it
gets during a query. This will improve the speed of the query since the BatchWriter is
non-blocking.

Performance Improvement

On current master branch, syncing 3200 filters on my local regtest network takes 4m25s
With this PR, syncing the same number of filters takes 4.2s

LND CI

LND CI with this PR is being tested here: lightningnetwork/lnd#7788

filterdb/db.go

chanutils/batch_writer.go

query.go

yyforyongyu

Very cool optimization⚡️ Left some comments on the usage of Batch and questions about the method manageNewItems.

filterdb/db.go

chanutils/queue.go

chanutils/batch_writer.go

positiveblue

from 4m25s to 4.2s 🔥

Those are amazing improvements Elle 🎉

It looks like we can be optimistic when we fetch the filters and assume "eventually they will be persisted". Worst case scenario we will ask for them again, but if we use sensitive default for flushing to disk it should not be a problem.

My main concern is about the concurrent queue. Isn't a simple struct with a slice + a lock enough for this?

positiveblue · 2023-05-17T05:19:20Z

filterdb/db.go

+			var targetBucket walletdb.ReadWriteBucket
+			switch filterData.Type {
+			case RegularFilter:
+				targetBucket = filters.NestedReadWriteBucket(


nit: instead of calling NestedReadWriteBucket in every iteration we can call it once before the loop and change targetBucket in the switch statement. Now we have only two optio: regBucket or error but this would work even if we add more in the future

ah yes, great idea - updated 👍

ellemouton

Thanks for the review everyone! 🙏

You guys were completely right about just using a slice for the batch writer :) I was too trigger happy with the channel 😂

Updated accordingly!

It looks like we can be optimistic when we fetch the filters and assume "eventually they will be persisted". Worst case scenario we will ask for them again

Yes exactly :)

filterdb/db.go

ellemouton · 2023-05-31T13:16:42Z

filterdb/db.go

+			var targetBucket walletdb.ReadWriteBucket
+			switch filterData.Type {
+			case RegularFilter:
+				targetBucket = filters.NestedReadWriteBucket(


ah yes, great idea - updated 👍

chanutils/queue.go

chanutils/batch_writer.go

Chinwendu20 · 2023-06-07T06:35:28Z

Hello @ellemouton may I please ask how you timed this?

ellemouton · 2023-06-07T07:15:51Z

@Chinwendu20 - I set up a regtest node & mined a bunch of blocks. Then I connected to it with that script i sent you in slack the other day and would time how long the rescan of those blocks would take. Note that for header syncing (which is what I believe you are working on), you would just want to time that part & not the rescan.

Lint the files and update the tests to use the `require` package.

Add a new FilterData type that encapsulates the filter, blockhash and type associated with a filter. This is done in order to prepare for a commit that will enable batch writes of these filters.

ellemouton · 2023-07-11T06:46:07Z

@yyforyongyu & @positiveblue - this is ready! The dependant PR has been merged. PTAL 🙏

lightninglabs-deploy · 2023-07-18T07:02:26Z

@Roasbeef: review reminder
@positiveblue: review reminder
@yyforyongyu: review reminder

Roasbeef

LGTM 🏐

One small suggestion to use list.List[T]

filterdb/db.go

Roasbeef · 2023-07-18T11:00:32Z

chanutils/queue.go

+
+	chanIn   chan T
+	chanOut  chan T
+	overflow *list.List


Can use this generic list to inherit type safety (or sort of push down the concerns to another layer):

neutrino/cache/lru/list.go

Lines 62 to 63 in 5aac983

// NewList returns an initialized list.

func NewList[V any]() *List[V] { return new(List[V]).Init() }

oh sweet - that is perfect. Updated!

chanutils/batch_writer.go

ellemouton

Thanks @Roasbeef! updated. Also updated the LND PR pointing to this 👍

ellemouton · 2023-07-18T11:18:50Z

chanutils/queue.go

+
+	chanIn   chan T
+	chanOut  chan T
+	overflow *list.List


oh sweet - that is perfect. Updated!

yyforyongyu

LGTM🥳 and def impressive optimization😻

filterdb/db.go

chanutils/queue.go

chanutils/batch_writer.go

yyforyongyu · 2023-07-24T23:00:28Z

chanutils/batch_writer.go

+
+	// Stop the ticker since we don't want it to tick unless there is at
+	// least one item in the queue.
+	ticker.Stop()


nit: there's a near-impossible case, that if the DBWritesTickerDuration is extremely small, we may end up having a tick already inside ticker, causing an immediate write - no big deal here. But in a hindsight it looks like the lnd/ticker package is more suitable for this case.

cool yeah im happy to update to the lnd ticker.

The only reason not to would be for performance reasons: this ticker stops and starts the same one the whole time while the LND one keeps creating new ones & discarding old ones.
So maybe makes sense to keep as is? Agreed re the possible immediate write but also agree that it is not a big deal 👍

lemme know :)

also doesnt really make sense to make the DBWritesTickerDuration too small cause then the batching benefits are lost

yeah i think it's fine to leave it as-is, in the future we can add some config sanity check to make sure it won't happen - or let it happen, worse case we get an immediate call to the batch write.

chanutils/batch_writer_test.go

ellemouton

Thanks @yyforyongyu !

chanutils/queue.go

chanutils/batch_writer.go

ellemouton · 2023-07-25T13:09:20Z

chanutils/batch_writer.go

+
+	// Stop the ticker since we don't want it to tick unless there is at
+	// least one item in the queue.
+	ticker.Stop()


cool yeah im happy to update to the lnd ticker.

The only reason not to would be for performance reasons: this ticker stops and starts the same one the whole time while the LND one keeps creating new ones & discarding old ones.
So maybe makes sense to keep as is? Agreed re the possible immediate write but also agree that it is not a big deal 👍

lemme know :)

ellemouton · 2023-07-25T13:18:04Z

chanutils/batch_writer.go

+
+	// Stop the ticker since we don't want it to tick unless there is at
+	// least one item in the queue.
+	ticker.Stop()


also doesnt really make sense to make the DBWritesTickerDuration too small cause then the batching benefits are lost

yyforyongyu

LGTM⛵️

yyforyongyu · 2023-07-25T16:57:25Z

chanutils/queue.go

+				select {
+				case item, ok := <-cq.chanIn:
+					if !ok {
+						log.Warnf("ConcurrentQueue " +


Chinwendu20 · 2023-07-31T19:38:10Z

It seems like this change has not been reflected in btcwallet. I just rebased my branch and I could not run lnd with this new change because of this line of code in btcwallet @ellemouton
https://github.com/btcsuite/btcwallet/blob/23eda3cadf717dbf79d191873fc524178492aef9/chain/pruned_block_dispatcher.go#L194

I want to know if there is any way I can bypass this or if I am doing anything wrong. Thank you.

ellemouton · 2023-08-01T08:02:27Z

@Chinwendu20 see btcsuite/btcwallet#884 and lightningnetwork/lnd#7788

This was referenced May 5, 2023

rescan: use batch filter fetching #275

Merged

neutrino+query+rescan: improve rescan speed #236

Closed

Roasbeef requested review from positiveblue and yyforyongyu May 5, 2023 22:57

Roasbeef reviewed May 5, 2023

View reviewed changes

filterdb/db.go Outdated Show resolved Hide resolved

chanutils/batch_writer.go Show resolved Hide resolved

chanutils/batch_writer.go Show resolved Hide resolved

query.go Show resolved Hide resolved

ellemouton force-pushed the batchFilterWriter branch from 12d007d to 4f2cabe Compare May 8, 2023 07:57

yyforyongyu requested changes May 15, 2023

View reviewed changes

saubyk assigned ellemouton May 16, 2023

positiveblue reviewed May 17, 2023

View reviewed changes

ellemouton force-pushed the batchFilterWriter branch 2 times, most recently from 972afbd to 9a991c7 Compare May 31, 2023 14:31

ellemouton commented May 31, 2023

View reviewed changes

ellemouton requested review from yyforyongyu and positiveblue May 31, 2023 14:35

positiveblue mentioned this pull request Jun 6, 2023

neutrino: implement parallel header download via header checkpoints #71

Open

ellemouton force-pushed the batchFilterWriter branch from 9a991c7 to cca7080 Compare June 23, 2023 14:06

ellemouton mentioned this pull request Jun 23, 2023

go.mod: bump neutrino and btcwallet versions lightningnetwork/lnd#7788

Merged

ellemouton added 3 commits July 11, 2023 08:35

filterdb: lint and modernize the pkg

cdd05ae

Lint the files and update the tests to use the `require` package.

filterdb: add FilterData type

c0d6775

Add a new FilterData type that encapsulates the filter, blockhash and type associated with a filter. This is done in order to prepare for a commit that will enable batch writes of these filters.

filterdb: update for batch filter writes

66599be

ellemouton force-pushed the batchFilterWriter branch from cca7080 to 5c5c7bd Compare July 11, 2023 06:36

ellemouton requested a review from Roasbeef July 11, 2023 06:37

Roasbeef approved these changes Jul 18, 2023

View reviewed changes

ellemouton force-pushed the batchFilterWriter branch from 5c5c7bd to 0ad438c Compare July 18, 2023 11:20

ellemouton commented Jul 18, 2023

View reviewed changes

chanutils/queue.go Outdated

chanIn chan T

chanOut chan T

overflow *list.List

Copy link

Member Author

ellemouton Jul 18, 2023

Choose a reason for hiding this comment

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

oh sweet - that is perfect. Updated!

ellemouton force-pushed the batchFilterWriter branch from 0ad438c to a0f373f Compare July 18, 2023 11:37

yyforyongyu approved these changes Jul 24, 2023

View reviewed changes

ellemouton added 3 commits July 25, 2023 15:00

chanutils+log: add concurrent queue impl

9cdfc7b

chanutils: add BatchWriter

6b17fb3

neutrino+query: use BatchWriter for filter persistance

4cd9538

ellemouton force-pushed the batchFilterWriter branch from a0f373f to 4cd9538 Compare July 25, 2023 13:20

ellemouton commented Jul 25, 2023

View reviewed changes

yyforyongyu approved these changes Jul 25, 2023

View reviewed changes

chanutils/queue.go

select {

case item, ok := <-cq.chanIn:

if !ok {

log.Warnf("ConcurrentQueue " +

Copy link

Member

yyforyongyu Jul 25, 2023

Choose a reason for hiding this comment

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

🙏

Roasbeef merged commit f8b6f23 into lightninglabs:master Jul 25, 2023

ellemouton deleted the batchFilterWriter branch July 25, 2023 18:13

buck54321 mentioned this pull request May 8, 2024

catch spv up with lightninglabs/neutrino dcrlabs/ltcwallet#6

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

multi: introduce a batch filter writer #274

multi: introduce a batch filter writer #274

ellemouton commented May 5, 2023 •

edited

Loading

yyforyongyu left a comment

positiveblue left a comment

positiveblue May 17, 2023

ellemouton May 31, 2023

ellemouton left a comment •

edited

Loading

ellemouton May 31, 2023

Chinwendu20 commented Jun 7, 2023

ellemouton commented Jun 7, 2023

ellemouton commented Jul 11, 2023

lightninglabs-deploy commented Jul 18, 2023

Roasbeef left a comment

Roasbeef Jul 18, 2023

ellemouton Jul 18, 2023

ellemouton left a comment

ellemouton Jul 18, 2023

yyforyongyu left a comment

yyforyongyu Jul 24, 2023

ellemouton Jul 25, 2023

ellemouton Jul 25, 2023

yyforyongyu Jul 25, 2023

ellemouton left a comment

ellemouton Jul 25, 2023

ellemouton Jul 25, 2023

yyforyongyu left a comment

yyforyongyu Jul 25, 2023

Chinwendu20 commented Jul 31, 2023 •

edited

Loading

ellemouton commented Aug 1, 2023

	// NewList returns an initialized list.
	func NewList[V any]() *List[V] { return new(List[V]).Init() }

multi: introduce a batch filter writer #274

multi: introduce a batch filter writer #274

Conversation

ellemouton commented May 5, 2023 • edited Loading

Performance Improvement

LND CI

yyforyongyu left a comment

Choose a reason for hiding this comment

positiveblue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellemouton left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chinwendu20 commented Jun 7, 2023

ellemouton commented Jun 7, 2023

ellemouton commented Jul 11, 2023

lightninglabs-deploy commented Jul 18, 2023

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellemouton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yyforyongyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellemouton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yyforyongyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chinwendu20 commented Jul 31, 2023 • edited Loading

ellemouton commented Aug 1, 2023

ellemouton commented May 5, 2023 •

edited

Loading

ellemouton left a comment •

edited

Loading

Chinwendu20 commented Jul 31, 2023 •

edited

Loading