Opened 3 years ago

Last modified 21 months ago

#2590 new project-idea

Use Alembic instead of SQLAlchemy-Migrate

Reported by: dustin Owned by:
Priority: major Milestone: 0.9.+
Version: Keywords: database
Cc: rutsky.vladimir@…

Description (last modified by dustin)

Migrate has some compatibility problems - in particular, it doesn't work with SQLAlchemy >= 8.0. Its API is a little unpredictable, too, and it doesn't do a very good job of masking differences between dialects. Its linear numbering scheme is also problematic when developing on topic branches, as a merge of branches with overlapping DB version numbers will need some substantial changes to put them in linear order.

Alembic seems to be the fix to all that, although that's as far as we've gotten.

This would make a decent, if small, Google Summer of Code project.

Scope

The goal of this project is clear: replace the functionality of SQLAlchemy-Migrate with equivalent functionality implemented with Alembic. A GSoC application should also address:

  • Compatibility - users with current versions of Buildbot must be able to upgrade to Alembic
  • Testing - when database migrations fail, users lose data. The current migrations are thoroughly tested to ensure they do the right thing on all supported databases, and the new implementation must do the same.

Change History (19)

comment:1 Changed 3 years ago by dustin

  • Milestone changed from 0.8.+ to 0.9.+
  • Version 0.8.8 deleted

comment:2 Changed 3 years ago by rutsky

  • Cc rutsky.vladimir@… added

comment:3 Changed 3 years ago by dustin

  • Description modified (diff)

comment:4 Changed 2 years ago by Dustin J. Mitchell <dustin@…>

In d830cde7a802e7613d088ac766eb3ea7443999ba:

Remove Progress/ETA support.

This is part of removing the reliance on status APIs in bug 2818. The
progress interface wasn't very good, anyway, and will be re-implemented
in bug 2590.

comment:5 Changed 2 years ago by stibbons

Hi.

I work on it with Pierre on cactus.

Several side of the story:

  1. Is Alembic the best choice? Even if it really useful for us, it is an external program you will have to setup everywhere buildbot is installed. I clearly dislike this (the buildbot package should be self containing).
  2. Migration point: does buildbot keep the old "migration" script, to support old database migration, or do we start from scratch?
  3. Alembic comes with the autogenerate feature that is able, up to a certain point, to find out the difference between the model described in your python files (so, with you new development) and the current database state. update and downgrade functions are automatically generated. But it is able to only find some classical differences, you still need to write code for complex data migration,...
  4. But at the end, I usually just write the direct SQL statements to execute. It easy to do on our system since we have the control of our database, but this may be quite difficult to validate against the large variety of database.
  5. On the numbering problem, the issue will be a bit easier. We use <iso date>-description.py as naming scheme for file, it is very powerful. The real thing that "compute" the dependencies between script is the revision / down_revision reference each script has. So it should be easy to insert a script in between two existing script with minor modification.

I have some questions:

  1. what is the use of having a dedicated command to update the database (upgrade-master). For me it is useless, should be done automatically at startup, every time. The only advantage to have a different command, is if the user that performs the SQL structure alteration is different that the user that uses the database.
  2. How can the user lose data? user should never loose data. Never. Upgrade cannot fail since the database is managed by buildbot and only it. Alembic should not add more "security" that alchemy-migrate. Review and heavy unit tests should ensure migration scripts are fully validated. Alembic has a rollback mechanism, but I can't figure out how to guarantee it.

comment:6 Changed 2 years ago by dustin

Alembic is a new Python dependency, and can be installed anywhere Buildbot can -- so the new dependency isn't an issue.

I'd like to rewrite all of the existing migration scripts, with some glue in place to translate migrate versions (integers) into alembic versions.

Regarding autogeneration and SQL statements -- one of the things I *do* like about sqlalchemy-migrate is that it moneypatches SQLAlchemy to support common operations like adding and removing columns. I don't want to get to the point of writing raw SQL strings -- we have SQLAlchemy to do that kind of work for us -- but we'll need some functionality beyond what SQLAlchemy provides.

The buildbot upgrade-master command allows users to *explicitly* upgrade their master. Imagine the mayhem if someone accidentally started a newer version of Buildbot in a staging environment, but with the production database credentials, and it automatically upgraded the database. That would certainly cause downtime as all of the un-upgraded production masters were suddenly using a newer database. It might also mean data loss.

So, the upgrade-master command is a way to ensure that users are explicit about when database upgrades are performed.

Regarding "users lose data", I meant that a *broken* migration can easily lose data -- so testing will be important, as you said.

comment:7 Changed 2 years ago by stibbons

Why do you want to migrate all script to alembic vs why not freeze one point? Do you want users to be able to migrate from buildbot 0.8 (migrate) to 0.9 (alembic)? I don't think this would be an issue, anyaway.

What do you call "monkeypatch"? It generates upgrade/downgrade method by comparing your model to the current state of the database, but there are lot of stuff he is not able to see: column rename (it does a add/remove), action in enums,...

For your migration script translation, I would use the same "dialect" (import sqlalchemy as sa) just the pattern of the script will be the same.

I agree with you statement on upgrade-master, however I still wonder if it is a usecase it worth adding this complexity. If you want to protect against bad use, this is a way BUT this wont protect against everytime, I would say it is rare case. I don't remember the number of time I had to do "upgrade-master" on 0.8 in order to fix the deadly message on buildbot start. Result, we now execute upgrade-master everytime and it works like a charm in our prod for a while. Anyway, let's assume it is a design choice.

back to alembic, I can give a try if you want, to have something real to debate on.

comment:8 Changed 2 years ago by dustin

Yes, we need to make the transition from 0.8.x to 0.9.x as easy as possible, and that should include an automatic DB migration.

"monkeypatch" means that it's taking advantage of the dynamic nature of Python to add methods to various SQLAlchemy classes. If we were to drop SQLAlchemy-Migrate, we would lose some of those methods.

Yes, the explicit 'upgrade-master' is a design choice, and I think will become more important as more users go to a multi-master installation.

Absolutely, let's give Alembic a try!

comment:9 Changed 2 years ago by tardyp

Indeed, there is also the multimaster issue, if we want to add the auto-upgrade feature. As in multimaster mode, masters are likely to start approximately at the same time. We must use a db backed mutex so that the upgrade only happens in one master, and the other master must wait this mutex before starting.

If one goes up to some crazy continuous deployment mode, this would mean that several versions buildbot master might live at the same time, attached to the same db. Not sure if this is something we need to support right now, but I guess this is something worth to mention as a future improvment.

Autoupgrade could be something configurable, but I tend to agree with Dustin the problem looks simpler in a deployment devops script.

comment:10 Changed 2 years ago by stibbons

ok, I can imagine such a mutex, this can be easily done using the env.py script of alembic

comment:11 Changed 22 months ago by tflink

Out of curiosity, is the current plan to release nine with SQLAlchemy-Migrate? From the conversation here, I'm a little unclear on whether transitioning to Alembic is even desired, much less thought of as any sort of required/nice-to-have feature for nine.

comment:12 Changed 22 months ago by dustin

Alembic is desired, but yes, we'll stick to what we have, which is SA-M

comment:13 Changed 22 months ago by tflink

Just to make sure I understand - if a patch showed up to swap SQLAlchemy-Migrate out for Alembic, it wouldn't be considered until after nine releases?

comment:14 Changed 22 months ago by dustin

I don't see the timing of the two as connected. If a patch showed up right now, we'd merge it -- better to have it in at the start of nine, when many users will be starting over anyway.

comment:15 Changed 22 months ago by tflink

I interpreted the previous comment as "we don't want alembic for the initial nine release. some day, but not initially" hence the question.

I'll try to make the time to get this done. If I start making progress, I'll assign the ticket to myself but until then, assume no progress on my part :)

comment:16 Changed 22 months ago by tardyp

I think if we go for alembic, I would restrict the number of db migration scripts to the buildbots that have actually been published.

I mean there are tons of version between eight and nine, because of the long development. I think this would be wise the squash all of them to a few scripts. At least the trivial changes can be squashed, maybe we can keep the one where there is a need for data conversion. I dont think there are many.

comment:17 Changed 22 months ago by stibbons

I see 2 technical solutions:

A) What could be done is to find a way to keep the existing migration script from version n -> m using sqlmigrate, and then to all version higher than m using alembic. I think it is not that difficult to handle, a bit of hacking in alembic's env.py

B) Or all migration scripts need to be ported, as if there have been only alembic from the start (but still need to 'adapt' the version table so alembic can read it).

comment:18 Changed 21 months ago by dustin

I anticipate a time when sqlmigrate no longer works, so I'd rather avoid a case where we continue to carry it as a dependency.

comment:19 Changed 21 months ago by stibbons

this would have been a wonderful gsoc project... useful for the project, easily sizable because we know how many migrations scripts need to be translated. Good playground for unit test. But might be quite boring for the student...

Note: See TracTickets for help on using tickets.