Database migration with protocol features

[bowenwang1996]

Often times when we implements a new protocol change, we change some data structure as well. For example, a change could look like

pub struct A {
   ...
   #[cfg(feature = "protocol_feature_*")]
   new_field: u64
}

and if A is stored in storage, we need to do a database migration for the protocol change. The issue is that this migration will naturally be guarded by the feature flag when implemented and thus may cause conflicts with either a nonprotocol change that migrates the database or some other protocol feature that also happens to need a database migration. To make things worse, if those migrations modify the same column, there will be some conflicts that need to be resolved.

Some ideas on how we can address the issue:

1. Maintain the database version according to the order in which PRs are merged. If a new protocol feature changes the database version, the next nonprotocol change that changes database format has to respect the previous change even if it is behind a feature flag. More specifically, instead of having

#[cfg(not(feature = "nightly_protocol"))]
pub const DB_VERSION = ...
#[cfg(feature = "nightly_protocol")]
pub const DB_VERSION = ...

we simply have one DB_VERSION and some migrations will only be applied when nightly is enabled. For example,

if version <= 17 {
    #[cfg(feature = "protocol_feature_*")]
    apply_migration(...)
}

2. Alternatively, instead of maintaining a monotonic database version, maintain a separate one for nightly. The problem is that when there is a nonprotocol change that changes the database version on master, we need to rebase the nightly change on top of it to avoid conflict, which can be annoying.
3. To avoid managing conflicts in migrations, we can disregard database migrations for nightly protocol features and only implement them when we do a release. The downside here is that some migrations can be nontrivial (see 14 -> 15 migration for example) and we may run into unforeseen issues when we only implement the migration right before the release.

I'd love to hear what people think. Please feel free to comment and suggest better ideas!
@frol @evgenykuzyakov @SkidanovAlex

[frol]

1. I think it is going to lead to skipped migrations since once you migrate to version 16, you won’t get back to re-apply migration 14 that was behind a feature-flag. We will need to increment the DB version on release and conditionally apply migration 14 as version 17 if we detect that the data is not migrated yet, but considering the case of conflicting migrations, I don’t think it is manageable

2. There is no other way, really. Our stable releases will need to apply the stabilized nightly migration after the latest stable migration, so we will need to rebase anyway. I agree that it might be counterproductive at times, but I hope we won’t have too many conflicting migrations in nightly.

3. The risks are real, and it will cause delays in release and will have less testing that it could have during nightly versions testing

[bowenwang1996]

Yeah I just realized I forgot to write down the downside of 1. As you said, it is not really manageable when you have conflicting migrations since once that is released, a node cannot upgrade to the new version unless it wipes out all data and resync, which is not acceptable. I am fine with going with 2.