Allow Blockstore to open unknown columns

[steviez]

Problem

As we develop new features / modifications, we occasionally need to introduce new columns to the Blockstore. Adding new columns create a compatibility break due to a requirement that all columns present in the database must be opened. That is, if a new version introduces a new column, older software that is unaware of the column is no longer able to open the blockstore. We have gotten around this in the past by backporting a "stub" PR for new columns to older versions. This is annoying, and only creates compatibility for the one or two versions that we decide to backport to.

Summary of Changes

In order to avoid the above problem, add logic to the rocksdb open code that detects all of the columns in the database. Then, compare this to the list of "known" columns, and create descriptors for the detected columns that were unknown.

Testing

I have a unit test that exercises this; however, I also tested manually with a blockstore modified with master (which recently had a new column from #33979) and v1.17 (which is unaware of the new column).

With v1.17, the blockstore can be opened if the access mode is NOT Primary (ie read-only). So, the read-only solana-ledger-tool operations would still work. However, trying to open the database with Primary (which is what solana-validator needs), an error is encountered:

...
[2023-11-20T06:33:03.531947483Z INFO  solana_ledger::blockstore] Opening database at "/home/sol/ledger/rocksdb"
Failed to open blockstore at "/home/sol/ledger/": Error { message: "Invalid argument: Column families not opened: merkle_root_meta" }

Then, with my change in place:

...
[2023-11-20T06:37:08.103438353Z INFO  solana_ledger::blockstore] Opening database at "/home/sol/ledger/rocksdb"
[2023-11-20T06:37:08.106858572Z INFO  solana_ledger::blockstore_db] Detected unknown column default, opening column with basic options
[2023-11-20T06:37:08.106870414Z INFO  solana_ledger::blockstore_db] Detected unknown column merkle_root_meta, opening column with basic options

Funny enough, it complains about us not having a descriptor for default, but that column must have special handling as we've never created a descriptor for that column. So, I added handling for that in a later commit

[codecov]

Codecov Report

Merging #34174 (b5e3797) into master (e832765) will increase coverage by 0.0%.
Report is 10 commits behind head on master.
The diff coverage is 100.0%.

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #34174    +/-   ##
========================================
  Coverage    81.9%    81.9%            
========================================
  Files         819      819            
  Lines      219350   219417    +67     
========================================
+ Hits       179698   179804   +106     
+ Misses      39652    39613    -39     

[steviez]

To answer my own question, the rust-rocksdb crate auto handles opening the default column for us:
https://github.com/rust-rocksdb/rust-rocksdb/blob/1c6ea8f81a2921594f05b2aae4f7cbd40ac89655/src/db.rs#L618-L623

            // Always open the default column family.
            if !cfs_v.iter().any(|cf| cf.name == DEFAULT_COLUMN_FAMILY_NAME) {
                cfs_v.push(ColumnFamilyDescriptor {
                    name: String::from(DEFAULT_COLUMN_FAMILY_NAME),
                    options: Options::default(),
                });
            }

[CriesofCarrots]

Looks good! Just nits and minor question.

[steviez]

Rebased to tip of master for good measure too

[CriesofCarrots]

lgtm!

[yhchiang-sol]

Thank you for working on this. This is one of the compatibility issues in my mind that I haven't had a chance to work on.

This PR solves the missing CF case when we revert to an older version where the new CF created in the newer version doesn't exist in the older version. The PR looks good to me.

One thing that might be a bit tricky is the CF options change under the same CF.
From RocksDB's consistency, it's better to use the CF options stored in the DB. However, from the reversion's point of view, using the CF options crafted in the older code makes more sense, which is what this PR uses.

Both options have some use cases in different situations, but this would be something to keep in mind when we have CF options change.

[steviez]

One thing that might be a bit tricky is the CF options change under the same CF. From RocksDB's consistency, it's better to use the CF options stored in the DB. However, from the reversion's point of view, using the CF options crafted in the older code makes more sense, which is what this PR uses.

Both options have some use cases in different situations, but this would be something to keep in mind when we have CF options change.

Hmm, I see your point and that is an interesting dilemma. We haven't had change to settings in a while, but I could certainly see how this could be problematic. Something like changing memtable size is probably fine; however, changing the size (reducing) per level could be problematic, and I'm not sure how Rocks would handle that (massive compaction when opening?).

Even if we don't explicitly change the settings, our custom settings that we apply to every column certainly differ from the default that I used to open unknown columns with. I would hope that by disabling compaction and not reading/writing the unknown column, that columns are left as-is

In any, going to push this PR but a good item to think about for the future

[mergify]

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.