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

Allow get_persistence_condvar_value outside of tests #1661

Closed

Conversation

MaxFangX
Copy link
Contributor

@MaxFangX MaxFangX commented Aug 10, 2022

Summary

Allow usage of ChannelManager::get_persistence_condvar_value outside of tests.

Motivation

Polling

We implemented a Tokio-based background processor that runs entirely in a single task and which doesn't need its own thread. Accordingly, we made our entire node single-threaded. However, the only exposed methods for determining whether a ChannelManager has a persistable update require blocking (await_persistable_update, await_persistable_update_timeout), and the only way to simulate 'polling' is to do something like the following:

let needs_persist = channel_manager.await_persistable_update_timeout(Duration::from_millis(10));

Which is really just a jank way of doing what we really want: to inspect the value inside the persistence condvar. This PR removes the cfgs on get_persistence_condvar_value and allows us to poll for persistence updates in a completely non-blocking manner.

Performance

This impacts performance too: with a polling frequency of 100ms (the default in rust-lightning background processor) and a poll timeout of 10ms, using await_persistable_update_timeout as above causes our single-threaded node to be blocked and unable to do anything else about 10% of the time.

@TheBlueMatt
Copy link
Collaborator

TheBlueMatt commented Aug 10, 2022

You may be interested in #1657. Would that address your goal here?

@MaxFangX
Copy link
Contributor Author

MaxFangX commented Aug 10, 2022

That's perfect - both the solution and the timing lol. Right now, we are 'polling' await_persistable_update_timeout every time a tokio::time::interval ticks. get_persistable_update_future allows us to use the Future directly in our select! event loop and get rid of the polling interval. I'll close this PR and wait for #1657.

@MaxFangX MaxFangX closed this Aug 10, 2022
@MaxFangX MaxFangX deleted the allow-poll-persistence branch August 11, 2022 01:12
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.

2 participants