doc: Clarify -blocksdir usage #14364

sangaman · 2018-10-02T05:16:45Z

This PR attempts to clarify and correct the -blocksdir argument description and default value. -blocksdir does not refer to the full path to the actual blocks directory, but rather the root/parent directory which contains the blocks directory. Accordingly, the default value is <datadir> and not <datadir>/blocks - this behavior of defaulting to the datadir can also be seen in init.cpp:

    if (gArgs.IsArgSet("-blocksdir")) {
        path = fs::system_complete(gArgs.GetArg("-blocksdir", ""));
        if (!fs::is_directory(path)) {
            path = "";
            return path;
        }
    } else {
        path = GetDataDir(false);
    }

It also attempts to clarify that only the .dat files containing block data are impacted by -blocksdir, not the index files.

I believe this would close #12828.

DrahtBot · 2018-10-02T07:24:58Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16010 (Correct description of blocksdir default value by kristapsk)
#15457 (Check std::system for -[alert|block|wallet]notify by Sjors)
#12557 ([WIP] 64 bit iOS device support by Sjors)

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.

promag

ACK, help message needs fixing.

It also attempts to clarify that only the .dat files containing block data are impacted by -blocksdir, not the index files.

How does it clarify? It doesn't mention index files.

promag · 2018-10-02T09:13:35Z

doc/man/bitcoin-qt.1

These are updated automatically, see https://github.com/bitcoin/bitcoin/commits/master/doc/man/bitcoind.1

sangaman · 2018-10-02T12:57:49Z

@promag Thank you! I wasn't sure how detailed/verbose to make the description. How about Specify blocks root directory for .dat files, index .ldb files remain under datadir?

src/init.cpp

sangaman · 2018-12-18T05:07:49Z

Anything I should do to help get this merged? Let me know, I'd be happy to make any changes.

sangaman · 2019-01-17T02:17:20Z

I went ahead and changed the description to Specify root for blocks subdirectory to store .dat files per the feedback I've gotten. I can't think of a clearer way to put it.

src/httpserver.cpp

src/init.cpp

This commit attempts to clarify and correct the `-blocksdir` argument description and default value. `-blocksdir` does not refer to the full path to the actual `blocks` directory, but rather the root/parent directory which contains the `blocks` directory. Accordingly, the default value is `<datadir>` and not `<datadir>/blocks`. It also attempts to clarify that only the `.dat` files containing block data are impacted by `-blocksdir`, not the index files.

sangaman · 2019-01-18T04:45:45Z

@hebasto Thanks, that's a good way to put it I think.

hebasto · 2019-01-18T08:07:55Z

utACK ccc27bd

maflcko · 2019-02-12T20:49:33Z

src/init.cpp

    gArgs.AddArg("-alertnotify=<cmd>", "Execute command when a relevant alert is received or we see a really long fork (%s in cmd is replaced by message)", false, OptionsCategory::OPTIONS);
    gArgs.AddArg("-assumevalid=<hex>", strprintf("If this block is in the chain assume that it and its ancestors are valid and potentially skip their script verification (0 to verify all, default: %s, testnet: %s)", defaultChainParams->GetConsensus().defaultAssumeValid.GetHex(), testnetChainParams->GetConsensus().defaultAssumeValid.GetHex()), false, OptionsCategory::OPTIONS);
-    gArgs.AddArg("-blocksdir=<dir>", "Specify blocks directory (default: <datadir>/blocks)", false, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-blocksdir=<dir>", "Specify directory to hold blocks subdirectory for *.dat files (default: <datadir>)", false, OptionsCategory::OPTIONS);


Not sure if *.dat files is worth to mention, maybe block and undo data?

maflcko · 2019-02-12T20:49:57Z

ACK

Sjors · 2019-02-19T13:51:47Z

Concept ACK.

@MarcoFalke wrote:

Not sure if *.dat files is worth to mention, maybe block and undo data?

I think *.dat is actually more clear and doesn't require understanding the inner workings.

This behavior is thoroughly confusing so I wouldn't worry about making the text a bit verbose.

I recommend adding an explanation that blocks/index stays in the datadir, because that's a source of mistakes when people think they can just move the entire blocks directory to some other disk.

So maybe:

-blocksdir=<blocksdir>
Specify directory to hold blocks subdirectory (default: <datadir>). The *.dat files will
be placed in <blocksdir>/blocks, but note that <datadir>/blocks/index remains in place for
performance reasons, even when <datadir>/blocks is otherwise empty.

Maybe we can detect the presence of <blocksdir>/blocks/index, abort loading with Please move <blocksdir>/blocks/index to <datadir>/blocks/index.

Longer term imo we should move <datadir>/blocks/index to <datadir>/indexes/blocks, and make -blocksdir a network specific setting.

sangaman · 2019-02-19T14:12:58Z

I'd agree that being verbose is worth it if it makes things clearer, and I agree that *.dat files might be easier to grasp for users who are less familiar with the disk data of bitcoin and are unsure what block and undo data are - but ultimately I think either way is fine and a significant improvement over how its currently worded.

hebasto · 2019-02-19T18:07:59Z

@Sjors

Longer term imo we should move <datadir>/blocks/index to <datadir>/indexes/blocks, and make -blocksdir a network specific setting.

#14409 plus network specific directories, right?

Sjors · 2019-02-20T08:53:40Z

Ah, good to see -blocksdir was already made network specific. In that case we can drop the creation of a /blocks and /testnet3/blocks subdirectory (unless it already exists, for backwards compatibility).

sangaman · 2019-04-09T16:20:27Z

Is there any consensus on whether the way it's currently worded is good? It certainly seems fine to me, but I would gladly change it if that would help move this along. I think anything would be a lot clearer than how that config option is currently described. Thanks.

ccc27bd doc: Clarify -blocksdir usage (Daniel McNally) Pull request description: This PR attempts to clarify and correct the `-blocksdir` argument description and default value. `-blocksdir` does not refer to the full path to the actual `blocks` directory, but rather the root/parent directory which contains the `blocks` directory. Accordingly, the default value is `<datadir>` and not `<datadir>/blocks` - this behavior of defaulting to the datadir can also be seen in init.cpp: ```cpp if (gArgs.IsArgSet("-blocksdir")) { path = fs::system_complete(gArgs.GetArg("-blocksdir", "")); if (!fs::is_directory(path)) { path = ""; return path; } } else { path = GetDataDir(false); } ``` It also attempts to clarify that only the `.dat` files containing block data are impacted by `-blocksdir`, not the index files. I believe this would close bitcoin#12828. ACKs for commit ccc27b: hebasto: utACK ccc27bd Tree-SHA512: 7b65f66b0579fd56e8c8cd4f9f22d6af56181817762a68deccd7fca51820ad82d9a0c48f5f1f012e746c67bcdae7af4555fad867cb620a9ca538d465c9d86c2b

maflcko · 2019-05-13T16:48:29Z

Longer term imo we should move /blocks/index to /indexes/blocks, and make -blocksdir a network specific setting.

@Sjors Create an issue about that?

ccc27bd doc: Clarify -blocksdir usage (Daniel McNally) Pull request description: This PR attempts to clarify and correct the `-blocksdir` argument description and default value. `-blocksdir` does not refer to the full path to the actual `blocks` directory, but rather the root/parent directory which contains the `blocks` directory. Accordingly, the default value is `<datadir>` and not `<datadir>/blocks` - this behavior of defaulting to the datadir can also be seen in init.cpp: ```cpp if (gArgs.IsArgSet("-blocksdir")) { path = fs::system_complete(gArgs.GetArg("-blocksdir", "")); if (!fs::is_directory(path)) { path = ""; return path; } } else { path = GetDataDir(false); } ``` It also attempts to clarify that only the `.dat` files containing block data are impacted by `-blocksdir`, not the index files. I believe this would close bitcoin#12828. ACKs for commit ccc27b: hebasto: utACK ccc27bd Tree-SHA512: 7b65f66b0579fd56e8c8cd4f9f22d6af56181817762a68deccd7fca51820ad82d9a0c48f5f1f012e746c67bcdae7af4555fad867cb620a9ca538d465c9d86c2b

… Allow to specify blocks storage directory b0e0d55 doc: add '-blocksdir' option to release-notes (furszy) 50e66c2 Turn TryCreateDirectory() into TryCreateDirectories() (Marko Bencun) 035d3c2 util: Simplify path argument for CBlockTreeDB ctor (Hennadii Stepanov) a571d24 Fail if either disk space check fails (Ben Woosley) e567003 Improve blocksdir functional test. (Hennadii Stepanov) ff0ae45 Make blockdir always net specific (Hennadii Stepanov) 13a4119 doc: Clarify -blocksdir usage (Daniel McNally) a4ff899 -blocksdir: keep blockindex leveldb database in datadir (Jonas Schnelli) 3054076 QA: Add -blocksdir test (Jonas Schnelli) 39b8127 Allow to optional specify the directory for the blocks storage (Jonas Schnelli) a821b0b BugFix: Move TestingSetup::ECCVerifyHandle member to BasicTestingSetup. (furszy) 5844452 Remove unneeded GetTempPath, only use temp directory in unit tests (furszy) 7fc893b [Test] BasicTestingSetup constructor receiving the network param. (furszy) Pull request description: Grouped two different, zero risk, modifications here to not have to create two/three small and straightforward PRs: #### 1) Unit Tests: * Fixing basic unit test setup issue, was not initializing the libsecp256k1 context. Thus why we have been forced to use the extended unit testing setup for almost every test case (which includes CConnman, scheduler, validation interface, etc.. that are not needed so often). * Refactor: `BasicTestingSetup` receiving the network in the constructor. #### 2) Back ported the ability to specify a custom directory for the blocks storage. > Since the actual block files taking up more and more space, it may be desirable to have them stored in a different location then the data directory (use case: SSD for chainstate, etc., HD for blocks). This PR adds a -blocksdir option that allows one to keep the blockfiles external from the data directory (instead of creating symlinks). * bitcoin#9895. * bitcoin#12653. * bitcoin#14364. * bitcoin#14409. * bitcoin#15124. * bitcoin#17059. ACKs for top commit: Fuzzbawls: re-ACK b0e0d55 random-zebra: utACK b0e0d55 and merging... Tree-SHA512: 289e5b826c8fc23a5933a5b30e6c043f82bad827b10f3093cbc5869a9e278d7ad0a325dd19b5ad3c97f23a3dee69f5b0bee3869e486eeed5e8d8342a39919cf5

ccc27bd doc: Clarify -blocksdir usage (Daniel McNally) Pull request description: This PR attempts to clarify and correct the `-blocksdir` argument description and default value. `-blocksdir` does not refer to the full path to the actual `blocks` directory, but rather the root/parent directory which contains the `blocks` directory. Accordingly, the default value is `<datadir>` and not `<datadir>/blocks` - this behavior of defaulting to the datadir can also be seen in init.cpp: ```cpp if (gArgs.IsArgSet("-blocksdir")) { path = fs::system_complete(gArgs.GetArg("-blocksdir", "")); if (!fs::is_directory(path)) { path = ""; return path; } } else { path = GetDataDir(false); } ``` It also attempts to clarify that only the `.dat` files containing block data are impacted by `-blocksdir`, not the index files. I believe this would close bitcoin#12828. ACKs for commit ccc27b: hebasto: utACK ccc27bd Tree-SHA512: 7b65f66b0579fd56e8c8cd4f9f22d6af56181817762a68deccd7fca51820ad82d9a0c48f5f1f012e746c67bcdae7af4555fad867cb620a9ca538d465c9d86c2b

fanquake added the Docs label Oct 2, 2018

DrahtBot mentioned this pull request Oct 2, 2018

[WIP] 64 bit iOS device support #12557

Closed

promag reviewed Oct 2, 2018

View reviewed changes

sangaman force-pushed the blocksdir-description branch from 1db7727 to 5ff869b Compare October 2, 2018 13:00

hebasto reviewed Oct 2, 2018

View reviewed changes

src/init.cpp Outdated Show resolved Hide resolved

sangaman force-pushed the blocksdir-description branch from 5ff869b to ef5d858 Compare January 17, 2019 02:16

hebasto requested changes Jan 17, 2019

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

src/init.cpp Outdated Show resolved Hide resolved

sangaman force-pushed the blocksdir-description branch from ef5d858 to 80e31ef Compare January 18, 2019 04:43

sangaman force-pushed the blocksdir-description branch from 80e31ef to ccc27bd Compare January 18, 2019 04:45

maflcko reviewed Feb 12, 2019

View reviewed changes

maflcko modified the milestone: 0.18.0 Feb 19, 2019

maflcko merged commit ccc27bd into bitcoin:master May 13, 2019

kristapsk mentioned this pull request May 13, 2019

Correct description of blocksdir default value #16010

Closed

sangaman deleted the blocksdir-description branch May 13, 2019 19:33

furszy mentioned this pull request Apr 20, 2021

[Test][Misc] Fix libsecp256k1 init for unit test setup + Allow to specify blocks storage directory PIVX-Project/PIVX#2326

Merged

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

doc: Clarify -blocksdir usage #14364

doc: Clarify -blocksdir usage #14364

Uh oh!

Conversation

sangaman commented Oct 2, 2018

Uh oh!

DrahtBot commented Oct 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

promag left a comment • 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!

promag Oct 2, 2018

Choose a reason for hiding this comment

Uh oh!

sangaman commented Oct 2, 2018

Uh oh!

Uh oh!

sangaman commented Dec 18, 2018

Uh oh!

sangaman commented Jan 17, 2019

Uh oh!

Uh oh!

Uh oh!

sangaman commented Jan 18, 2019

Uh oh!

hebasto commented Jan 18, 2019

Uh oh!

maflcko Feb 12, 2019

Choose a reason for hiding this comment

Uh oh!

maflcko commented Feb 12, 2019

Uh oh!

Sjors commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sangaman commented Feb 19, 2019

Uh oh!

hebasto commented Feb 19, 2019

Uh oh!

Sjors commented Feb 20, 2019

Uh oh!

sangaman commented Apr 9, 2019

Uh oh!

maflcko commented May 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

DrahtBot commented Oct 2, 2018 •

edited

Loading

promag left a comment •

edited

Loading

Sjors commented Feb 19, 2019 •

edited

Loading