Rewrite sync bg processor#3820
Conversation
|
👋 I see @TheBlueMatt was un-assigned. |
| } | ||
| } | ||
|
|
||
| pub struct Sleep { |
There was a problem hiding this comment.
There is already Sleeper in the repository, but from_single_future takes a future and isn't just a sleep.
No need to make it complicated using async tasks?
joostjager
force-pushed
the
rewrite-sync-bg-processor
branch
from
27a8ad2 to
74f5af1
Compare
| } | ||
| }, | ||
| |_| Instant::now(), | ||
| |time: &Instant, dur| time.elapsed().as_secs() > dur, |
There was a problem hiding this comment.
Is there a reason why this wasn't done in the first place? All the timer checks seem to be non-blocking.
| let waker_clone = waker.clone(); | ||
|
|
||
| thread::spawn(move || { | ||
| thread::sleep(duration); |
There was a problem hiding this comment.
Chatgpt implementation - probably a lot wrong about it.
|
@TheBlueMatt is this a viable approach? |
|
🔔 1st Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
We discussed this offline a few times, but I imagine this will ultimately end up with a good chunk more boilerplate than we'd get if we just let the code be duplicated, not to mention an extra sleeper thread. Doesn't really seem worth it to me, not really sure what we even gain.
|
In the mean time, we've expanded the |
|
Hi @joostjager, Thanks for your contributions to After too many struggles with bugs, outages, contributor bans, and, finally, a multi-week CI ban, the rust-lightning project is moving off of GitHub for day-to-day development. You can still file issues and access the git tree here, but PRs will now take place exclusively at https://git.rust-bitcoin.org/. As such, this PR has been migrated to https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/pulls/3820 If you log in using GitHub (or otherwise link your GitHub account from https://git.rust-bitcoin.org/user/settings/security), ownership of your PRs, issues, and comments will automatically transfer. To push updates to this PR, you'll need to use |
3 participants
Initial attempt to rewrite the sync bg processor so that it uses the async version of it. Prepares for #3778 where the
Persistertrait is made async, and wouldn't be callable from the sync bg processor anymore.Additionally it allows us to get rid of the
define_run_bodymacro.Various open ends marked by comments on this PR.