Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions lightning-background-processor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ const FRESHNESS_TIMER: u64 = 60;
#[cfg(test)]
const FRESHNESS_TIMER: u64 = 1;

const PING_TIMER: u64 = 5;

/// Trait which handles persisting a [`ChannelManager`] to disk.
///
/// [`ChannelManager`]: lightning::ln::channelmanager::ChannelManager
Expand Down Expand Up @@ -137,7 +139,8 @@ impl BackgroundProcessor {
let stop_thread = Arc::new(AtomicBool::new(false));
let stop_thread_clone = stop_thread.clone();
let handle = thread::spawn(move || -> Result<(), std::io::Error> {
let mut current_time = Instant::now();
let mut last_freshness_call = Instant::now();
let mut last_ping_call = Instant::now();
loop {
peer_manager.process_events();
channel_manager.process_pending_events(&event_handler);
Expand All @@ -152,11 +155,27 @@ impl BackgroundProcessor {
log_trace!(logger, "Terminating background processor.");
return Ok(());
}
if current_time.elapsed().as_secs() > FRESHNESS_TIMER {
log_trace!(logger, "Calling ChannelManager's and PeerManager's timer_tick_occurred");
if last_freshness_call.elapsed().as_secs() > FRESHNESS_TIMER {
log_trace!(logger, "Calling ChannelManager's timer_tick_occurred");
channel_manager.timer_tick_occurred();
last_freshness_call = Instant::now();
}
if last_ping_call.elapsed().as_secs() > PING_TIMER * 2 {
// On various platforms, we may be starved of CPU cycles for several reasons.
// E.g. on iOS, if we've been in the background, we will be entirely paused.
// Similarly, if we're on a desktop platform and the device has been asleep, we
// may not get any cycles.
// In any case, if we've been entirely paused for more than double our ping
// timer, we should have disconnected all sockets by now (and they're probably
// dead anyway), so disconnect them by calling `timer_tick_occurred()` twice.
log_trace!(logger, "Awoke after more than double our ping timer, disconnecting peers.");
peer_manager.timer_tick_occurred();
peer_manager.timer_tick_occurred();
Comment on lines +172 to +173
Copy link
Contributor

Choose a reason for hiding this comment

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

It reads to me as odd to disconnect peers this way, rather than explicitly through some disconnect_peer() or so API

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, to some extent I agree, but at the same times its what has happened - the timer should have fired twice, so that's just what we do. The PeerManager could in the future tweak how it handles timer events and I think this would be more correct.

last_ping_call = Instant::now();
} else if last_ping_call.elapsed().as_secs() > PING_TIMER {
log_trace!(logger, "Calling PeerManager's timer_tick_occurred");
peer_manager.timer_tick_occurred();
current_time = Instant::now();
last_ping_call = Instant::now();
}
}
});
Expand Down Expand Up @@ -440,8 +459,10 @@ mod tests {
let bg_processor = BackgroundProcessor::start(persister, event_handler, nodes[0].chain_monitor.clone(), nodes[0].node.clone(), nodes[0].peer_manager.clone(), nodes[0].logger.clone());
loop {
let log_entries = nodes[0].logger.lines.lock().unwrap();
let desired_log = "Calling ChannelManager's and PeerManager's timer_tick_occurred".to_string();
if log_entries.get(&("lightning_background_processor".to_string(), desired_log)).is_some() {
let desired_log = "Calling ChannelManager's timer_tick_occurred".to_string();
let second_desired_log = "Calling PeerManager's timer_tick_occurred".to_string();
if log_entries.get(&("lightning_background_processor".to_string(), desired_log)).is_some() &&
log_entries.get(&("lightning_background_processor".to_string(), second_desired_log)).is_some() {
break
}
}
Expand Down
9 changes: 6 additions & 3 deletions lightning/src/ln/peer_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1372,9 +1372,12 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
}
}

/// This function should be called roughly once every 30 seconds.
/// It will send pings to each peer and disconnect those which did not respond to the last
/// round of pings.
/// Send pings to each peer and disconnect those which did not respond to the last round of
/// pings.
///
/// This may be called on any timescale you want, however, roughly once every five to ten
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/want, however,/want. However,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't think it needs a new sentence here?

/// seconds is preferred. The call rate determines both how often we send a ping to our peers
/// and how much time they have to respond before we disconnect them.
///
/// May call [`send_data`] on all [`SocketDescriptor`]s. Thus, be very careful with reentrancy
/// issues!
Expand Down