init: Remove auto-import of `bootstrap.dat` and associated code #17044

fjahr · 2019-10-03T19:00:31Z

This picks up #15954

I fixed the code and added at a functional test utilizing the scripts in contrib/linearize as suggested by @MarcoFalke .

mzumsande

The CIs don't like the new test yet, probably because the relative paths require one to be in the bitcoin root dir for the scripts to be found.

Also, the help text for loadblock, "Imports blocks from external blk000??.dat file on startup" could be updated to remove the file name.

test/functional/feature_loadblock.py

jonasschnelli · 2019-10-09T06:31:12Z

Concept ACK

ci/test/06_script_b.sh

test/functional/feature_loadblock.py

DrahtBot · 2019-10-09T15:51:37Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16981 (Improve runtime performance of --reindex by LarryRuane)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fjahr · 2019-10-09T23:00:02Z

As the test got much larger than the initial commit and also fixes a separate issue #17019 it is now split into its own PR at #17091

promag

Concept ACK

src/init.cpp

luke-jr · 2019-10-12T18:46:40Z

Concept ACK

89339d1 tests: Add test for loadblock option (Fabian Jahr) Pull request description: Fixes #17019 Was initially part of #17044 but as the test got larger it made sense to split it into its own commit as suggested in #17019 . This is testing the `-loadblock` option by using the scripts in `contrib/linearize` to generate a `bootstrap.dat` file and starting a disconnected node with it. So it is also testing the linearize scripts which were untested before and needed to be made available for the CI environment, hence they are added to `DIST_CONTRIB` in `Makefile.am`. ACKs for top commit: laanwj: ACK 89339d1 Tree-SHA512: aede0cd6e8b21194973f3633bc07fa2672d66a6f85dfe6a57cee2bb269a65d19ea49d5f9ed7914a173b3847c76e70257aa865f44bde170c1999d9655b4862d1c

maflcko

some questions

src/init.cpp

maflcko · 2019-10-23T14:32:23Z

src/init.cpp

@@ -374,7 +374,7 @@ void SetupServerArgs()
    gArgs.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (-nodebuglogfile to disable; default: %s)", DEFAULT_DEBUGLOGFILE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
    gArgs.AddArg("-feefilter", strprintf("Tell other nodes to filter invs to us by our mempool min fee (default: %u)", DEFAULT_FEEFILTER), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
    gArgs.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-loadblock=<file>", "Imports blocks from external blk000??.dat file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-loadblock=<file>", "Imports blocks from external file on startup", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);


why is this removed? I'd rather add a test for importing blk000.dat files.

@MarcoFalke: I suggested removing that above, because it seemed to imply a non-existing naming convention for the file (might as well load from bootstrap.dat or whatever your file is named).

See for example this test:

diff --git a/test/functional/feature_loadblock.py b/test/functional/feature_loadblock.py index bf2a4ff61f..a532a83946 100755 --- a/test/functional/feature_loadblock.py +++ b/test/functional/feature_loadblock.py @@ -25,7 +25,7 @@ from test_framework.util import assert_equal, wait_until class LoadblockTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 2 + self.num_nodes = 3 def run_test(self): self.nodes[1].setnetworkactive(state=False) @@ -71,13 +71,16 @@ class LoadblockTest(BitcoinTestFramework): subprocess.run([sys.executable, linearize_data_file, cfg_file], check=True) - self.log.info("Restart second, unsynced node with bootstrap file") + self.log.info("Restart unsynced nodes with bootstrap file or raw block file") self.stop_node(1) self.start_node(1, ["-loadblock=" + bootstrap_file]) - wait_until(lambda: self.nodes[1].getblockcount() == 100) + self.stop_node(2) + self.start_node(2, ["-loadblock=" + os.path.join(blocks_dir, 'blk00000.dat')]) + for i in [1, 2]: + wait_until(lambda: self.nodes[i].getblockcount() == 100) - assert_equal(self.nodes[1].getblockchaininfo()['blocks'], 100) - assert_equal(self.nodes[0].getbestblockhash(), self.nodes[1].getbestblockhash()) + assert_equal(self.nodes[i].getblockchaininfo()['blocks'], 100) + assert_equal(self.nodes[0].getbestblockhash(), self.nodes[i].getbestblockhash()) if __name__ == '__main__':

Agree with removing this. blkXXXXX.dat are the block files and the way to (re)index them is -reindex, it has nothing to do with -loadblock.

Isn't the point that you can import the block.dat files from another datadir with -loadblock=blk001.dat -loadblock=blk002.dat -loadblock=...

I don't think that's the point. In that case it's less hassle to copy over all the blk files and do a reindex.

I think the idea behind -loadblock is to import the output from linearize; huge consecutive 'block archives' (there used to be torrents for this). But I might be wrong. I'm not sure what it's used for nowadays.

Oh, so it is multi-arg to support a split-up bootstrap.dat?

Yeah, that was my understanding as well. But I did see it spelled out anywhere explicitly.

Oh, so it is multi-arg to support a split-up bootstrap.dat?

I think so; linearize splits it into 1GB files by default.

…ze scripts 89339d1 tests: Add test for loadblock option (Fabian Jahr) Pull request description: Fixes bitcoin#17019 Was initially part of bitcoin#17044 but as the test got larger it made sense to split it into its own commit as suggested in bitcoin#17019 . This is testing the `-loadblock` option by using the scripts in `contrib/linearize` to generate a `bootstrap.dat` file and starting a disconnected node with it. So it is also testing the linearize scripts which were untested before and needed to be made available for the CI environment, hence they are added to `DIST_CONTRIB` in `Makefile.am`. ACKs for top commit: laanwj: ACK 89339d1 Tree-SHA512: aede0cd6e8b21194973f3633bc07fa2672d66a6f85dfe6a57cee2bb269a65d19ea49d5f9ed7914a173b3847c76e70257aa865f44bde170c1999d9655b4862d1c

maflcko

ACK 5453e2d

laanwj · 2019-11-02T11:31:24Z

nit:

diff --git a/doc/developer-notes.md b/doc/developer-notes.md
index 1a7ce91ca67d47903a3b97d45944fb2df6ea2fa1..08004778e3e13284bf1de95efd24a62202847bfc 100644
--- a/doc/developer-notes.md
+++ b/doc/developer-notes.md
@@ -384,7 +384,7 @@ Threads
 
 - ThreadScriptCheck : Verifies block scripts.
 
-- ThreadImport : Loads blocks from blk*.dat files or bootstrap.dat.
+- ThreadImport : Loads blocks from `blk*.dat` files or `-loadblock=<file>`.
 
 - StartNode : Starts other threads.

- only load blockfiles when we have paths - add release notes for modified bootstrap functionality - amend documentation on ThreadImport

fjahr · 2019-11-05T15:50:27Z

@laanwj good catch, added that.

maflcko · 2019-11-05T17:01:21Z

ok, fine LGTM

laanwj · 2019-11-05T18:24:54Z

ACK 104f7de

@MarcoFalke

…ted code 104f7de remove old bootstrap relevant code (tryphe) Pull request description: This picks up #15954 I fixed the code and added at a functional test utilizing the scripts in `contrib/linearize` as suggested by @MarcoFalke . ACKs for top commit: laanwj: ACK 104f7de Tree-SHA512: acac9f285f9785fcbc3afc78118461e45bec2962f90ab90e9f82f3ad28adc90a44f0443b712458ccf486e46d891eb8a67f53e7bee5fa6d89e4387814fe03f117

Summary: > - only load blockfiles when we have paths > - add release notes for modified bootstrap functionality > - amend documentation on ThreadImport This is a backport of Core [[bitcoin/bitcoin#17044 | PR17044]] Note that the test related to this PR is already backported (D6512) Test Plan: `ninja && ./test/functional/test_runner.py feature_loadblock` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, majcosta Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8253

fjahr force-pushed the pr15954 branch from 43e17e5 to 4cd9cd8 Compare October 3, 2019 19:29

DrahtBot added Docs Refactoring Tests labels Oct 3, 2019

maflcko added Block storage and removed Docs Refactoring Tests labels Oct 3, 2019

maflcko changed the title ~~refactor: remove old bootstrap relevant code~~ remove old bootstrap relevant code Oct 3, 2019

fjahr force-pushed the pr15954 branch from 4cd9cd8 to 078d0a5 Compare October 3, 2019 23:13

mzumsande reviewed Oct 4, 2019

View reviewed changes

test/functional/feature_loadblock.py Outdated Show resolved Hide resolved

test/functional/feature_loadblock.py Outdated Show resolved Hide resolved

maflcko reviewed Oct 4, 2019

View reviewed changes

test/functional/feature_loadblock.py Outdated Show resolved Hide resolved

laanwj changed the title ~~remove old bootstrap relevant code~~ init: Remove auto-import of bootstrap.dat and associated code Oct 5, 2019

fjahr force-pushed the pr15954 branch from 078d0a5 to eeecdfa Compare October 5, 2019 11:44

maflcko reviewed Oct 7, 2019

View reviewed changes

test/functional/feature_loadblock.py Outdated Show resolved Hide resolved

fjahr force-pushed the pr15954 branch 2 times, most recently from 05fdea9 to 351cc69 Compare October 8, 2019 23:11

fjahr force-pushed the pr15954 branch 3 times, most recently from f9487eb to 2ac1637 Compare October 9, 2019 12:47

maflcko reviewed Oct 9, 2019

View reviewed changes

ci/test/06_script_b.sh Outdated Show resolved Hide resolved

fanquake added the Waiting for author label Oct 9, 2019

maflcko reviewed Oct 9, 2019

View reviewed changes

ci/test/06_script_b.sh Outdated Show resolved Hide resolved

maflcko reviewed Oct 9, 2019

View reviewed changes

test/functional/feature_loadblock.py Outdated Show resolved Hide resolved

fjahr force-pushed the pr15954 branch from 2ac1637 to 6a768e5 Compare October 9, 2019 15:09

maflcko removed the Waiting for author label Oct 9, 2019

This was referenced Oct 9, 2019

contrib/linearize scripts need tests #17019

Closed

tests: Add test for loadblock option and linearize scripts #17091

Merged

fjahr force-pushed the pr15954 branch from 6a768e5 to d617ff7 Compare October 9, 2019 22:54

promag reviewed Oct 10, 2019

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

fjahr force-pushed the pr15954 branch from d617ff7 to 04aeb5d Compare October 12, 2019 18:25

maflcko reviewed Oct 23, 2019

View reviewed changes

bitcoin deleted a comment from jgarzik Oct 23, 2019

fjahr force-pushed the pr15954 branch from 04aeb5d to 5453e2d Compare October 31, 2019 21:43

maflcko reviewed Nov 1, 2019

View reviewed changes

remove old bootstrap relevant code

104f7de

- only load blockfiles when we have paths - add release notes for modified bootstrap functionality - amend documentation on ThreadImport

fjahr force-pushed the pr15954 branch from 5453e2d to 104f7de Compare November 5, 2019 15:49

laanwj merged commit 104f7de into bitcoin:master Nov 5, 2019

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

achow101 mentioned this pull request Aug 22, 2021

bitcoin-qt does not load bootstrap.dat if blocks are already there in /blocks, --loadblock=<dirto/bootstrap.dat> needed #22773

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

init: Remove auto-import of `bootstrap.dat` and associated code #17044

init: Remove auto-import of `bootstrap.dat` and associated code #17044

fjahr commented Oct 3, 2019

mzumsande left a comment

jonasschnelli commented Oct 9, 2019

DrahtBot commented Oct 9, 2019 •

edited

Loading

fjahr commented Oct 9, 2019

promag left a comment

luke-jr commented Oct 12, 2019

maflcko left a comment

maflcko Oct 23, 2019

mzumsande Oct 31, 2019

maflcko Nov 1, 2019

laanwj Nov 4, 2019

maflcko Nov 4, 2019

laanwj Nov 4, 2019 •

edited

Loading

maflcko Nov 4, 2019

fjahr Nov 5, 2019

laanwj Nov 5, 2019

maflcko left a comment

laanwj commented Nov 2, 2019

fjahr commented Nov 5, 2019

maflcko commented Nov 5, 2019

laanwj commented Nov 5, 2019

init: Remove auto-import of bootstrap.dat and associated code #17044

init: Remove auto-import of bootstrap.dat and associated code #17044

Conversation

fjahr commented Oct 3, 2019

mzumsande left a comment

Choose a reason for hiding this comment

jonasschnelli commented Oct 9, 2019

DrahtBot commented Oct 9, 2019 • edited Loading

Conflicts

fjahr commented Oct 9, 2019

promag left a comment

Choose a reason for hiding this comment

luke-jr commented Oct 12, 2019

maflcko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laanwj Nov 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maflcko left a comment

Choose a reason for hiding this comment

laanwj commented Nov 2, 2019

fjahr commented Nov 5, 2019

maflcko commented Nov 5, 2019

laanwj commented Nov 5, 2019

init: Remove auto-import of `bootstrap.dat` and associated code #17044

init: Remove auto-import of `bootstrap.dat` and associated code #17044

DrahtBot commented Oct 9, 2019 •

edited

Loading

laanwj Nov 4, 2019 •

edited

Loading