Page MenuHomePhabricator

Add -datadir option to allow specification of seeder's data directory
Changes PlannedPublic

Authored by nakihito on May 4 2020, 22:40.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

Currently, the seeder will write its database files to the current
working directory. This is clumsy and can lead to starting the seeder
up without its database if it is started up from a different location.

This change will not only create a default directory for the seeder's
database files (and any future files it writes/reads) but also allow the
user to define where that data directory is.

Test Plan
ninja
./bitcoin-seeder --help

Verify help text for new option

mkdir dirtest
./bitcoin-seeder -host=localhost -ns=blah.bitframe.org -port=8888 -mbox=info@bitframe.org --datadir=dirtest

Let the seeder run for a while and then verify the files dnsseed.dat, dnsseed.dump, and dnsstats.log are written to the dirtest/seeder/ directory. Also verify the relative datadir warning shows up.

./bitcoin-seeder -host=localhost -ns=blah.bitframe.org -port=8888 -mbox=info@bitframe.org

Verify the files dnsseed.dat, dnsseed.dump, and dnsstats.log are written to the ~/.bitcoin/seeder/ directory.

Confirm the above works for testnet and that the files are written to <path>/seeder/testnet3/<file>.
Also confirm that the above works for regtest and that the files are written to <path>/seeder/regtest/<file>.

rm -r dirtest
./bitcoin-seeder -host=localhost -ns=blah.bitframe.org -port=8888 -mbox=info@bitframe.org --datadir=dirtest

Verify that the seeder errors out with the error "Specified data directory <path>/dirtest/ does not exist.".

mv ~/.bitcoin/ ~/.bubitcoin/
./bitcoin-seeder -host=localhost -ns=blah.bitframe.org -port=8888 -mbox=info@bitframe.org

Verify that the seeder operates as usual without error and that ~/.bitcoin/seeder/ directory is created and
eventually the files are written there.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
SeederWriteOption
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10598
Build 19006: Default Diff Build & Tests
Build 19005: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.May 4 2020, 22:40
nakihito requested review of this revision.May 4 2020, 22:40
nakihito retitled this revision from Add -seederdir option to allow selection of seeder's data directory to Add -seederdir option to allow specification of seeder's data directory.May 4 2020, 22:41
deadalnix requested changes to this revision.May 5 2020, 02:54
deadalnix added a subscriber: deadalnix.

This is introducing dependencies ont he seeder in the base libraries, this makes no sense whatsoever.

src/util/system.h
92 ↗(On Diff #19425)

No.

This revision now requires changes to proceed.May 5 2020, 02:54

Moved seeder specific functions out of regular node code.

deadalnix requested changes to this revision.May 7 2020, 14:40

It seems like there is already a machinery to select the folder in which business happen, why does this needs to be all duplicated with a different name?

src/seeder/main.cpp
99

Isn't this an error?

This revision now requires changes to proceed.May 7 2020, 14:40

Error outputs to std::cerr and changed GetSeederDir() to call GetDefaultDataDir() directly when -seederdir is not set.

It seems like there is already a machinery to select the folder in which business happen, why does this needs to be all duplicated with a different name?

I'm assuming you are talking about the -datadir related functions in util/system.cpp, those functions do not allow the seeder to create its own subdirectory. If I were to use those then all of the seeder files would be mixed in with the regular node files.

I'm assuming you are talking about the -datadir related functions in util/system.cpp, those functions do not allow the seeder to create its own subdirectory. If I were to use those then all of the seeder files would be mixed in with the regular node files.

You can use a subdirectory if you think that it is important. But ultimately, this makes no sense because I can run the seeder with one datadir and the node with another if I want to.

Removed added files/functions. Option uses pre-existing -datadir related code.

deadalnix requested changes to this revision.May 25 2020, 16:23
deadalnix added inline comments.
src/seeder/main.cpp
99 ↗(On Diff #20372)

The diff still mention seederdir.

100 ↗(On Diff #20372)

Wouldn't it be preferable to always append a seedersubfolder to the datadir?

This revision now requires changes to proceed.May 25 2020, 16:23
nakihito retitled this revision from Add -seederdir option to allow specification of seeder's data directory to Add -datadir option to allow specification of seeder's data directory.May 26 2020, 17:38
nakihito edited the test plan for this revision. (Show Details)

Added some output that specifies the default directory and the directory in use on start up.

src/seeder/main.cpp
100 ↗(On Diff #20372)

I do not think so. The help text says that the option specifies the directory that is read from and/or written to. Right now, the option is flexible in that a user can specify any directory and the seeder will read and write from there. This gives users more control if, for example, someone wants to keep the seeder directory separate from the regular node directory. If a user specifies -datadir=~/.seeder/ for example, it is expected that reads from and writes to the specified directory ~/.seeder/ and not ~/.seeder/seeder/. This also more closely matches the behavior of the node software's -datadir option.