procrastinate-org / procrastinate

PostgreSQL-based Task Queue for Python
https://procrastinate.readthedocs.io/
MIT License
849 stars 52 forks source link

[v3] Migrations backwards compatibility #1189

Open ewjoachim opened 2 weeks ago

ewjoachim commented 2 weeks ago

According to our own doc, migrations named with version X are the migrations you can apply once you run on version X. According to our doc, people on v2.13 wishing to upgrade without disruption would:

Compatibility matrix:

Code ➡️
⬇️ Schema
v2.13 v3.0 v3.1
v2.13
v3.0
v3.1

❌ means we don't have to make it compatible, but if it happens to be, it's not a problem, of course

We need to decide what we put in v3.0 and v3.1

Note: the change of status = aborting -> abort_requested might need a couple of trigger just for the time of the dual compatibility (adding the `abort_requested column and update its value based on the status, then add a trigger that synchronizes the 2 columns, and in the next release, delete the triggers and the column)

medihack commented 3 days ago

I must confess that I get totally confused by naming the migrations now. In the contributing docs we say "xx.yy.zz is the number of the latest released version of Procrastinate" and in the migrations docs "you want to update to Procrastinate 1.15.0 ... migration scripts whose versions are greater than or equal to 1.9.0, and lower than 1.15.0". But then in the above table and #1186 you name the migration 03.00.00_01_cancel_notification.sql which won't be lower when we include this migration in a 3.0 release. Unfortunately, I also don't fully understand the "Note". Have I missed something in my migration?

ewjoachim commented 2 days ago

It is confusing :( When we wrote procrastinate, we were in an environment where we couldn't stop the service while applying migrations, so we had to ensure we created compatible migrations, that there would always be a path where you upgrade code, then db, then code, then db etc, without service interruption. As of today this is still an official commitment procrastinate does. We can revisit this but I believe it's worth trying to keep it.

For v3, you're right that the move I make is probably wrong but the migration was named for 2.9.2 which is also wrong. The reality is that:

We should definitely run the tests with the DB on the previous schema to enforce this, otherwise it's probably prone to wishful thinking. I think it would also be interesting to start versioning our procedures, so anytime we change them, we can control easily what version of procrastinate uses what version of the procedures while still having multiple ones in the code. That might be much less :exploding_head: for us. Though, we can't do the same with all objects (tables, enums, triggers...), of course.

I sometimes wonder if it's worth using procedures at all, compared to doing everything in the queries, but that's a whole other debate.

medihack commented 1 day ago

Sorry, I am still a bit confused 😞. So with #1186 we will do it the other way around than

there would always be a path where you upgrade code, then db, then code, then db

because

we'll do a v3.0 that does almost no change in the code (so almost identical to v2.x) but introduces DB changes

If the migration will be part of v3.0, wouldn't it be more congruent to check the latest v2.x version (straight before the release) and name it that way?

But maybe I take care of those other open issues for v3 and just watch and learn what you do with this issue here 😉.

We should definitely run the tests with the DB on the previous schema to enforce this, otherwise it's probably prone to wishful thinking. I think it would also be interesting to start versioning our procedures, so anytime we change them, we can control easily what version of procrastinate uses what version of the procedures while still having multiple ones in the code. That might be much less 🤯 for us. Though, we can't do the same with all objects (tables, enums, triggers...), of course.

I sometimes wonder if it's worth using procedures at all, compared to doing everything in the queries, but that's a whole other debate.

And all those triggers that use them?

ewjoachim commented 1 day ago

and just watch and learn what you do with this issue here 😉.

Haha you seem to think I have it all thought through and I have a plan. I'm learning as I go, and what I've learnt so far is that if you say there's something strange, I better get looking, because you've shown an excellent gut feeling for those issues so far :)

And all those triggers that use them?

I believe the set of procedures used by triggers is quite distinct from the set of "business-logic" procedures, the former aren't very complex (save procrastinate_trigger_status_events_procedure_update) nor often updated. The ones that may benefit from not being procedures could be procrastinate_defer_job, procrastinate_fetch_job, procrastinate_finish_job and the likes.