Skip to content
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

routing+server: add new QueryBandwidth method to reduce outbound fail… #1203

Merged
merged 1 commit into from May 15, 2018

Conversation

@Roasbeef
Copy link
Member

commented May 8, 2018

…ures

In this commit, we introduce a new method to the channel router's config
struct: QueryBandwidth. This method allows the channel router to query
for the up-to-date available bandwidth of a particular link. In the case
that this link emanates from/to us, then we can query the switch to see
if the link is active (if not bandwidth is zero), and return the current
best estimate for the available bandwidth of the link. If the link,
isn't one of ours, then we can thread through the total maximal
capacity of the link.

The aim of this change is to reduce the number of unnecessary failures
during HTLC payment routing as we'll now skip any links that are
inactive, or just don't have enough bandwidth for the payment. Nodes
that have several hundred channels (all of which in various states of
activity and available bandwidth) should see a nice gain from this w.r.t
payment latency.

@Roasbeef Roasbeef added this to the 0.4.2-beta milestone May 8, 2018

@meshcollider meshcollider added the routing label May 8, 2018

@Roasbeef Roasbeef force-pushed the Roasbeef:router-view-switch branch 2 times, most recently from 0b9cce7 to b4fd959 May 8, 2018

@Roasbeef

This comment has been minimized.

Copy link
Member Author

commented May 9, 2018

Currently investigating a deadlock inadvertently introduced by the PR. As is, it prevents the async_payments_benchmark test from passing.

@Roasbeef Roasbeef force-pushed the Roasbeef:router-view-switch branch from b4fd959 to baa7e68 May 10, 2018

@Roasbeef

This comment has been minimized.

Copy link
Member Author

commented May 10, 2018

Resolved the deadlock by reworking the implementation slightly, just pushed up the new version!

@halseth halseth requested a review from wpaulino May 14, 2018

@wpaulino
Copy link
Collaborator

left a comment

Nice improvement! LGTM other than some minor style nits.

edgeBandwidth lnwire.MilliSatoshi
ok bool
)
edgeBandwidth, ok = bandwidthHints[edgeInfo.ChannelID]

This comment has been minimized.

Copy link
@wpaulino

wpaulino May 14, 2018

Collaborator

Style nit: Looks like we can just declare these here rather than within the var () above.

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef May 14, 2018

Author Member

Yep, prior code was slightly diff and needed it, but not the case anymore.

server.go Outdated
// If this is a link on the graph detected from us,
// then we'll just thread through the capacity of the
// edge as we know it.
return lnwire.NewMSatFromSatoshis(edge.Capacity)

This comment has been minimized.

Copy link
@wpaulino

wpaulino May 14, 2018

Collaborator

Style nit: Could rewrite this to return early and avoid most of the indentation above.

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef May 14, 2018

Author Member

Condensed.

routing+server: add new QueryBandwidth method to reduce outbound fail…
…ures

In this commit, we introduce a new method to the channel router's config
struct: QueryBandwidth. This method allows the channel router to query
for the up-to-date available bandwidth of a particular link. In the case
that this link emanates from/to us, then we can query the switch to see
if the link is active (if not bandwidth is zero), and return the current
best estimate for the available bandwidth of the link. If the link,
isn't one of ours, then we can thread through the total maximal
capacity of the link.

In order to implement this, the missionControl struct will now query the
switch upon creation to obtain a fresh bandwidth snapshot. We take care
to do this in a distinct db transaction in order to now introduced a
circular waiting condition between the mutexes in bolt, and the channel
state machine.

The aim of this change is to reduce the number of unnecessary failures
during HTLC payment routing as we'll now skip any links that are
inactive, or just don't have enough bandwidth for the payment. Nodes
that have several hundred channels (all of which in various states of
activity and available bandwidth) should see a nice gain from this w.r.t
payment latency.

@Roasbeef Roasbeef force-pushed the Roasbeef:router-view-switch branch from baa7e68 to f494433 May 14, 2018

@Roasbeef Roasbeef merged commit 9f6af3a into lightningnetwork:master May 15, 2018

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.03%) to 53.993%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.