-
Notifications
You must be signed in to change notification settings - Fork 186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rescan: use batch filter fetching #275
rescan: use batch filter fetching #275
Conversation
9a49d03
to
a2e91f4
Compare
a2e91f4
to
9f4a9d9
Compare
9f4a9d9
to
a899564
Compare
a899564
to
df74c66
Compare
bca0cb2
to
833a492
Compare
@Roasbeef: review reminder |
833a492
to
0c22f0e
Compare
Introduce a new `rescanState` struct which holds the variables required for a rescan. We then also conver the existing `rescan` function into a method of `rescanState`. This refactor will assist in making future clean up of the `rescan` method easier.
Add a new `waitForBlocks` helper method to rescanState that will wait on block notifications until a given predicate returns true. Currently this method is only called with one predicate but an upcoming commit will make use of it using a differnet predicate.
This is required for the follow up commit where access to the IsCurrent method is required in the rescanState.rescan method which only has access to the ChainSource interface.
Let the rescan function wait until the filter headers have either caught up to the back end chain or until they have caught up to the specified rescan end block. This lets the rescan operation take advantage of doing batch filter fetching during rescan making the operation a lot faster since filters can be fetched in batches of 1000 instead of one at a time.
0c22f0e
to
b38fa7f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🦜
@@ -446,6 +446,36 @@ func (rs *rescanState) rescan() error { | |||
return err | |||
} | |||
|
|||
// To ensure that we batch as many filter queries as possible, we also | |||
// wait for the header chain to either be current or for it to at least | |||
// have caught up with the specified end block. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
cc @ProofOfKeags for review request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Epic performance improvements @ellemouton 🎉
GTG
post merge ACK b38fa7f It took me a sec to realize most of the diff was just nesting some values under the state structure. Seems like the only major change was blocking on waiting for the header chain to get far enough ahead to maximize the benefits of batching. |
With this PR, we ensure that
rescan
can make use of batch filter fetching bywaiting until the header chain is either current or until it is ahead of the specified
end height before starting the scan. This greatly improves the speed of a rescan
because before this commit, the filters would be fetched one-by-one.
The first few commits of this PR are just refactor commits that clean-up the
rescan
function and then the actual behaviour change is added in the final commit.
Fixes #68
Replaces #236
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 653ms
LND CI
lightningnetwork/lnd#7788