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

Always flush block and undo when switching to new file #6948

Merged
merged 1 commit into from Nov 5, 2015

Conversation

@sipa
Copy link
Member

commented Nov 4, 2015

Previously, the undo files weren't being flushed during a reindex because fKnown was set to true in FindBlockPos. That is the correct behaviour for block files as they aren't being touched, but undo files are
touched.

This changes the behaviour to always flush when switching to a new file (even for block files, though that isn't really necessary).

Intended to fix #6923.

@sipa

This comment has been minimized.

Copy link
Member Author

commented Nov 4, 2015

@jonasschnelli @laanwj You may want to test this.

@sipa sipa force-pushed the sipa:fixflush branch Nov 4, 2015

Always flush block and undo when switching to new file
Previously, the undo weren't being flushed during a reindex because
fKnown was set to true in FindBlockPos. That is the correct behaviour
for block files as they aren't being touched, but undo files are
touched.

This changes the behaviour to always flush when switching to a new file
(even for block files, though that isn't really necessary).

@sipa sipa force-pushed the sipa:fixflush branch to 22e7807 Nov 4, 2015

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 5, 2015

I expect that this fixes the issue. #6923 seemed to be a case of a truncated undo file (there was a strangely-sized old undo file and a very small new one). Although I don't fully remember if it happened during -reindex. May well be the case.

Will test.

if (!fKnown) {
LogPrintf("Leaving block file %i: %s\n", nFile, vinfoBlockFile[nFile].ToString());
}
FlushBlockFile(!fKnown);

This comment has been minimized.

Copy link
@morcos

morcos Nov 5, 2015

Member

So on a reindex this means undo files still won't be truncated I suppose?

@morcos

This comment has been minimized.

Copy link
Member

commented Nov 5, 2015

utACK

Would an alternative option be to at the end of the reindex loop, go through and flush all the undo files. It seems if you crash during a reindex, you don't care about the state of the undo files, its only if you were to crash after reindex before they were flushed that you would have a problem?

Such alternative option would potentially have problems in the event of new blocks being written during reindex, but thats already a problem, see #6691.

@jgarzik

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2015

tested ACK

@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Nov 5, 2015

Tested ACK.

@gmaxwell gmaxwell modified the milestones: 0.11.0, 0.12.0 Nov 5, 2015

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 5, 2015

It seems if you crash during a reindex, you don't care about the state of the undo file

I don't understand your reasoning here. The point of this PR is to avoid corruption in the undo files when there is a crash during reindex. It can then pick up where it left off next time the application is launched.

@laanwj laanwj merged commit 22e7807 into bitcoin:master Nov 5, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Nov 5, 2015
Merge pull request #6948
22e7807 Always flush block and undo when switching to new file (Pieter Wuille)
@paveljanik

This comment has been minimized.

Copy link
Contributor

commented Nov 6, 2015

This change brought a new compiler warning:

main.cpp:2554:15: warning: comparison of integers of different signs: 'unsigned int' and 'int' [-Wsign-compare]
    if (nFile != nLastBlockFile) {
        ~~~~~ ^  ~~~~~~~~~~~~~~
1 warning generated.
sipa added a commit that referenced this pull request Nov 6, 2015
Always flush block and undo when switching to new file
Previously, the undo weren't being flushed during a reindex because
fKnown was set to true in FindBlockPos. That is the correct behaviour
for block files as they aren't being touched, but undo files are
touched.

This changes the behaviour to always flush when switching to a new file
(even for block files, though that isn't really necessary).

Rebased-From: 22e7807
Github-Pull: #6948
@harding harding referenced this pull request Nov 8, 2015
0 of 3 tasks complete
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Dec 8, 2015
Always flush block and undo when switching to new file
Previously, the undo weren't being flushed during a reindex because
fKnown was set to true in FindBlockPos. That is the correct behaviour
for block files as they aren't being touched, but undo files are
touched.

This changes the behaviour to always flush when switching to a new file
(even for block files, though that isn't really necessary).

Rebased-From: 22e7807
Github-Pull: bitcoin#6948
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.