Fix several node initialization issues #8392

sipa · 2016-07-22T14:08:50Z

There were several issues:

InitBlockIndex was still calling ActivateBestChain (intended so the genesis block would be activated), which caused the full chain activate to occur immediately (before the node was fully started and RPC was enabled). Remove this, as this is dealt with during the background import thread now.
Use a signal and a condition variable to make AppInit2 wait for the genesis block to activate, rather than a sleep-and-test loop (which needs the heavily contended cs_main at that point).
Introduce an extra init message after loading the banlist (which is confusing).
Do a block space check earlier.

If accepted, I propose that the first commit is backported to 0.13. The second needs extra translation, unfortunately.

sipa · 2016-07-22T14:17:59Z

I guess this does not entirely fix #8117: the main problem I was observing is that after a call with -reindex-chainstate, RPC calls would fail with "error code: -28: error message: Loading banlist..." until the full reindex would complete. That should be fixed by this, though fully activating may be delayed still due to the contention between CNode creation and block processing in the background.

NicolasDorier · 2016-07-24T02:41:43Z

src/init.cpp

-            MilliSleep(10);
+    {
+        boost::unique_lock<boost::mutex> lock(cs_GenesisWait);
+        while (!fHaveGenesis) {


I'm not familiar with this code so I may be wrong.
However, it seems you are locking cs_GenesisWait waiting for fHaveGenesis to be true, while at the same time, the code responsible to make fHaveGenesis true at https://github.com/bitcoin/bitcoin/pull/8392/files#diff-c865a8939105e6350a50af02766291b7R521 need the same cs_GenesisWait lock. Seems deadlock ?

@NicolasDorier Waiting on a condition variable releases the lock temporarily.

sipa · 2016-07-30T00:19:05Z

Rebased and fixed a bug in unit tests (it needed a call to ActivateBestChain as well now, since removing it from InitBlockIndex).

laanwj · 2016-08-02T06:34:45Z

Going to test this one

laanwj · 2016-08-02T13:02:21Z

It now does all the catching up at Starting network threads..., which I suppose is better than at Loading banlist... but the underlying problem is still there:

2016-08-02 12:57:08 init message: Loading banlist...
2016-08-02 12:57:08 Loaded 0 banned node ips/subnets from banlist.dat  47ms
2016-08-02 12:57:08 init message: Starting network threads...
2016-08-02 12:57:08 Added connection peer=0
2016-08-02 12:58:07 Pre-allocating up to position 0x900000 in rev00559.dat
2016-08-02 12:58:07 UpdateTip: new best=000000000000000000b4507ddb1dd656f75cf36889931653f83775e69920963c height=418553 version=0x20000001 log2_work=84.910427 tx=139363153 date='2016-06-29 22:02:20' progress=0.982011 cache=10.7MiB(4325tx)
2016-08-02 12:58:45 UpdateTip: new best=000000000000000000b3fbefdf962686338d9a0b7177adfb21962041f2147b6f height=418554 version=0x20000000 log2_work=84.910463 tx=139363318 date='2016-06-29 21:58:48' progress=0.982010 cache=15.2MiB(6634tx)
2016-08-02 12:59:11 UpdateTip: new best=00000000000000000247db830d97fd6d6ac97e471b5e0e3062a6b66eec02b5a6 height=418555 version=0x30000001 log2_work=84.910499 tx=139364312 date='2016-06-29 22:06:07' progress=0.982013 cache=17.8MiB(10005tx)
2016-08-02 12:59:13 UpdateTip: new best=0000000000000000043fdfe42285486df7ac2192d82aad1e1d240fa9ed4f8eba height=418556 version=0x20000001 log2_work=84.910535 tx=139364477 date='2016-06-29 22:06:05' progress=0.982013 cache=18.4MiB(10401tx)
2016-08-02 12:59:22 UpdateTip: new best=000000000000000003feebd19bb1745ff866e15e3e4f9939ec3dbd5c9f9d6bde height=418557 version=0x30000000 log2_work=84.91057 tx=139365187 date='2016-06-29 22:11:05' progress=0.982014 cache=20.7MiB(11931tx)
2016-08-02 12:59:24 UpdateTip: new best=000000000000000001036a974077ff29582c9504169f286c620d1c86b078b682 height=418558 version=0x20000001 log2_work=84.910606 tx=139365525 date='2016-06-29 22:12:54' progress=0.982015 cache=21.9MiB(12671tx)
2016-08-02 12:59:31 UpdateTip: new best=000000000000000004663f9109b89879caf6d41a671ce17d60c14e6413724df0 height=418559 version=0x20000001 log2_work=84.910642 tx=139366069 date='2016-06-29 22:17:59' progress=0.982017 cache=23.5MiB(13780tx)
2016-08-02 12:59:33 UpdateTip: new best=000000000000000004a15c63f0a457094f608a65f8eab712d33d80baca3922c0 height=418560 version=0x20000001 log2_work=84.910677 tx=139366361 date='2016-06-29 22:20:13' progress=0.982018 cache=23.7MiB(14223tx)
2016-08-02 12:59:33 UpdateTip: new best=000000000000000004867bc3d9e57941bb9df2d883a44d220a255998a0ffcc64 height=418561 version=0x20000001 log2_work=84.910713 tx=139366420 date='2016-06-29 22:20:37' progress=0.982018 cache=23.8MiB(14319tx)
...

Tried adding a yield in ActivateBestChain. That didn't work (also found this article on the necessity of explicit yields when unlocking). Adding a boost::this_thread::sleep(boost::posix_time::milliseconds(100)); between every block did however work. But that's wasteful...

Adding a single 200ms sleep before ActivateBestChain in ThreadImport did the trick too. Adding an arbitrary sleep is quite imprecise/fragile though. Could we launch ThreadImport after the network initialization is done? I guess not as-is, as the poll for the genesis block is expected to be present before that. But some other coordination could maybe work.

In any case ACK on the changes already here.

9d4eb9a Do diskspace check before import thread is started (Pieter Wuille) aa59f2e Add extra message to avoid a long 'Loading banlist' (Pieter Wuille) 0fd2a33 Use a signal to continue init after genesis activation (Pieter Wuille)

rebroad · 2016-08-23T09:58:31Z

src/init.cpp

    threadGroup.create_thread(boost::bind(&ThreadImport, vImportFiles));

    // Wait for genesis block to be processed
-    bool fHaveGenesis = false;
-    while (!fHaveGenesis && !fRequestShutdown) {


This line was only just added in May by Patrick! Is the loss of sensitivity to a shutdown request intentional?

Github-Pull: bitcoin#8392 Rebased-From: 0fd2a33

Github-Pull: bitcoin#8392 Rebased-From: aa59f2e

Github-Pull: bitcoin#8392 Rebased-From: 9d4eb9a

9d4eb9a Do diskspace check before import thread is started (Pieter Wuille) aa59f2e Add extra message to avoid a long 'Loading banlist' (Pieter Wuille) 0fd2a33 Use a signal to continue init after genesis activation (Pieter Wuille)

Github-Pull: bitcoin#8392 Rebased-From: 9d4eb9a

9d4eb9a Do diskspace check before import thread is started (Pieter Wuille) aa59f2e Add extra message to avoid a long 'Loading banlist' (Pieter Wuille) 0fd2a33 Use a signal to continue init after genesis activation (Pieter Wuille)

ebbb876 Peer initialization issues, bitcoin#8392 backport (furszy) Pull request description: Upstream@[8392](bitcoin#8392) backport. ACKs for top commit: random-zebra: ACK ebbb876 Mrs-X: utACK ebbb876 Tree-SHA512: 4abe3200e5b883df925f3144e22368d48569a25364b60ae63ade9ed4d4d66da759a7b427be8460bb6999342ca3fcb4653cbe5c93fc4da7f6843fab3bb1400ce7

ebbb876740a8ed1a436ba73660b2383b24a18d6a Peer initialization issues, #8392 backport (furszy) Pull request description: Upstream@[8392](bitcoin/bitcoin#8392) backport. ACKs for top commit: random-zebra: ACK ebbb876740a8ed1a436ba73660b2383b24a18d6a Mrs-X: utACK PIVX-Project/PIVX@ebbb876 Tree-SHA512: 4abe3200e5b883df925f3144e22368d48569a25364b60ae63ade9ed4d4d66da759a7b427be8460bb6999342ca3fcb4653cbe5c93fc4da7f6843fab3bb1400ce7

banlist.dat: store banlist on disk Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6310 - bitcoin/bitcoin#6315 - Only the new signal and net changes, no QT. - bitcoin/bitcoin#7350 - bitcoin/bitcoin#7458 - bitcoin/bitcoin#7696 - bitcoin/bitcoin#7959 - bitcoin/bitcoin#7906 - Only the ban-related commits. - bitcoin/bitcoin#8392 - Only the second commit. - bitcoin/bitcoin#8085 - Only the second commit. - bitcoin/bitcoin#10564 Part of #2074.

banlist.dat: store banlist on disk Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6310 - bitcoin/bitcoin#6315 - Only the new signal and net changes, no QT. - bitcoin/bitcoin#7350 - bitcoin/bitcoin#7458 - bitcoin/bitcoin#7696 - bitcoin/bitcoin#7959 - bitcoin/bitcoin#7906 - Only the ban-related commits. - bitcoin/bitcoin#8392 - Only the second commit. - bitcoin/bitcoin#8085 - Only the second commit. - bitcoin/bitcoin#10564 - bitcoin/bitcoin#13061 - Wasn't needed when I first made this backport in 2017, but it is now! Part of #2074.

jonasschnelli added the Refactoring label Jul 23, 2016

NicolasDorier reviewed Jul 24, 2016
View reviewed changes

sipa added 3 commits July 30, 2016 02:17

Use a signal to continue init after genesis activation

0fd2a33

Add extra message to avoid a long 'Loading banlist'

aa59f2e

Do diskspace check before import thread is started

9d4eb9a

sipa force-pushed the fixactivatewait branch from cfa5d1f to 9d4eb9a Compare July 30, 2016 00:17

sipa mentioned this pull request Jul 30, 2016

Initialization reports "Loading banlist..." while doing something else #8117

Closed

sipa mentioned this pull request Aug 3, 2016

Store mempool and prioritization data to disk #8448

Merged

laanwj merged commit 9d4eb9a into bitcoin:master Aug 4, 2016

rebroad reviewed Aug 23, 2016
View reviewed changes

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Sep 21, 2016

Use a signal to continue init after genesis activation

30eac2d

Github-Pull: bitcoin#8392 Rebased-From: 0fd2a33

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Sep 21, 2016

Add extra message to avoid a long 'Loading banlist'

3b354d2

Github-Pull: bitcoin#8392 Rebased-From: aa59f2e

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Sep 21, 2016

Do diskspace check before import thread is started

fc34928

Github-Pull: bitcoin#8392 Rebased-From: 9d4eb9a

laanwj mentioned this pull request Mar 16, 2017

QT UI never appears #10005

Closed

dagurval mentioned this pull request May 31, 2018

Use non-atomic flushing with block replay bitcoinxt/bitcoinxt#439

Merged

lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Oct 24, 2018

Do diskspace check before import thread is started

6fe2142

Github-Pull: bitcoin#8392 Rebased-From: 9d4eb9a

furszy added a commit to furszy/bitcoin-core that referenced this pull request Feb 14, 2020

Peer initialization issues, bitcoin#8392 backport

ebbb876

furszy mentioned this pull request Feb 14, 2020

[Backport] Wait locking for genesis connection. PIVX-Project/PIVX#1333

Merged

str4d mentioned this pull request Nov 11, 2020

banlist.dat: store banlist on disk zcash/zcash#2812

Merged

str4d mentioned this pull request Apr 12, 2021

Sync backports zcash/zcash#5057

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix several node initialization issues #8392

Fix several node initialization issues #8392

Uh oh!

sipa commented Jul 22, 2016 •

edited

Loading

Uh oh!

sipa commented Jul 22, 2016

Uh oh!

NicolasDorier Jul 24, 2016 •

edited

Loading

Uh oh!

sipa Jul 24, 2016 via email

Uh oh!

sipa commented Jul 30, 2016

Uh oh!

laanwj commented Aug 2, 2016

Uh oh!

laanwj commented Aug 2, 2016 •

edited

Loading

Uh oh!

rebroad Aug 23, 2016

Uh oh!

Uh oh!

Fix several node initialization issues #8392

Fix several node initialization issues #8392

Uh oh!

Conversation

sipa commented Jul 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipa commented Jul 22, 2016

Uh oh!

NicolasDorier Jul 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sipa Jul 24, 2016 via email

Choose a reason for hiding this comment

Uh oh!

sipa commented Jul 30, 2016

Uh oh!

laanwj commented Aug 2, 2016

Uh oh!

laanwj commented Aug 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rebroad Aug 23, 2016

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sipa commented Jul 22, 2016 •

edited

Loading

NicolasDorier Jul 24, 2016 •

edited

Loading

laanwj commented Aug 2, 2016 •

edited

Loading