Remove block file location upgrade code#9822
Conversation
|
concept ACK |
|
Related: could we just also remove that part? The comment says |
|
@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 45d7ee2 |
|
utACK fb678229429364f9a0a93add93e21fe444ae0fcb |
luke-jr
left a comment
There was a problem hiding this comment.
Wow, I forgot they were even somewhere else before! utACK
src/init.cpp
Outdated
There was a problem hiding this comment.
I'll push a commit soon that removes this line again, which creates the directory in CDBWrapper where it belongs.
|
I think an unrelated commit snuck into here accidentally: 4cce79c |
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.
|
It was not unrelated, I needed it to cleanly remove the full block and not leave an
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, |
|
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 ***@***.***> 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.
|
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. |
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)? |
For such old releases, the procedure should be: backup the wallet and nuke the rest of the data directory. 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. |
|
utACK 4b183d3 |
My default expectation would be compatibility between upgrades. Any non-compatibility issues should be documented in release notes.
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. |
4b183d3 Remove block file location upgrade code (Marko Bencun) Tree-SHA512: fac1fce95341e0df645c08c7e794195b22b54df08826aa8728f2f97aede1e42f724f8133781b97f836d4a392d044d08c846bce471a6b478582014f8be501a712
|
@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. |
4b183d3 Remove block file location upgrade code (Marko Bencun) Tree-SHA512: fac1fce95341e0df645c08c7e794195b22b54df08826aa8728f2f97aede1e42f724f8133781b97f836d4a392d044d08c846bce471a6b478582014f8be501a712
Remove dead code Closes #4012 Equivalent to bitcoin/bitcoin/pull/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.