doc: Clarify -blocksdir usage #14364

sangaman commented at 5:16 AM on October 2, 2018: contributor

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.

fanquake added the label Docs on Oct 2, 2018

DrahtBot commented at 7:24 AM on October 2, 2018: member

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.

in doc/man/bitcoin-qt.1:42 in 1db7727c65 outdated

  38 | @@ -39,7 +39,7 @@ Extra transactions to keep in memory for compact block reconstructions
  39 |  .HP
  40 |  \fB\-blocksdir=\fR<dir>
  41 |  .IP
  42 | -Specify blocks directory (default: <datadir>/blocks)
  43 | +Specify blocks root directory for .dat files (default: <datadir>)

promag commented at 9:13 AM on October 2, 2018:

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

promag commented at 9:16 AM on October 2, 2018: member

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.

sangaman commented at 12:57 PM on October 2, 2018: contributor

@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?

sangaman force-pushed on Oct 2, 2018

in src/init.cpp:336 in 5ff869b24b outdated

 332 | @@ -333,7 +333,7 @@ void SetupServerArgs()
 333 |      gArgs.AddArg("-version", "Print version and exit", false, OptionsCategory::OPTIONS);
 334 |      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);
 335 |      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);
 336 | -    gArgs.AddArg("-blocksdir=<dir>", "Specify blocks directory (default: <datadir>/blocks)", false, OptionsCategory::OPTIONS);
 337 | +    gArgs.AddArg("-blocksdir=<dir>", "Specify blocks root directory for .dat files (default: <datadir>)", false, OptionsCategory::OPTIONS);

hebasto commented at 4:53 PM on October 2, 2018:

Could reference block data files as described in doc/files.md: blocks/*.dat ?

sangaman commented at 5:31 PM on October 2, 2018:

Good idea but I'm not sure the best way to phrase that... Specify root directory for blocks/*.dat files...?

jonasschnelli commented at 5:47 AM on December 18, 2018:

I would reed this as the .dat files will be written to the "root directory" (but they get written into "<root>/blocks"). I think mentioning the "blocks" subdirectory (that will e created) makes sense.

sangaman commented at 6:22 AM on December 18, 2018:

I see. One of the tricky things is that the blocks subdirectory could be nested within a testnet3 folder when running testnet. How about:

Specify root for blocks subdirectory to store .dat files

hebasto commented at 5:05 AM on January 17, 2019:

"Specify directory to hold blocks subdirectory for *.dat files (default: <datadir>)"

sangaman commented at 5:07 AM on December 18, 2018: contributor

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

sangaman force-pushed on Jan 17, 2019

sangaman commented at 2:17 AM on January 17, 2019: contributor

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.

in src/httpserver.cpp:281 in ef5d858e24 outdated

 277 | @@ -278,7 +278,7 @@ static void http_reject_request_cb(struct evhttp_request* req, void*)
 278 |      evhttp_send_error(req, HTTP_SERVUNAVAIL, nullptr);
 279 |  }
 280 |  
 281 | -/** Event dispatcher thread */
 282 | +/** Event dispatcher thread */ x

hebasto commented at 5:02 AM on January 17, 2019:

Mistype?

sangaman commented at 4:45 AM on January 18, 2019:

Oops yes, sorry about that. Thanks!

hebasto changes_requested

sangaman force-pushed on Jan 18, 2019

doc: Clarify -blocksdir usage

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.

ccc27bdcd2

sangaman force-pushed on Jan 18, 2019

sangaman commented at 4:45 AM on January 18, 2019: contributor

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

hebasto commented at 8:07 AM on January 18, 2019: member

utACK ccc27bdcd2d91fe2c023ad004019d5b970f21dbf

in src/init.cpp:336 in ccc27bdcd2

 332 | @@ -333,7 +333,7 @@ void SetupServerArgs()
 333 |      gArgs.AddArg("-version", "Print version and exit", false, OptionsCategory::OPTIONS);
 334 |      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);
 335 |      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);
 336 | -    gArgs.AddArg("-blocksdir=<dir>", "Specify blocks directory (default: <datadir>/blocks)", false, OptionsCategory::OPTIONS);
 337 | +    gArgs.AddArg("-blocksdir=<dir>", "Specify directory to hold blocks subdirectory for *.dat files (default: <datadir>)", false, OptionsCategory::OPTIONS);

MarcoFalke commented at 8:49 PM on February 12, 2019:

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

MarcoFalke commented at 8:49 PM on February 12, 2019: member

ACK

Sjors commented at 1:51 PM on February 19, 2019: member

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 commented at 2:12 PM on February 19, 2019: contributor

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 commented at 6:07 PM on February 19, 2019: member

@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?

MarcoFalke added this to the milestone 0.18.0 on Feb 19, 2019

MarcoFalke removed this from the milestone 0.18.0 on Feb 19, 2019

Sjors commented at 8:53 AM on February 20, 2019: member

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 commented at 4:20 PM on April 9, 2019: contributor

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.

MarcoFalke referenced this in commit 667a861741 on May 13, 2019

MarcoFalke merged this on May 13, 2019

MarcoFalke closed this on May 13, 2019

MarcoFalke commented at 4:48 PM on May 13, 2019: member

Longer term imo we should move <datadir>/blocks/index to <datadir>/indexes/blocks, and make -blocksdir a network specific setting. @Sjors Create an issue about that?

sangaman deleted the branch on May 13, 2019

sidhujag referenced this in commit 627c65b09b on May 14, 2019

random-zebra referenced this in commit edfaec63c2 on May 1, 2021

PastaPastaPasta referenced this in commit c0a6e3328e on Jun 27, 2021

PastaPastaPasta referenced this in commit a947a1ac89 on Jun 28, 2021

PastaPastaPasta referenced this in commit 5ad8f68ebb on Jun 29, 2021

PastaPastaPasta referenced this in commit 82f971760f on Jul 1, 2021

PastaPastaPasta referenced this in commit ace7367e89 on Jul 1, 2021

PastaPastaPasta referenced this in commit e70305c97a on Jul 8, 2021

PastaPastaPasta referenced this in commit 7dee7786b7 on Jul 10, 2021

DrahtBot locked this on Dec 16, 2021