Skip to content

lightningd/io_loop_with_timers.c: Move mainloop to its own source file, have chaintopology use it#2690

Merged
cdecker merged 2 commits into
ElementsProject:masterfrom
ZmnSCPxj:refactor-mainloop
May 31, 2019
Merged

lightningd/io_loop_with_timers.c: Move mainloop to its own source file, have chaintopology use it#2690
cdecker merged 2 commits into
ElementsProject:masterfrom
ZmnSCPxj:refactor-mainloop

Conversation

@ZmnSCPxj
Copy link
Copy Markdown
Contributor

Fixes: #2687

@ZmnSCPxj
Copy link
Copy Markdown
Contributor Author

Alternate solution for #2687 is to just fudge the fee estimation at the start.

@ZmnSCPxj
Copy link
Copy Markdown
Contributor Author

Missing a #include"../io_loop_with_timers.c" in lightningd/test/run-find_my_abspath.c, will fix after several hours.

@ZmnSCPxj
Copy link
Copy Markdown
Contributor Author

For that matter, why does struct plugins have its own timers? Why not just unify it to a single struct timers?

@cdecker
Copy link
Copy Markdown
Member

cdecker commented May 30, 2019

For that matter, why does struct plugins have its own timers? Why not just unify it to a single struct timers?

I did that to better encapsulate the plugins and make sure we have full control over the timers we create (in the same allocation hierarchy).

@cdecker
Copy link
Copy Markdown
Member

cdecker commented May 30, 2019

Am I correct in assuming the issue is that we don't process timers when starting the topology, therefore a failed fee estimate or a failed first block fetch will result in us hanging indefinitely?

@ZmnSCPxj
Copy link
Copy Markdown
Contributor Author

Am I correct in assuming the issue is that we don't process timers when starting the topology, therefore a failed fee estimate or a failed first block fetch will result in us hanging indefinitely?

Yes, I believe so.

@ZmnSCPxj ZmnSCPxj force-pushed the refactor-mainloop branch from 9b53610 to 292fa58 Compare May 30, 2019 14:08
cdecker and others added 2 commits May 31, 2019 14:41
Signed-off-by: Christian Decker <decker.christian@gmail.com>
@cdecker cdecker force-pushed the refactor-mainloop branch from 292fa58 to cb0b9f7 Compare May 31, 2019 12:43
@ZmnSCPxj ZmnSCPxj requested a review from cdecker as a code owner May 31, 2019 12:43
@cdecker
Copy link
Copy Markdown
Member

cdecker commented May 31, 2019

I took the liberty of add a test that reproduces #2687 first, this way we make sure the fix works and we don't get a regression later :-)

@cdecker
Copy link
Copy Markdown
Member

cdecker commented May 31, 2019

I'll ack @ZmnSCPxj's part and let someone else ack mine :-)

ACK cb0b9f7

@m-schmoock
Copy link
Copy Markdown
Collaborator

ACK 592db38

Your test 592db38 without the fix properly xfails and demonstates the issue after a timeout of 70 seconds or so. It is also properly fixed by cb0b9f7 . I have some unrelated flakes sometimes okay sometimes not when running locally, but behaves same on master with or without this fix:

  • tests/test_connection.py::test_no_fee_estimate (flaky)
  • tests/test_plugin.py::test_openchannel_hook (always timeout wait_for ONCHAIN)
  • tests/test_misc.py::test_funding_reorg_private (always timeout wait_for_log "Deleting channel")

@cdecker cdecker merged commit 48df6c8 into ElementsProject:master May 31, 2019
@ZmnSCPxj ZmnSCPxj deleted the refactor-mainloop branch June 1, 2019 03:28
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.

lightningd hanging, not opening RPC port, not syncing, reducing latest block by 15

3 participants