Drop support for databases repair attempt.

[ytrezq]

This comes from openethereum/openethereum#264.
Currently, if the database is corrupted, kvdb-Rocksdb will attempt to repair it using the code from leveldb.

As the code only works on single column databases, it’s no longer effective and just lur users into possible rescue which won’t happen. This should be removed and replaced with an option to trim the database (from the beginning to the specified block possibly the latest valid block in the case of sst corruption).

[ordian]

Hey @ytrezq

Here are some questions and thoughts:

1. What code works on single column databases?
2. What does number of columns have to do with db corruption and attempting to repair it?
3. What do you by block? Block as in blockchain? kvdb-rocksdb is a generic key-value store and knows nothing about its content.
4. kvdb-rocksdb is a library, you're free to implement a function to open the db and delete it on error if you want.

[ytrezq]

@ordian the problem is kvdb-rocksdb attempt repairs automatically https://github.com/paritytech/parity-common/blob/master/kvdb-rocksdb/src/lib.rs#L326 regardless of the number of columns which results in deleting all columns in the Manifest and thus doing more harm than good.
So at next restart this code is triggered https://github.com/paritytech/parity-common/blob/master/kvdb-rocksdb/src/lib.rs#L337

[ordian]

So you're saying that DB::repair only works for single column databases?
If so, do you have any evidence of that?
By looking at the rocksdb code, it doesn't seem to be the case:

1. parity-common/kvdb-rocksdb/src/lib.rs

   Line 415 in 77ddc33

   DB::repair(&opts, path.as_ref()).map_err(other_io_err)?;

2. rust-rocksdb: https://github.com/rust-rocksdb/rust-rocksdb/blob/2257be1563470181f1776b687750c7159effb740/src/db.rs#L852
3. rocksdb ffi: https://github.com/facebook/rocksdb/blob/b443d24f4d1a2a215e2b176d7c882b5540df6972/db/c.cc#L1841-L1846
4. rocksdb RepairDB: https://github.com/facebook/rocksdb/blob/9d77bf8f7b772495d7b615f9fc6624ed75f8f637/db/repair.cc#L745-L759
5. rocksdb Repairer::Run: https://github.com/facebook/rocksdb/blob/9d77bf8f7b772495d7b615f9fc6624ed75f8f637/db/repair.cc#L186-L236

[ytrezq]

@ordain this is a well known issue upstream. The underlying logic comes from the leveldb era.

Those functions are no longer in use at Facebook.

[ordian]

@ytrezq could you be more specific which issue is it? Do you have a link or a repro test case?
I could only find this limitation: https://github.com/facebook/rocksdb/wiki/RocksDB-Repairer#limitations

If the column family is created recently and not persisted in sst files by a flush, then it will be dropped during the repair process. With this limitation repair would might even damage a healthy db if its column families are not flushed yet.

Which is mentioned in facebook/rocksdb#5073.

The reason I ask is there are tests for RepairDB for multiple columns: https://github.com/facebook/rocksdb/blob/b16655a547c3a44f8dcbe09614ef7ebb8daa83ac/db/repair_test.cc#L306.

And if those functions are no longer in use at Meta, do you happen to know what they use instead?

[ytrezq]

@ordian : Delete the manifest file of a properly shut down https://github.com/paritytech/polkadot database ; let the database to be rebuilt at next run ; Then, https://github.com/paritytech/parity-common/blob/master/kvdb-rocksdb/src/lib.rs#L337 is triggered beacause the database contains only 1 column.

[ordian]

The database won't be repaired if the MANIFEST file is missing:

Backend error: IO error: No such file or directory: While opening a file for sequentially reading: dev/chains/dev/db/full/MANIFEST-000008: No such file or directory

When deleting both CURRENT and MANIFEST-* files, the db opens with no problems.

[ytrezq]

@ordian : by no problems, do you mean it opens immediately or it does attempt to recover first ?

[ordian]

Sorry, my bad. The recovery attempt only happens if CORRUPTED file is present. Indeed running

$ polkadot --validator --dev -d dev --pruning=archive
^Ctrl-C
$ touch dev/chains/dev/db/full/CORRUPTED
$ polkadot --validator --dev -d dev --pruning=archive
Backend error: Invalid argument: Column families not opened: col9, col8, col5, col4, col3, col2, col1, col0

results in a corrupted manifest file. I'll submit a PR to remove the repairer attempt. Thanks for letting us know and bearing with me :)

[ytrezq]

@ordian : better. In the case of a corrupt crc32 on an entry, rewrite the crc or delete the entry so Polkadot or Openethereum can use the database again.

[ytrezq]

@ordian : an even better approach would be to bring multi column supports upstream.

[ordian]

@ytrezq our even better approach is replace RocksDB with https://github.com/paritytech/parity-db long-term ;)