How to migrate

[sehkone]

@minshao

Could you let me know how I add another migration in this part? I just added a function migrate_0.4_to_0.5 but couldn't figure out what I should do with it in this.

    let migration = vec![(
        VersionReq::parse(">=0.2,<0.5.0-alpha").expect("valid version requirement"),
        Version::parse("0.4.0").expect("valid version"),
        migrate_0_2_to_0_3,
    )];
    while let Some((_req, to, m)) = migration
        .iter()
        .find(|(req, _to, _m)| req.matches(&version))
    {
        m(data_dir, backup_dir)?;
        version = to.clone();
        if compatible.matches(&version) {
            create_version_file(&backup).context("failed to update VERSION")?;
            return create_version_file(&data).context("failed to update VERSION");
        }
    }

[minshao]

  let version_from = "0.4"; // this is the version that we'd like to migrate from
  let version_to = "0.5";// this is the version that we'd like to migrate to
  let version_to_next = "0.6.0-alpha"; // the upper boundary so that we are covering all "{version_to}.*"

  let migration = vec![(
        VersionReq::parse(">=0.2,<0.5.0-alpha").expect("valid version requirement"),
        Version::parse("0.4.0").expect("valid version"),
        migrate_0_2_to_0_3,
    ), (
       VersionReq::parse(format!(">={version_from},<{version_to_next}")).expect("valid version requirement"),
       Version::parse(format!("{version_to}")).expect("valid version"),
       migrate_FROM_to_TO, // migration function name, `migrate_0_4_to_0_5` in this case,
 )];

With this setting, it should migrate. Let me know if there's any problem~

[sehkone]

I got this.

error[E0308]: mismatched types
  --> src/migration.rs:64:13
   |
64 |             migrate_0_4_to_0_5, // migration function name, `migrate_0_4_to_0_5` in this case,
   |             ^^^^^^^^^^^^^^^^^^ expected fn item, found a different fn item
   |
   = note: expected fn item `fn(_, _) -> std::result::Result<_, _> {migrate_0_2_to_0_3::<_>}`
              found fn item `fn(_, _) -> std::result::Result<_, _> {migrate_0_4_to_0_5::<_>}`
   = note: different fn items have unique types, even if their signatures are the same
   = help: consider casting both fn items to fn pointers using `as fn(_, _) -> std::result::Result<(), anyhow::Error>`

[sehkone]

Is this what you expected me to do?

    let migration = vec![
        (
            VersionReq::parse(">=0.2,<0.5.0-alpha").expect("valid version requirement"),
            Version::parse("0.4.0").expect("valid version"),
            migrate_0_2_to_0_3 as fn(_, _) -> Result<()>,
        ),
        (
            VersionReq::parse(&format!(">={version_from},<{version_to_next}"))
                .expect("valid version requirement"),
            Version::parse(&format!("{version_to}")).expect("valid version"),
            migrate_0_4_to_0_5 as fn(_, _) -> Result<()>,
        ),
    ];

[minshao]

It should work, and the following might be simpler~

  let migration: Vec<(_, _, fn(_, _) -> Result<_, _>)> = vec![(
        VersionReq::parse(">=0.2,<0.5.0-alpha").expect("valid version requirement"),
        Version::parse("0.4.0").expect("valid version"),
        migrate_0_2_to_0_3,
    ), (
       VersionReq::parse(format!(">={version_from},<{version_to_next}")).expect("valid version requirement"),
       Version::parse(format!("{version_to}")).expect("valid version"),
       migrate_0_4_to_0_5,
 )];

[sehkone]

Thank you for your help so much. I modified the code as you told.

[minshao]

Happy that I could be of help~

[sehkone]

@minshao

The current code writes like the below,

let migration = vec![(
        VersionReq::parse(">=0.2,<0.5.0-alpha").expect("valid version requirement"),
        Version::parse("0.4.0").expect("valid version"),
        migrate_0_2_to_0_3,
    )];

But, according to what you explained to me, should it be modified to the following?

let migration = vec![(
        VersionReq::parse(">=0.2,<0.4.0-alpha").expect("valid version requirement"),
        Version::parse("0.3.0").expect("valid version"),
        migrate_0_2_to_0_3,
    )];

[minshao]

The migration function is to upgrade the rocksdb from 0.2 to 0.3, however, in 0.4, review rocksdb doesn't have any breaking changes, the VersionReq upper bound then become 0.5.0-alpha, and the version it migrates to become 0.4. It was to avoid adding a dummy function to upgrade from 0.3 to 0.4. That was the reasoning behind the current code.

The loop below is used to update the current database using the corresponding migration function. The function corresponds to req (">=0.2,<0.5.0-alpha")and to ("0.4.0") as defined earlier. The condition that current db version satisfies req must be met. And after executing the migration function, the db version will become to("0.4.0"). And then some migration function provided in migration should correspond to a req include version "0.4.0" so that the migration process continues until the most recent version.

while let Some((_req, to, m)) = migration
        .iter()
        .find(|(req, _to, _m)| req.matches(&version))
    {
        m(data_dir, backup_dir)?;
        version = to.clone();
        if compatible.matches(&version) {
            create_version_file(&backup).context("failed to update VERSION")?;
            return create_version_file(&data).context("failed to update VERSION");
        }
    }

In migration, migration.iter().map(|(req, _to, _m)| req).collect() should provide a consecutive version requirement where migration.iter().map(|(_req, to, _m)| to).collect() should be the ending version number of migration.iter().map(|(req, _to, _m)| req).collect().

To conclude everything above, with current design, we could either include 0.4 either as is in:

vet![(
        VersionReq::parse(">=0.2,<0.5.0-alpha").expect("valid version requirement"),
        Version::parse("0.4.0").expect("valid version"),
        migrate_0_2_to_0_3,
    ),
(
        VersionReq::parse(">=0.4,<0.6.0-alpha").expect("valid version requirement"),
        Version::parse("0.5.0").expect("valid version"),
        migrate_0_4_to_0_5,
    )]

or include 0.4.0 in the newly added item as below:

vet![(
        VersionReq::parse(">=0.2,<0.4.0-alpha").expect("valid version requirement"),
        Version::parse("0.3.0").expect("valid version"),
        migrate_0_2_to_0_3,
    ),
(
        VersionReq::parse(">=0.3,<0.6.0-alpha").expect("valid version requirement"),
        Version::parse("0.5.0").expect("valid version"),
        migrate_0_4_to_0_5,
    )]

There are other options as well, but will incur different design changes in the code.

[sehkone]

In sum, is this right?

    let migration: Vec<(_, _, fn(_, _) -> Result<_, _>)> = vec![
        (
            VersionReq::parse(">=0.2,<0.4.0-alpha").expect("valid version requirement"),
            Version::parse("0.3.0").expect("valid version"),
            migrate_0_2_to_0_3,
        ),
        (
            VersionReq::parse(">=0.4,<0.6.0-alpha").expect("valid version requirement"),
            Version::parse("0.5.0-alpha.4").expect("valid version"),
            migrate_0_4_to_0_5,
        ),
    ];

[minshao]

As explained in the earlier comment, this will cause the migration process to break I believe, since after calling migrate_0_2_to_0_3 the version of db is 0.3.0 which will not match ">=0.4,<0.6.0-alpha" thus migrate_0_4_to_0_5 will not be called.

[sehkone]

Sorry for bothering you, but could you explain what DATABASE_VERSION_REQ means? Plus, in my case, what should I write for it? I used =0.5.0-alpha.4 as Min told me but this seems to run migrate_0_2_to_0_3 as well.

[minshao]

It is indeed a confusing topic.

Let me explain in this way: think of DATABASE_VERSION_REQ is a version requirement, as long as the current database version satisfies this requirement, then the migration program will guarantee that after migration the database will work with the current program.

Both option I provided in previous comment, and I hereby listed below. I opted for option 1., (open for option 2), and we have other options as well, as for which one is better that might need some discussion with @msk:
1.

vet![(
        VersionReq::parse(">=0.2,<0.5.0-alpha").expect("valid version requirement"),
        Version::parse("0.4.0").expect("valid version"),
        migrate_0_2_to_0_3,
    ),
(
        VersionReq::parse(">=0.4,<0.6.0-alpha").expect("valid version requirement"),
        Version::parse("0.5.0").expect("valid version"),
        migrate_0_4_to_0_5,
    )]

vet![(
        VersionReq::parse(">=0.2,<0.4.0-alpha").expect("valid version requirement"),
        Version::parse("0.3.0").expect("valid version"),
        migrate_0_2_to_0_3,
    ),
(
        VersionReq::parse(">=0.3,<0.6.0-alpha").expect("valid version requirement"),
        Version::parse("0.5.0").expect("valid version"),
        migrate_0_4_to_0_5,
    )]

As for your question, could you provide the database version before running the migration? Along with it the DATABASE_VERSION_REQ and migration settings as well. Just with the given information, I'm not certain about the culprit.

[sehkone]

After considering the information you shared and my understanding of Min's intentions with regard to DATABASE_VERSION_REQ, I made some changes to the code in the PR. Can you review the modifications and inform me if I have misunderstood anything?