Respect minUpgradableAppVersion in the pkgdef.

[zenhack]

There is a field defined in package.capnp called minUpgradableAppVersion, which indicates the oldest version of the app that a package can safely upgrade. As far as I can tell, Sandstorm doesn't actually do anything with this field.

We should recognize the field somehow.

Ideally we'd be smart about this, and update to the latest compatible version of an app, but it seems maybe a little fiddly to do this correctly: Obviously it isn't safe to just upgrade to an intermediate package, not start the grain, and then upgrade again. The intermediate version needs to actually run its migration before the app can be upgraded again.

This came up in conversation on IRC because I was looking for a way to not have to keep bundling niscudb with meteor-spk forever; at some point it would be nice if we could use this field to drop support for niscudb grains. But obviously we actually need to implement it first.

[ocdtrekkie]

We'd also have to find the latest compatible version. That might require an app-index feature. I think mass transfers basically tries to request the exact package ID for a given transferred grain, which mostly works for apps because the app index has all the previous packages submitted dumped in there somewhere. However, this also has some weird quirks: #3167 especially since users might have grains using different versions as well.

So I think we'd need to be able to query the app index for a specific appVersion for a given packageId, boot any grains that this is applicable to, shut them back down, and update to the latest. And presumably we need to check all this recursively, to make sure the minUpgradeableAppVersion's package doesn't also have a minUpgradeableAppVersion above that of the grain.

I also think I'd really want #2755 if there was scary stuff like this going on. I am still super scared to upgrade existing grains on my Sandstorm server.

[zenhack]

Quoting Jacob Weisz (2020-02-24 15:37:26)

We'd also have to find the latest compatible version. That might require an app-index feature. I think mass transfers basically tries to request the exact package ID for a given transferred grain, which mostly works for apps because the app index has all the previous packages submitted dumped in there somewhere. However, this also has some weird quirks: [1]#3167 especially since users might have grains using different versions as well.

Yeah, we'd probably need to start supporting updates per-grain.

So I think we'd need to be able to query the app index for a specific appVersion for a given packageId, boot any grains that this is applicable to, shut them back down, and update to the latest. And presumably we need to check all this recursively, to make sure the minUpgradeableAppVersion's package doesn't also have a minUpgradeableAppVersion above that of the grain.

I think the logic for this isn't too bad; the query is basically "Give me the latest version of <app-id> that has a minUpgradableAppVersion of at most <current-version>." That's not a terribly complex database query. I haven't looked at the app index code *at all* though, and the contributing doc is fairly bearish on its state, so I have no idea what modifications need to happen there. I also have absolutely no idea how checking for updates works at all right now; do we actually send queries to the app market? Do we periodically download a database and query it ourselves, like many package managers do? I'd have to investigate.

I also think I'd really want [2]#2755 if there was scary stuff like this going on. I am still super scared to upgrade existing grains on my Sandstorm server.

Maybe this should get deferred until after we have some bits of our backup solution in place (which might also address a good chunk of your concerns there). We should automatically make periodic backups of grains' storage, and allow grain owners to restore previous versions. If we also do this right before an upgrade, this gives the grain owner some recourse if a buggy upgrade hoses the grain. Reverting to an old version of the grain should revert both the storage *and* the version of the app it is using.

…

--- One fiddly implementation issue which I mentioned briefly above that needs addressing is: if the user upgrades a grain from version A to B, but doesn't launch it, and then version C comes out that specifies minimumUpgradableAppVersion = B, it isn't actually safe to upgrade that grain to C, because the migration logic in B hasn't run, so the storage is still that of version A. An obvious solution is to keep track of the last app version that was "run," but that's still racy; just because a grain has been started doesn't mean its migration logic has actually finished. Right now Sandstorm has no way to tell if an upgrade is "complete." Maybe the solution is to add an 'upgradeCommand' or such to the package definition, which gets run the first time a grain is launched with a new app version. When this command completes, sandstorm marks the grain as being migrated to the new version, and then runs the regular continueCommand.

[kentonv]

TBH I'm skeptical that minUpgradableAppVersion ever made sense. As has been noted, Sandstorm doesn't actually know anything about the version of the data on disk. It only knows what version of the app it intends to start when the grain is opened. Migration of the data is up to the app and happens asynchronously.

I think we'd need to design some separate notion of a data version, along with an API by which an app can inform sandstorm once a migration has been completed. An app package could then declare that it requires at least some particular data version. The package metadata would also need to explicitly designate some other package which can be used to migrate older versions to the required verison. Note that this isn't necessarily an older version of the same app: Imagine that a bug is discovered in the migration long after the app has stopped supporting older versions. There needs to be a way to push a fixed version of the migration itself.

But I'm really not convinced all this is worthwhile. If you have to maintain those migrations anyway, why not just include them in your package? Is bundling "niscudb" into Meteor apps forever actually that big a burden? It's one binary and a small script to drive the migration. The binary should never even need to be recompiled; we can just keep copying over the exact binary from the previous version of meteor-spk.

(Hmm, admittedly, I just noticed that the "niscu" binary is a little large, but maybe we can rebuild it with -Os and strip it...)

[ocdtrekkie]

If we don’t think that property makes sense and doesn’t do anything either, is there a way to either remove it or mark it as useless so people don’t believe it works?

[zenhack]

The typical thing is to just rename it, see e.g.

sandstorm/src/sandstorm/hack-session.capnp

Line 70 in 1b23560

obsoleteRevokeApiToken @5 (tokenId :Text);

[zenhack]

This came up again in the context of sandstorm-io/vagrant-spk#276; upgrading to a new version of mysql requires embedding the old version too, in case the db's journal needs to be recovered before upgrading. Furthermore, From the mysql docs, they don't officially support skipping major releases, so although jumping from 5.5 to 5.7 seems to work in this case, in principle this means each db upgrade is a new version of the software that needs to be embedded forever. It would be nice if the necessary number of old versions was at least bounded.

I think we could make this work as follows:

• Add a method to SandstormApi called markMigrationComplete or such. Then, for upgrade purposes, minUpgradableAppVersion would treat a grain as being the last version of the app that called markMigrationComplete (or 0 if it has never done so).
• Make it so that the app index can handle queries like "give me the most recent version of app id X that has a minUpgradableAppVersion of at most Y"

Note that it is possible for a new version of an app to lower the value of minUpgradableAppVersion, so if a bug in the migration is discovered, a new version could be released that can handle fixing the old previously unsupported versions, and then support can be dropped again in a later version. This might be a bit of a pain to put together, but it is feasible. This, combined with the app index logic I describe above, obviates the need for the newer version to designate a version to migrate with.

[ocdtrekkie]

If common vagrant-spk stacks the logic for markMigrationComplete or whatever, I'd be okay with this. However, I am concerned about adding another need for packagers to figure out how to integrate the API. Is it possible to automatically assume that after a given grain has run a given amount of time after being started, it should be safe, and automatically update this value for the grain?

For most scenarios, I would imagine the standard 90ish seconds before a grain is auto-shutdown should be expected to have handled any migration tasks required for a grain upgrade.

[zenhack]

Eh, under most circumstances it will probably work, but murphy's law...

But I think we could easily integrate this into a command line tool, so launcher.sh could just have a command mark-migration-complete or such at an appropriate point. Furthermore, apps wouldn't actually need to use this unless they choose to drop support for old storage formats.

We should include in the vagrant-spk docs guidance on migrating to newer versions of stacks, which for stuff like the lemp stack will include guidance on how to set minUpgradableAppVersion.

[ocdtrekkie]

I do feel like for lemp, in particular, we should be able to take like 90% of the scaffolding you put around TTRSS and just give it to everyone. Most PHP/MySQL apps want to be handed a database and not have to worry much about it after that. But yeah, if we provided a little command line tool that "just did it", that'd be good too.