Periodical external pathfinding scores merge #449
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joostjager
force-pushed
the
import-scores
branch
2 times, most recently
from
e0b7070 to
58acc12
Compare
joostjager
commented
Jan 30, 2025
joostjager
force-pushed
the
import-scores
branch
3 times, most recently
from
5858404 to
fdf2b5d
Compare
joostjager
commented
Jan 31, 2025
tnull
reviewed
Jan 31, 2025
joostjager
force-pushed
the
import-scores
branch
2 times, most recently
from
5a42ff9 to
0cf5774
Compare
joostjager
commented
Feb 1, 2025
joostjager
commented
Feb 1, 2025
joostjager
force-pushed
the
import-scores
branch
3 times, most recently
from
619c9c9 to
6ded8af
Compare
tnull
reviewed
Feb 3, 2025
tnull
reviewed
Feb 3, 2025
joostjager
force-pushed
the
import-scores
branch
5 times, most recently
from
7bf68af to
14167eb
Compare
joostjager
changed the title
tnull
reviewed
Feb 6, 2025
| async fn sync_external_scores( | ||
| logger: &Logger, scorer: &Mutex<CombinedScorer<Arc<Graph>, Arc<Logger>>>, | ||
| node_metrics: &RwLock<NodeMetrics>, kv_store: Arc<DynStore>, url: &String, | ||
| ) -> () { |
There was a problem hiding this comment.
Should this return a Result?
There was a problem hiding this comment.
I don't think so - this function cannot fail?
src/scoring.rs
Outdated
| match body { | ||
| Err(e) => { | ||
| log_error!(logger, "Failed to read external scores update: {}", e); | ||
| return; |
There was a problem hiding this comment.
Let's avoid these explicit returns if possible.
There was a problem hiding this comment.
Indeed, not necessary. I wanted to exit the parent function with these returns, but just letting it run would have the same result.
In this current revision I now have a few real ones, after unwrapping the matches a bit.
joostjager
force-pushed
the
import-scores
branch
from
14167eb to
1a4b051
Compare
|
Comments addressed. Now need to decide in lightningdevkit/rust-lightning#3562 how to read the combined scorer from disk (with local and merged data), and apply that in this PR. |
tnull
reviewed
Feb 10, 2025
joostjager
force-pushed
the
import-scores
branch
from
7c9309d to
d6bb7cf
Compare
Save external pathfinding scores in a cache so that they will be available immediately after a node restart. Otherwise there might be a time window where new scores need to be downloaded still and the node operates on local data only.
joostjager
force-pushed
the
import-scores
branch
from
d6bb7cf to
5ae6a35
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #313
Depends on lightningdevkit/rust-lightning#3562
Export functionality in #458