implement CAutoFile via std::fstream #3277

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:use_fstream changing 8 files +206 −164

Diapolo commented at 11:12 AM on November 18, 2013: none
- implement CAutoFile via std::fstream and use it in the code
- unify log/error messages for serializing/deserializing exceptions
- add debug and benchmark messages for writing/reading block/undo files
- remove boost::path member from CAddrDB class
- add new helper functions GetBlockFile() and GetUndoFile()
If considered usefull, I'm open to feedback and comments. I've been using this for ages with my local build and never had any problems with it. I know that currently there is no "flush to disk" when downloading blocks, but perhaps this pull can help investigating the Mac corruption problems.
brandondahler commented at 6:43 AM on November 30, 2013: contributor
I see no problems with the implementation, but I would suggest changing
```
class CAutoFile
```
to
```
class CAutoFile : protected std::fstream
```
Making this change will require the removal of the fstream file member variable, likewise that will cause you to have to replace all instances of "file." with "fstream::" .

This allows the underlying fstream to be used in spite of being wrapped by a CAutoFile, if an only if it is referenced as a fstream and not a CAutoFile. In the end you could add it to an array of fstreams, while ensuring explicit CAutoFile references are forced to use the RAII function versions.
Diapolo commented at 1:36 PM on December 1, 2013: none

@brandondahler Thanks for that suggestion, I'm going to update this pull. Have you got a nice idea, how I could add back the FileCommit() whilst beeing compatible to sdt::fstream? I guess a ::flush() is not enough...

implement CAutoFile via std::fstream

- implement CAutoFile via std::fstream and use it in the code
- unify log/error messages for serializing/deserializing exceptions
- add debug and benchmark messages for writing/reading block/undo files
- remove boost::path member from CAddrDB class
- add new helper functions GetBlockFile() and GetUndoFile()

5ee9baec85

BitcoinPullTester commented at 2:36 PM on December 1, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/5ee9baec85bcc972853af954967ee6b24bdeff2c for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
brandondahler commented at 4:53 PM on December 1, 2013: contributor
@Diapolo I've researched this for some other changes to move everything to fstream. It appears that there is no easy way to get the fileno from a c++ fstream, and likewise there's no way to directly request the OS flush the data to the disk.

The best bet that I've found is doing:
```
::rdbuf().pubsync()
```
That being said, I question the need to request/force the OS to flush the cache buffers to disk. It seems like we are trying to creep into the operating system's responsibilities instead of accepting that the OS may or may not follow our requests at the time that we make them.
sipa commented at 8:22 PM on December 1, 2013: member

Being able to flush block files to disk is essential for consistency of on-disk structures (you don't want to make a database update that refers to a block being on disk before knowing that block is actually on disk).
jgarzik commented at 12:08 AM on December 13, 2013: contributor

This does not look like an improvement.
laanwj commented at 4:22 AM on December 13, 2013: member

I have a slight preference of the low-level file functions to fstream implementations here, because the warts of those are a little bit more apparent to me than those of particular C++ libraries.

And this change did not help with fixing the MacOSX corruption either.
laanwj closed this on Dec 13, 2013
Diapolo commented at 7:42 AM on December 13, 2013: none

AFAIK no one ever tried this anyway... @brandondahler Isn't ::rdbuf().pubsync() for reading from files?
Diapolo deleted the branch on Jan 30, 2014
Bushstar referenced this in commit 91b4a38398 on Apr 8, 2020
DrahtBot locked this on Sep 8, 2021

Contributors