Don't delete old db after migration

[dvdplm]

Fixes unwanted shuffling&deletion of the old database after a
migration that merely deletes data in one of the existing database
columns.

[ordian]

The problem with alters_existing still remains, I suggest we remove it completely, it doesn't seem to be used.

[denisgranha]

Just tested this PR migrating from 2.7.2 chaindata and it works.

2020-04-29 09:53:07 UTC Migrating database from version 14 to 15
2020-04-29 10:08:29 UTC Removing accounts existence bloom (209176 keys)
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 10000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 20000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 30000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 40000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 50000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 60000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 70000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 80000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 90000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 100000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 110000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 120000
2020-04-29 10:08:29 UTC Account Bloom entries queued for deletion: 130000
2020-04-29 10:08:41 UTC Deleted 131074 account existence bloom items from the DB
2020-04-29 10:08:41 UTC Migration finished
2020-04-29 10:08:41 UTC Starting OpenEthereum/v3.0.0-alpha.1-nightly-446a806b9-20200428/x86_64-alpine-linux-musl/rustc1.43.0
2020-04-29 10:08:41 UTC Keys path /mnt/ethereum/.mainnet/keys/ethereum
2020-04-29 10:08:41 UTC DB path /mnt/ethereum/.mainnet/chains/ethereum/db/906a34e69aec8c0d
2020-04-29 10:08:41 UTC State DB configuration: fast
2020-04-29 10:08:41 UTC Operating mode: active
2020-04-29 10:08:42 UTC Configured for Ethereum using Ethash engine
2020-04-29 10:08:42 UTC Error reading Ethereum Node Record: Os { code: 2, kind: NotFound, message: "No such file or directory" }
2020-04-29 10:08:43 UTC Updated conversion rate to Ξ1 = US$206.64 (23044448 wei/gas)
2020-04-29 10:08:48 UTC Public node URL: enode://d9d3e6f18edcc2c53a3ec9298a1a716aa677442928193ff5d46e4bef725f4f5133482b9163689eb56a455fe912c518d114742eb04a6fe9560e56f313d38f3bd5@127.0.0.1:30303
2020-04-29 10:09:02 UTC Syncing #9955204 0x7867…9c51 0.00 blk/s 0.0 tx/s 0.0 Mgas/s 98+ 153 Qed #9955458 2/25 peers 102 KiB chain 95 MiB db 39 MiB queue 58 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:07 UTC Syncing #9955204 0x7867…9c51 0.00 blk/s 0.0 tx/s 0.0 Mgas/s 0+ 253 Qed #9955458 2/25 peers 102 KiB chain 95 MiB db 41 MiB queue 88 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:12 UTC Syncing #9955205 0x42a9…d83f 0.20 blk/s 35.0 tx/s 2.0 Mgas/s 0+ 241 Qed #9955458 4/25 peers 120 KiB chain 97 MiB db 39 MiB queue 88 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:17 UTC Syncing #9955205 0x42a9…d83f 0.00 blk/s 0.0 tx/s 0.0 Mgas/s 946+ 559 Qed #9956727 5/25 peers 121 KiB chain 97 MiB db 185 MiB queue 2 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:22 UTC Syncing #9955206 0xa567…f636 0.20 blk/s 41.0 tx/s 2.0 Mgas/s 396+ 1111 Qed #9956727 7/25 peers 341 KiB chain 99 MiB db 190 MiB queue 2 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:27 UTC Syncing #9955206 0xa567…f636 0.00 blk/s 0.0 tx/s 0.0 Mgas/s 0+ 1510 Qed #9956727 6/25 peers 632 KiB chain 99 MiB db 194 MiB queue 2 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:32 UTC Syncing #9955208 0x1815…3b31 0.40 blk/s 33.6 tx/s 2.0 Mgas/s 0+ 1510 Qed #9956727 6/25 peers 667 KiB chain 101 MiB db 194 MiB queue 2 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:37 UTC Syncing #9955209 0x6f48…eafd 0.20 blk/s 42.8 tx/s 2.0 Mgas/s 0+ 1510 Qed #9956727 6/25 peers 667 KiB chain 103 MiB db 194 MiB queue 2 MiB sync RPC: 0 conn, 0 req/s, 0 µs
2020-04-29 10:09:42 UTC Syncing #9955210 0xc61d…c1d2 0.20 blk/s 22.0 tx/s 2.0 Mgas/s 0+ 1510 Qed #9956727 7/25 peers 2 MiB chain 104 MiB db 194 MiB queue 2 MiB sync RPC: 0 conn, 0 req/s, 0 µs

[dvdplm]

I think the migration code in general is problematic. We have deleted the migrations themselves and are left with this weird infrastructure of traits and types that has no use in today’s code. I can rip it out completely ofc and leave nothing for the next guy that has to write a migration.
Maybe it’s better to only have ad-hoc code written once and deleted in the subsequent release. It would definitely lead to code that is easier to follow and reason about.

Specifically though, if there were a migration in the code that does read+process+write data in one or more columns, I think alters_existing does the right thing, or what am I missing?

[ordian]

Specifically though, if there were a migration in the code that does read+process+write data in one or more columns, I think alters_existing does the right thing, or what am I missing?

The fact that after the blooms migration the db sync started from scratch means there is a bug somewhere in the path logic. We didn't use alters_existing migration previously so I don't know if it ever worked.

leave nothing for the next guy that has to write a migration

leaving non-working alters_existing migration for the next guy is kicking the can down the road :P

[dvdplm]

We didn't use alters_existing migration previously so I don't know if it ever worked.

I think you're missing a piece here (my fault): my migration code writes to the source database, but the intention is that alters_existing migrations use two databases: the source for reading and the destination for writing.

This is why I think I misused an alters_existing-type migration. And why I think that a proper alters_existing-type migration might work just fine.

That said I'm very skeptical to keeping migration setup code around that does not have any actual use. But is this the right PR to tackle that?

[ordian]

I think you're missing a piece here (my fault): my migration code writes to the source database, but the intention is that alters_existing migrations use two databases: the source for reading and the destination for writing.

Ah, ok, thanks for the clarification. I stand corrected then :)