moves seeding to the initialization of the datastore

[vroldanbet]

Addresses one of the follow ups identified in #525

This moves the seeding responsibility out of IsReady and makes it side-effect free. It's conceptually unexpected that IsReady can have side effects, and beyond that, having the mutation as part of the datastore initialization gives us some nice properties:

• the application fails to start if seeding didn't succeed. Lack of seeding is not something the application can recover from,
  and we wouldn't want to have a SpiceDB pod running as healthy when the datastore isn't seeded. If seeding failed due to a transient, the pod will get another chance as its restarted by kubernetes when non-zero error codes are returned.
• IsReady is not invoked right after SpiceDB starts, so SpiceDB can start even though the datastore is not ready, leading to a situation where the service is healthy but unseeded until IsReady is invoked. This was reported in fail to start spice serve command if Datastore isn't ready #482 but there hasn't been traction on the proposal, so by moving seeding to datastore initialization we can make sure the application will fail to start.

[jakedt]

What is the plan for incremental seeding? For example, if a datastore were already seeded with the zeroth transaction and a new migration, e.g. the one proposed in #546 needed to do more "initial" DML statements, how would these get picked up? Can we add separate DML statements to migrations which get aggregated into a "seeded-through-migration" value which parallels the DDL statements? Maybe the migration executor could have both a migrate and a seed function?

[vroldanbet]

What is the plan for incremental seeding? For example, if a datastore were already seeded with the zeroth transaction and a new migration, e.g. the one proposed in #546 needed to do more "initial" DML statements, how would these get picked up?

Hey @jakedt, that's a great question. TL;DR I don't think it's needed, for now. Read further for the nitty gritty details:

The way I think about the seeding is that is closely related to / needed by changes in the codebase, so deploying a new version of the codebase is warranted anyway, and so I haven't seen the need to run the seeding at any other moment during of the lifecycle of the spicedb processes. So in short, I don't think it's needed. Could you think of scenarios where it is? Doing this in initialization is somewhat the same because it makes sure that the application does not successfully startup unless the required migration and seeding is in place. This is also related to #482

A more fundamental question #546 opens up, and that I keep going back to, is the chicken and egg problem (we discussed about this recently in Discord). As far as I can tell, we can identify at least 2 classes of migrations:

• one that require codebase changes to make sure the application is able to work with both "before and after" migration states (e.g. dropping column, changes to column type...)
• migrations that are by nature backward compatible (e.g. adding an index, adding new table) and does not require the application to be "ready for it".

#546 falls under the latter - it's adding a new table, and the currently running SpiceDB would remain unaffected by it. Then you deploy the application, and seeding happens. Great! 🎉 . Unfortunately due to how Datastore.IsReady() is implemented (it checks the system is running at HEAD), running such migration would invariably lead to IsReady() == false, because suddenly the codebase finds its running with a migration version it does not know about (a.k.a future migrations). I'll admit I don't have a clear picture yet of when is IsReady invoked. As far as I could tell from us ending with migrations that led it to be not ready, the application was still responding to Check requests.

I addressed the reverse scenario with migrate.Manager.IsHeadCompatible() which helps the datastore run at head, or head-1. That works well for "deploy, then migrate". However, that still does not solve the problem where you need to run the migration first because the code needs it. You could change the codebase to handle both "with and without it the required changes", but it may be tricky / inconvenient to handle in the codebase 😢 So I think we could revisit the checks in IsReady to not report not ready if using a future migration:

• one option is to define an environment variable where customers can configure "migrations soon to be run". You redeploy the application, tell spicedb "there is a new future migration version coming, don't worry about it". This a low effort change (famous last words) and could be encapsulated in migrate.Manager.IsHeadCompatible().
• another option is to relax IsReady and say "future migrations are expected". That would be fine if it not were by the fact that then we loose the information of "which migrations have been run". We could change the implementation of the versions table to keep such a list, so when a new "future migration" comes, the system says "it's fine as long as migration X was run".

Can we add separate DML statements to migrations which get aggregated into a "seeded-through-migration" value which parallels the DDL statements? Maybe the migration executor could have both a migrate and a seed function?

I'm not sure I fully understood what you mentioned there. I thought about separating schema-migrations and data-migrations in spicedb migrate before landing onto this design. spicedb migrate could have subcommands to separate DML and DDL. That would certainly remove the need for these seeding functions on startup, at the expense of more operational burden. My preference was the system reconciling its own state, because regardless there is still the need to deploy a version that uses the changes introduced by migration anyway. But I don't discard scenarios where the seeding may not be trivial or practical on application bootstrap at all (e.g. normalize relation_tuple table to avoid duplicating namespace string representation that would lead to a large data migration) and that would warrant an offline data-migration process to be run.