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

[autopilot] Trigger channel opening on external score update #3520

Merged

Conversation

halseth
Copy link
Contributor

@halseth halseth commented Sep 18, 2019

This PR adds a new signal to the autopilot agent, meant to signal
when any of the available heuristics has gotten an update.

We currently use this to trigger a new channel opening after the
external scores have been updated.

@halseth halseth force-pushed the autopilot-heuristic-update-trigger branch 2 times, most recently from a32705d to a2257f9 Compare September 25, 2019 09:39
@Roasbeef Roasbeef added autopilot bug fix P3 might get fixed, nice to have labels Oct 3, 2019
@Roasbeef Roasbeef requested a review from carlaKC October 8, 2019 23:51
@Roasbeef Roasbeef added this to the 0.9 milestone Oct 8, 2019
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.

Straight forward diff, only request is for an additional unit tests to exercise this new attachment trigger.

autopilot/manager.go Show resolved Hide resolved
@halseth
Copy link
Contributor Author

halseth commented Oct 30, 2019

Unit test added.

When appending to a slice, there is no guarantee the slice won't be
modified. So instead of appending to the global slice
availableHeuristics, we create a temporary local one.
This commit adds a new signal to the autopilot agent, meant to signal
when any of the available heuristics has gotten an update.

We currently use this to trigger a new channel opening after the
external scores have been updated.
TestAgentHeuristicUpdateSignal tests that upon notification about a
heuristic update, the agent reconsults the heuristic.
@halseth halseth force-pushed the autopilot-heuristic-update-trigger branch from 765b2ee to 5a8ecfc Compare October 31, 2019 09:20
Copy link
Collaborator

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

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

LGTM 🚁

@@ -372,5 +379,11 @@ func (m *Manager) SetNodeScores(name string, scores map[NodeID]float64) error {
return fmt.Errorf("heuristic with name %v not found", name)
}

// If the autopilot agent is active, notify about the updated
// heuristic.
if m.pilot != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

m.IsActive() here?

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 🦞

@Roasbeef Roasbeef merged commit bef1cd0 into lightningnetwork:master Nov 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autopilot bug fix P3 might get fixed, nice to have
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants