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

Add Tokio example to process_events_async docs #2004

Merged

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Feb 2, 2023

With this we add as simple usage example of process_events_async based on Tokio.

This currently doesn't compile due to #2003, hence tagged blocked and in draft for the time being.

@tnull tnull marked this pull request as draft February 2, 2023 21:02
@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.75 🎉

Comparison is base (bc54441) 91.57% compared to head (610aa40) 92.33%.

❗ Current head 610aa40 differs from pull request most recent head feda5d3. Consider uploading reports for the commit feda5d3 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2004      +/-   ##
==========================================
+ Coverage   91.57%   92.33%   +0.75%     
==========================================
  Files         104      104              
  Lines       51553    61120    +9567     
  Branches    51553    61120    +9567     
==========================================
+ Hits        47212    56434    +9222     
- Misses       4341     4686     +345     
Impacted Files Coverage Δ
lightning-background-processor/src/lib.rs 77.10% <ø> (-6.47%) ⬇️

... and 39 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tnull tnull force-pushed the 2023-02-add-async-bp-example branch from 9079705 to 610aa40 Compare April 21, 2023 14:33
@tnull tnull marked this pull request as ready for review April 21, 2023 14:33
@tnull
Copy link
Contributor Author

tnull commented Apr 21, 2023

Rebased on main, should be ready-to-go as soon as #2199 lands.

G8XSU
G8XSU previously approved these changes Apr 21, 2023
@tnull
Copy link
Contributor Author

tnull commented Apr 24, 2023

@TheBlueMatt Given the async BP is usable now, should/can we still squeeze this in 115?

///
/// For example, in order to process background events in a [Tokio](https://tokio.rs/) task, you
/// could setup `process_events_async` like this:
/// ```ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be able to remove the ignore now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mh, not exactly, the ignore is there so I don't have to create pages of mock object for the below list of objects (persister, event handler, chain monitor, channel manager, etc.) without the test failing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. Would it be possible to do something like this?

//! type TxBroadcaster = dyn lightning::chain::chaininterface::BroadcasterInterface + Send + Sync;
//! type FeeEstimator = dyn lightning::chain::chaininterface::FeeEstimator + Send + Sync;
//! type Logger = dyn lightning::util::logger::Logger + Send + Sync;
//! type NodeSigner = dyn lightning::chain::keysinterface::NodeSigner + Send + Sync;
//! type UtxoLookup = dyn lightning::routing::utxo::UtxoLookup + Send + Sync;
//! type ChainFilter = dyn lightning::chain::Filter + Send + Sync;
//! type DataPersister = dyn lightning::chain::chainmonitor::Persist<lightning::chain::keysinterface::InMemorySigner> + Send + Sync;
//! type ChainMonitor = lightning::chain::chainmonitor::ChainMonitor<lightning::chain::keysinterface::InMemorySigner, Arc<ChainFilter>, Arc<TxBroadcaster>, Arc<FeeEstimator>, Arc<Logger>, Arc<DataPersister>>;
//! type ChannelManager = Arc<lightning::ln::channelmanager::SimpleArcChannelManager<ChainMonitor, TxBroadcaster, FeeEstimator, Logger>>;
//! type PeerManager = Arc<lightning::ln::peer_handler::SimpleArcPeerManager<lightning_net_tokio::SocketDescriptor, ChainMonitor, TxBroadcaster, FeeEstimator, UtxoLookup, Logger>>;
//!
//! // Connect to node with pubkey their_node_id at addr:
//! async fn connect_to_node(peer_manager: PeerManager, chain_monitor: Arc<ChainMonitor>, channel_manager: ChannelManager, their_node_id: PublicKey, addr: SocketAddr) {
//! lightning_net_tokio::connect_outbound(peer_manager, their_node_id, addr).await;
//! loop {
//! let event_handler = |event: Event| {
//! // Handle the event!
//! };
//! channel_manager.await_persistable_update();
//! channel_manager.process_pending_events(&event_handler);
//! chain_monitor.process_pending_events(&event_handler);
//! }
//! }
i.e. define dyn objects, then have a method that takes them. You could prefix the type def lines with # so they don't show up in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turned out to be not as straight-forward, but now went ~this way with 1f60a63.

@tnull tnull force-pushed the 2023-02-add-async-bp-example branch from 610aa40 to 1f60a63 Compare April 24, 2023 18:29
@TheBlueMatt
Copy link
Collaborator

Sorry missed your comment, happy to squeeze this in if its ready to go, can you go ahead and squash the fixups?

@tnull tnull force-pushed the 2023-02-add-async-bp-example branch from 1a92dc5 to dcbb0b3 Compare April 24, 2023 19:57
@tnull tnull force-pushed the 2023-02-add-async-bp-example branch from dcbb0b3 to bb4abda Compare April 24, 2023 20:23
@tnull tnull force-pushed the 2023-02-add-async-bp-example branch from bb4abda to feda5d3 Compare April 24, 2023 20:26
@tnull
Copy link
Contributor Author

tnull commented Apr 24, 2023

Squashed with the fixups:

diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
index fa8a0ffa..7e4f4791 100644
--- a/lightning-background-processor/src/lib.rs
+++ b/lightning-background-processor/src/lib.rs
@@ -468,4 +468,5 @@ use core::task;
 /// boolean indicating whether the background processing should exit. Once `sleeper` returns a
 /// future which outputs `true`, the loop will exit and this function's future will complete.
+/// The `sleeper` future is free to return early after it has triggered the exit condition.
 ///
 /// See [`BackgroundProcessor::start`] for information on which actions this handles.
@@ -514,5 +515,5 @@ use core::task;
 /// # type MyScorer = Mutex<lightning::routing::scoring::ProbabilisticScorer<Arc<MyNetworkGraph>, Arc<MyLogger>>>;
 ///
-/// # fn setup_background_processing(my_persister: Arc<MyPersister>, my_event_handler: Arc<MyEventHandler>, my_chain_monitor: Arc<MyChainMonitor>, my_channel_manager: Arc<MyChannelManager>, my_gossip_sync: Arc<MyGossipSync>, my_logger: Arc<MyLogger>, my_scorer: Arc<MyScorer>, my_peer_manager: Arc<MyPeerManager>) {
+/// # async fn setup_background_processing(my_persister: Arc<MyPersister>, my_event_handler: Arc<MyEventHandler>, my_chain_monitor: Arc<MyChainMonitor>, my_channel_manager: Arc<MyChannelManager>, my_gossip_sync: Arc<MyGossipSync>, my_logger: Arc<MyLogger>, my_scorer: Arc<MyScorer>, my_peer_manager: Arc<MyPeerManager>) {
 ///    let background_persister = Arc::clone(&my_persister);
 ///    let background_event_handler = Arc::clone(&my_event_handler);
@@ -525,15 +526,12 @@ use core::task;
 ///
 ///    // Setup the sleeper.
-///    let stop_background_processing = Arc::new(AtomicBool::new(false));
-///    let stop_fut = Arc::clone(&stop_background_processing);
+///    let (stop_sender, stop_receiver) = tokio::sync::watch::channel(());
 ///
 ///    let sleeper = move |d| {
-///            let stop = Arc::clone(&stop_fut);
+///            let mut receiver = stop_receiver.clone();
 ///            Box::pin(async move {
-///                    if stop.load(Ordering::Acquire) {
-///                            true
-///                    } else {
-///                            tokio::time::sleep(d).await;
-///                            false
+///                    tokio::select!{
+///                            _ = tokio::time::sleep(d) => false,
+///                            _ = receiver.changed() => true,
 ///                    }
 ///            })
@@ -542,5 +540,5 @@ use core::task;
 ///    let mobile_interruptable_platform = false;
 ///
-///    tokio::spawn(async move {
+///    let handle = tokio::spawn(async move {
 ///            process_events_async(
 ///                    background_persister,
@@ -560,5 +558,6 @@ use core::task;
 ///
 ///    // Stop the background processing.
-///    stop_background_processing.store(true, Ordering::Relaxed);
+///    stop_sender.send(()).unwrap();
+///    handle.await.unwrap();
 ///    # }
 ///```

@TheBlueMatt TheBlueMatt mentioned this pull request Apr 24, 2023
@TheBlueMatt TheBlueMatt merged commit ec3aa49 into lightningdevkit:main Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants