Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail AppInitMain if either disk space check fails #15124

Merged
merged 1 commit into from Jan 9, 2019

Conversation

@Empact
Copy link
Member

@Empact Empact commented Jan 7, 2019

Rather than both.

Introduced in 386a6b6, #12653

@Empact Empact changed the title Fail if either disk space check fails Fail AppInitMain if either disk space check fails Jan 7, 2019
@promag
Copy link
Member

@promag promag commented Jan 7, 2019

I think it's reasonable to split the expression to log which directory needs more space.

@jimpo
Copy link
Contributor

@jimpo jimpo commented Jan 8, 2019

utACK 11e85ca

src/init.cpp Outdated Show resolved Hide resolved
@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Jan 8, 2019

utACK 11e85ca

@Empact Empact force-pushed the Empact:disk-space-check branch 2 times, most recently Jan 8, 2019
src/init.cpp Outdated Show resolved Hide resolved
@jamesob
Copy link
Member

@jamesob jamesob commented Jan 8, 2019

Agree with @promag: while we're in the neighborhood, we should specify which disk is full.

@Empact Empact force-pushed the Empact:disk-space-check branch Jan 9, 2019
Rather than both.

Introduced in 386a6b6
@Empact Empact force-pushed the Empact:disk-space-check branch to ba8c8b2 Jan 9, 2019
@Empact
Copy link
Member Author

@Empact Empact commented Jan 9, 2019

Updated to address feedback.

@promag
Copy link
Member

@promag promag commented Jan 9, 2019

utACK ba8c8b2, thanks!

@laanwj
Copy link
Member

@laanwj laanwj commented Jan 9, 2019

utACK ba8c8b2

@laanwj laanwj merged commit ba8c8b2 into bitcoin:master Jan 9, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Jan 9, 2019
ba8c8b2 Fail if either disk space check fails (Ben Woosley)

Pull request description:

  Rather than both.

  Introduced in 386a6b6, #12653

Tree-SHA512: 24765dd3c62b742c491d7d9a751917c2ce6f3819a8764a7725ce84910ef69bffca07f4c0dfbeed8c4f978a12c4b04a2ac3b8c2ff59602330a8a3e8a68878c41b
@Empact Empact deleted the Empact:disk-space-check branch Jan 9, 2019
@MarcoFalke
Copy link
Member

@MarcoFalke MarcoFalke commented Jan 9, 2019

Could add a test, if possible somehow?

@Empact
Copy link
Member Author

@Empact Empact commented Jan 10, 2019

I don't know of any way to set up the scenario as-is - could refactor for more testability with stubbing/mocking.

random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request May 1, 2021
… 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
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jun 26, 2021
ba8c8b2 Fail if either disk space check fails (Ben Woosley)

Pull request description:

  Rather than both.

  Introduced in 386a6b6, bitcoin#12653

Tree-SHA512: 24765dd3c62b742c491d7d9a751917c2ce6f3819a8764a7725ce84910ef69bffca07f4c0dfbeed8c4f978a12c4b04a2ac3b8c2ff59602330a8a3e8a68878c41b
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
ba8c8b2 Fail if either disk space check fails (Ben Woosley)

Pull request description:

  Rather than both.

  Introduced in 386a6b6, bitcoin#12653

Tree-SHA512: 24765dd3c62b742c491d7d9a751917c2ce6f3819a8764a7725ce84910ef69bffca07f4c0dfbeed8c4f978a12c4b04a2ac3b8c2ff59602330a8a3e8a68878c41b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants