[BUG]: migrate isn't protected against simultaneous execution

[MatanYadaev]

What version of drizzle-orm are you using?

0.27.0

What version of drizzle-kit are you using?

0.19.3

Describe the Bug

There's no protection against the migrate function being executed simultaneously multiple times.

Executing await Promise.all([migrate(), migrate(), migrate()]) will lead to the migrations running three times and potentially causing problems.

Expected behavior

The migrate function should be designed to execute only once at any given time, probably by using a locking mechanism.

For reference, Prisma uses Advisory Locking.

Environment & setup

Postgres 15.3

[dankochetov]

In what case do you see this as a problem?

[MatanYadaev]

@dankochetov I deploy my Node app using Kubernetes, and I have multiple Node instances (pods) in production, and each one of them runs migrations pre-deployment.
If multiple pods are deployed at the same time, they will run the migrations at the same time.

[MatanYadaev]

@dankochetov Fixed the concurrency issue with an advisory locking. Would be great if it will be built-in.

const advisoryLock = async (
  {
    key1,
    key2,
    timeoutInMs = 10_000,
  }: { key1: number; key2: number; timeoutInMs?: number },
  callback: () => Promise<unknown>,
) => {
  const client = await pool.connect();

  await client.query(`SELECT pg_advisory_lock($1, $2)`, [key1, key2]);

  try {
    await Promise.race([
      callback(),
      new Promise((_, reject) => {
        setTimeout(() => {
          reject(new Error('Timeout'));
        }, timeoutInMs);
      }),
    ]);
  } finally {
    await client.query(`SELECT pg_advisory_unlock($1, $2)`, [key1, key2]);
    client.release();
  }
};

const MIGRATE_ADVISORY_LOCKING_KEY1 = 123_456;
const MIGRATE_ADVISORY_LOCKING_KEY2 = 456_789;

export const migrate = async () => {
  await advisoryLock(
    {
      key1: MIGRATE_ADVISORY_LOCKING_KEY1,
      key2: MIGRATE_ADVISORY_LOCKING_KEY2,
    },
    async () => {
      await drizzleMigrate(database, {
        migrationsFolder: './drizzle',
      });
    },
  );
};

// Now only one migration runs at a time
await Promise.all([migrate(), migrate(), migrate()]);

[hannasm]

It seems like a table lock on drizzle.__drizzle_migrations might be a bit more straightforward and would avoid the random keys 123_456, 456_789.

It definitely could become problematic if certain migrations are run multiple times on the same database though.

[scott-dn]

Indeed, it would be great if it supported the built-in advisory lock.

[soulchild]

Here we are in 2024 and another hyped JS ORM is delivered without a proper migration locking mechanism (cue in laughter from the Laravel/Rails/Django/… communities). I get it: Y'all are using some hyped up database SaaS provider which probably takes care of migrations for you automagically. But believe it or not: There are folks still running their own PostgreSQL clusters and deploying Docker containers themselves.

[castarco]

DISCLAIMER: I say this as myself, I'm not related to the development team in any way.

@soulchild I don't think that your comment adds anything of value to this thread.

Sure, it would be great if we had professional-grade tools, high quality software tools are excessively scarce.

But let's remember that:

• most of "us" haven't paid a penny for it
• most of "us" didn't contribute a single line of code (to this project or any other)
• they are under no obligation to bring us what we want/need.

I say this as an open source contributor: some of you are shamelessly disrespectful. It is one thing to point at a problem and debate its importance/priority, and another to mock the projects or its developers.

[soulchild]

Sorry to have come across rude. I didn't mean to attack the maintainers of Drizzle.

I'm just perplexed by the sheer number of developers who seemingly over night started bashing Prisma and other established ORMs on their social media channels while simultaneously praising Drizzle as a superior solution, just because it performs better on the edge or something.

Issues like this one tell a completely different story, IMHO. There's still lots of stuff missing or broken which other ORMs have figured out a long time ago.

I really want to like Drizzle, but in its current state it's just not for me and I'm having a hard time finding advantages over Prisma, which is far from perfect either and has its own set of weird problems/architectural decisions (like basic JOINs still being an experimental feature).

So again, apologies to anyone I have offended with my comment. I know that open-source is a lot of blood, sweat and tears and I absolutely appreciate the hard work the Drizzle team is doing! ❤️

[AndriiSherman]

Thanks, everyone! We are finally going through all the bugs and trying to address all of them. This one is important, and thanks a lot for the link explaining how it should be handled. I'm taking this one on and will try to ship it as fast as I can!

[soulchild]

@AndriiSherman Thanks a lot for taking this on!

One minor suggestion which would make the migrations feature even more useful IMHO: The migrate() method could return some sort of status report about the migration run (which migrations were applied successfully or failed). Most ORMs with a built-in migrator have something like this (for instance Kysely).

We're running migrations before starting the server and currently there's no way to see whether any migrations were run at all.