Implement CCoinsViewErrorCatcher::HaveCoin and check disk space periodically
#26331
+23
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sipa
reviewed
Oct 18, 2022
aureleoules
force-pushed
the
2022-10-disk-space-problems-fix
branch
from
2c4d837 to
1527570
Compare
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
sipa
reviewed
Oct 18, 2022
aureleoules
force-pushed
the
2022-10-disk-space-problems-fix
branch
from
1527570 to
ed52e71
Compare
w0xlt
approved these changes
Oct 31, 2022
sipa
reviewed
Sep 29, 2023
Member
|
@willcl-ark you might have some thoughts on writing some sort of test for this? |
Member
|
ACK ed52e71 |
Frank-GER
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 13, 2023
… check disk space periodically ed52e71 Periodically check disk space to avoid corruption (Aurèle Oulès) 7fe537f Implement CCoinsViewErrorCatcher::HaveCoin (Aurèle Oulès) Pull request description: Attempt to fix bitcoin#26112. As suggested by sipa in bitcoin#26112 (comment): > CCoinsViewErrorCatcher, the wrapper class used around CCoinsViewDB that's supposed to detect these problems and forcefully exit the application, has an override for GetCoins. But in CheckTxInputs, HaveInputs is first invoked, which on its turn calls HaveCoin. HaveCoin is implemented in CCoinsViewDB, but not in CCoinsViewErrorCatcher, and thus the disk read exception escapes. > A solution may be to just add an override for HaveCoin in CCoinsViewErrorCatcher. I implemented `CCoinsViewErrorCatcher::HaveCoin` and also added a periodic disk space check that shutdowns the node if there is not enough space left on disk, the minimum here is 50MB. For reviewers, it's possible to saturate disk space to test the PR by creating large files with `fallocate -l 50G test.bin` ACKs for top commit: achow101: ACK ed52e71 w0xlt: Code Review ACK bitcoin@ed52e71 sipa: utACK ed52e71 Tree-SHA512: 456aa7b996023df42b4fbb5158ee429d9abf7374b7b1ec129b21aea1188ad19be8da4ae8e0edd90b85b7a3042b8e44e17d3742e33808a4234d5ddbe9bcef1b78
Member
|
Post-merge concept ACK. Some kind of test coverage would be good. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempt to fix #26112.
As suggested by sipa in #26112 (comment):
I implemented
CCoinsViewErrorCatcher::HaveCoinand also added a periodic disk space check that shutdowns the node if there is not enough space left on disk, the minimum here is 50MB.For reviewers, it's possible to saturate disk space to test the PR by creating large files with
fallocate -l 50G test.bin