Remove block file location upgrade code #9822

An effort to reduce the size of AppInitMain().

The removed code upgrades the location of the block files when upgrading to 0.8. 0.8 seems to be the oldest version still in use.

concept ACK

Related: could we just also remove that part? The comment says Upgrading to 0.8, and according to https://bitnodes.21.co/nodes/, there is no Core node running older than that.

@benma I believe it may be impossible or at least very nontrivial to get a 0.7 node to sync at this point, so i'd be in favor of removing that piece of logic.

Thanks @sipa. Pushed a new commit that removes the code.

Yes, let's drop this for 0.15. utACK https://github.com/bitcoin/bitcoin/pull/9822/commits/45d7ee25f1bfaff7d65a248116e401a4a13d0608

utACK fb678229429364f9a0a93add93e21fe444ae0fcb

Wow, I forgot they were even somewhere else before! utACK

I think an unrelated commit snuck into here accidentally: https://github.com/bitcoin/bitcoin/pull/9822/commits/4cce79cc137d27900e1ceebe8e9bfc5d1befb302

It was not unrelated, I needed it to cleanly remove the full block and not leave an

boost::filesystem::create_directories(GetDataDir() / "blocks");

In any case, I removed the second commit and will make a new PR for that :)

I am a bit confused about the CI failure. Locally, test/test_bitcoin passes, and I am almost sure that the same version (but a previous commit) passed CI tests before. Might there be a problem with the CI?

We should include an upgrade compatibility note in the V0.15 release notes. Something like:

Upgrade to V0.15 is supported from V0.8 and higher. To upgrade from V0.7 and lower, you must first upgrade to V0.14, and then upgrade to V0.15.

To upgrade from V0.7 and lower, you must first upgrade to V0.14, and then upgrade to V0.15.

This is not required. Doing so has the only advantage that you don't have to download the blockchain again and that there are no duplicate block files in the datadir after upgrade.

same version (but a previous commit) passed CI tests

If the same commit passed review and travis, you might want to force push that instead, to not invalidate the results.

On Wed, Feb 22, 2017 at 3:27 PM, Marco Falke marco.falke@tum.de wrote:

To upgrade from V0.7 and lower, you must first upgrade to V0.14, and then upgrade to V0.15.

This is not required. Doing so has the only advantage that you don't have to download the blockchain again and that there are no duplicate block files in the datadir after upgrade.

Doing so has the only advantage that you don't have to download the blockchain again and that there are no duplicate block files in the datadir after upgrade.

Which is currently >100GB of duplicate data that the user has no way of removing except for going into the datadir and manually deleting. Why not just add a note to the release notes saying "don't upgrade directly from <=0.7 to >=0.15?

My take on that is that it is unlikely anyone would actually upgrade from <=0.7. It does not look like anybody is using it, and as @sipa mentioned, it would be impossible or very hard to run a <=0.7 client in the first place.

If the same commit passed review and travis, you might want to force push that instead, to not invalidate the results.

In that one I accidentally had some trailing whitespace. Wouldn't force-pushing it again re-run the CI?

Can I retrigger CI (magic comment or a UI button somewhere)?

Which is currently >100GB of duplicate data that the user has no way of removing except for going into the datadir and manually deleting. Why not just add a note to the release notes saying "don't upgrade directly from <=0.7 to >=0.15?

For such old releases, the procedure should be: backup the wallet and nuke the rest of the data directory. The configuration options changed, the layout of the data directory changed (multiple times), there can be no assumption of compatibility besides the wallet.

It should work but yes you'd be wasting some space. Not that much, mind you, as said 0.7 can't sync that far up the chain before getting stuck anyway.

I don't think this needs mentioning in the release notes: Realistically, no one is using such an old version anymore. Check the node stats.

Can I retrigger CI (magic comment or a UI button somewhere)?

Sure. But we should first figure out what is broken. It doesn't seem to work very well on the branches either (#9825, #9826) to it's almost certainly not related to your change.

utACK 4b183d3

there can be no assumption of compatibility besides the wallet.

My default expectation would be compatibility between upgrades. Any non-compatibility issues should be documented in release notes.

Realistically, no one is using such an old version anymore. Check the node stats.

We shouldn't make decisions based on what we see in node stats. We have a limited view of what happens in the network and what users are doing.

I'm actually not that concerned about this particular issue. I just think it's good practice (and good courtesy to users) to note upgrade compatibility issues in release notes.

How are those things usually resolved here? I don't think the release note is necessary, but I won't stand in the way of it either.

Let's leave writing a note for the release notes, if really necessary, to someone that insists on it, not going to block this pull on that.

@laanwj I'm happy to write the upgrade advice for the release notes. Just let me know where I need to do that.

@jnewbery Release notes for 0.15.0 are ready for amending. You can do it anytime prior to 0.15 branch off of master.