Skip to content

routing: persistent mission control #3164

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Aug 2, 2019
Merged

routing: persistent mission control #3164

merged 9 commits into from
Aug 2, 2019

Conversation

@joostjager
Copy link
Contributor

@joostjager joostjager commented Jun 5, 2019 •
edited
Loading

This PR adds persistence to mission control by storing the raw payment results and reprocessing them on startup.

@joostjager joostjager changed the title routing: persistent mission control routing: persistent mission control [wip] Jun 5, 2019
@joostjager joostjager force-pushed the persistent-mc branch 13 times, most recently from 06997c0 to 222e395 Compare June 14, 2019 13:19
@Roasbeef Roasbeef added database Related to the database/storage of LND enhancement Improvements to existing features / behaviour incomplete WIP PR, not fully finalized, but light review possible P2 should be fixed if one has time routing labels Jun 18, 2019
@joostjager joostjager force-pushed the persistent-mc branch 4 times, most recently from f5a1dc7 to 215f2d3 Compare June 19, 2019 10:12
@joostjager joostjager mentioned this pull request Jun 20, 2019
Merged
@joostjager joostjager force-pushed the persistent-mc branch 6 times, most recently from 8681348 to 69cc375 Compare June 27, 2019 12:47
@wpaulino wpaulino removed their request for review July 20, 2019 01:06
@wpaulino wpaulino added this to the 0.8.0 milestone Jul 25, 2019
@joostjager
routing: move failure interpretation into mission control
934ea8e
@joostjager
routing: move unreadable failure handling into mission control
334b6a3
@joostjager
routing: provide payment id to mission control
b4a7665
@joostjager
routing: extract timestamp from applying payment result
1507ff6 
This commit prepares for timestamps being supplied from the database for
processing of historical payment results.
@joostjager
routing: define payment result
9e26f0d 
This commit groups together all payment result data. It is a preparation
for historical payment results being retrieved from the database.
@joostjager
lnwire: create failure message encode/decode functions
67ddb72
@joostjager joostjager force-pushed the persistent-mc branch 5 times, most recently from 84d123a to c9d2d8e Compare July 29, 2019 11:42
@joostjager joostjager mentioned this pull request Jul 29, 2019
Closed
@Roasbeef Roasbeef removed the incomplete WIP PR, not fully finalized, but light review possible label Jul 30, 2019
cfromknecht
cfromknecht reviewed Jul 30, 2019
View reviewed changes
routing/missioncontrol_store.go Outdated
Copy link
Contributor

@cfromknecht cfromknecht Jul 30, 2019

Choose a reason for hiding this comment

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

in the past i think we observed a performance issue in calling First() then Delete() repetitively, since it actually results in a quadratic operation. fortunately i think we only expect this to be called once here, but something to keep in mind in the future. @Roasbeef was this not one of the root causes in that migration performance issue?

Copy link
Contributor Author

@joostjager joostjager Jul 31, 2019

Choose a reason for hiding this comment

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

Ok interesting. Yes, it is expected to be called only once.

Copy link
Member

@Roasbeef Roasbeef Aug 2, 2019

Choose a reason for hiding this comment

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

IIRC, in that case, we would have repeated calls to Delete(), then First(), as Delete would move the cursor one record after the deleted key. We ended up resolving it by using a two pass approach: 974ec02#diff-99f3013528876f55a0e46ff7014e8a19

@joostjager joostjager requested a review from cfromknecht July 31, 2019 06:46
cfromknecht
cfromknecht approved these changes Jul 31, 2019
View reviewed changes
Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@joostjager joostjager mentioned this pull request Aug 2, 2019
Merged
Roasbeef
Roasbeef approved these changes Aug 2, 2019
View reviewed changes
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🥨

routing/missioncontrol_store.go Outdated
Copy link
Member

@Roasbeef Roasbeef Aug 2, 2019

Choose a reason for hiding this comment

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

IIRC, in that case, we would have repeated calls to Delete(), then First(), as Delete would move the cursor one record after the deleted key. We ended up resolving it by using a two pass approach: 974ec02#diff-99f3013528876f55a0e46ff7014e8a19

@Roasbeef Roasbeef merged commit 7767eec into lightningnetwork:master Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Roasbeef Roasbeef Roasbeef approved these changes

@halseth halseth Awaiting requested review from halseth

+2 more reviewers

@wpaulino wpaulino wpaulino left review comments

@cfromknecht cfromknecht cfromknecht approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

database Related to the database/storage of LND enhancement Improvements to existing features / behaviour P2 should be fixed if one has time routing

Projects

None yet

Milestone

0.8.0

Development

Successfully merging this pull request may close these issues.

5 participants

@joostjager @Roasbeef @cfromknecht @halseth @wpaulino