Run migrations in a multy tenancy environment

[mario-lemes]

Hey guys,

I got stuck trying to adapt an automatic migration process at runtime in a multi-tenancy environment. I have been digging through the issues but it seems there is nothing similar to what I want to achieve. Any help would be greatly appreciated.

Scenario

sequelize: 6.3.5
umzug: 3.0.0-beta.9
dialect: postgresql
store: SequelizeStorage

For each tenant created at runtime, I run a migration process, however, it does not allow to run multiple times the same migration because of expected reasons (on the first migration it creates a new record with the migration name on the table "SequelizeMeta" in the "public" schema).

Based on that, I tried two different approaches:

1st approach

By passing {migrations: [...], rerun: 'ALLOW'} to umzug.up() I ensure the migration process runs anytime I call the function and the I just to go through all the tenants on the migration file and running all the actions:

// roles-migrations.js
async function up({ context: queryInterface }) {
  const schemas = await queryInterface.showAllSchemas();
  for (const schema of schemas) {
    await queryInterface.bulkInsert({ tableName: 'roles', schema }, [
      {
        id: 'admins',
        created_at: new Date(),
        updated_at: new Date(),
      },
      {
        id: 'users',
        created_at: new Date(),
        updated_at: new Date(),
      },
    ]);
  }
}

async function down({ context: queryInterface }) {
  const schemas = await queryInterface.showAllSchemas();
  for (const schema of schemas) {
      await queryInterface.bulkDelete({ tableName: 'roles', schema }, null, {});
  }
}

module.exports = { up, down };

The problem with this approach is that the second time I call to umzug.up() will try to run the migration again in the first tenant which it's already done so it will throw an expected 'duplicated key error'.

2nd approach

In this approach, I went a bit further playing around with the sequelize object but with no success. Basically, I tried to instantiate a new Umzug instance fo each time I wanted to run the migration process but also didn't work. This still creates the "SequelizeMeta" table in "public"

async function runMigration(schema) {
      const umzug = new Umzug({
        migrations: { glob: 'migrations/*.js' },
        context: sequelize.getQueryInterface(),
        storage: new SequelizeStorage({
          sequelize,
          schema
        }),
      });

      await umzug.up();
}

Any help is really appreciated.

Thanks.

[mmkal]

Your 2nd approach above looks sensible to me. What value does schema have? Umzug should be respecting that, and not putting the model in public every time.

Side note - make sure you return umzug.up() so it's not a floating promise.

[mario-lemes]

Hi @mmkal,

Thanks for your response!

In the second approach, schema is a string value with the schema name (tenant) but you got to the point because somehow I was not setting properly that value. Thanks for that!

Now that umzug is working as expected I got into a new situation. So, basically, since I'm calling umzug.up() per tenant migration, I don't need to go through all the tenant in the migration.js file because will cause duplicated index so let's get rid of that loop:

// roles-migrations.js
async function up({ context: queryInterface }) {
    await queryInterface.bulkInsert('roles', [
      {
        id: 'admins',
        created_at: new Date(),
        updated_at: new Date(),
      },
      {
        id: 'users',
        created_at: new Date(),
        updated_at: new Date(),
      },
    ]);
}

async function down({ context: queryInterface }) {
    await queryInterface.bulkDelete('roles', null, {});
}

module.exports = { up, down };

But what happened is that it throws the next error Error: 'SequelizeDatabaseError: «roles» relation does not exist. And actually make sense! The queryInterface belongs to the sequelize instance which does not have any schema established on its initialization because all this happens in runtime, so by default is looking into the public schema (based on the Sequelize documentation). So, now I need to look for a way to pass the schema value to up() and down() function from the migration file in order to address properly the queries. From umzug it seems only sends queryInterface and I haven't figured how to pass anything else out so I got with this dirty workaround:

async migrate(schemaName) {
      // Hardcoding the schema name into the sequelize connection options payload in runtime.
      connection.options.schema = schemaName;

      const umzug = new Umzug({
        migrations: { glob: 'migrations/*.js' },
        context: this.connection.getQueryInterface(),
        storage: new SequelizeStorage({
          sequelize: connection,
          modelName: 'migrations_meta',
          schema: schemaName,
        }),
        logger: databaseLogger.logger,
      });

      await umzug.up();     
      connection.options.schema = undefined;
}

And with that, the migration.js file turn into this:

// roles-migrations.js
async function up({ context: queryInterface }) {
  const schema = queryInterface.sequelize.options.schema;
  await queryInterface.bulkInsert({ tableName: 'roles', schema }, [
    {
      id: 'admins',
      created_at: new Date(),
      updated_at: new Date(),
    },
    {
      id: 'managers',
      created_at: new Date(),
      updated_at: new Date(),
    },
    {
      id: 'members',
      created_at: new Date(),
      updated_at: new Date(),
    },
  ]);
}

async function down({ context: queryInterface }) {
  const schema = queryInterface.sequelize.options.schema;
  await queryInterface.bulkDelete({ tableName: 'roles', schema }, null, {});
}

module.exports = { up, down };

I'm not aware at the moment of the possible side effects of this but since I haven't found any mechanism provided by sequelize to change the schema other than in the query functions itself so I came up with that solution.

However, nothing of this still worked, for some reason umzug runs the migration the first time on the first tenant properly but then with the second tenant, Umzug checks the migration table on the first tenant schema :

// Preparing migration for the first tenant called "usertenantexpired"
// I gess this is done by Umzug
info: Executing (default): CREATE TABLE IF NOT EXISTS "usertenantexpired"."migrations_meta" ("name" VARCHAR(255) NOT NULL UNIQUE , PRIMARY KEY ("name"));
info: Executing (default): SELECT i.relname AS name, ix.indisprimary AS primary, ix.indisunique AS unique, ix.indkey AS indkey, array_agg(a.attnum) as column_indexes, array_agg(a.attname) AS column_names, pg_get_indexdef(ix.indexrelid) AS definition FROM pg_class t, pg_class i, pg_index ix, pg_attribute a, pg_namespace s WHERE t.oid = ix.indrelid AND i.oid = ix.indexrelid AND a.attrelid = t.oid AND t.relkind = 'r' and t.relname = 'migrations_meta' AND s.oid = t.relnamespace AND s.nspname = 'usertenantexpired' GROUP BY i.relname, ix.indexrelid, ix.indisprimary, ix.indisunique, ix.indkey ORDER BY i.relname;
info: Executing (default): SELECT "name" FROM "usertenantexpired"."migrations_meta" AS "migrations_meta" ORDER BY "migrations_meta"."name" ASC;

// Migration running
info: == roles-migrations.js: migrating =======
info: Executing (default): INSERT INTO "usertenantexpired"."roles" ("id","created_at","updated_at") VALUES ('admins','2020-11-28 21:14:46.863 +00:00','2020-11-28 21:14:46.863 +00:00'),('managers','2020-11-28 21:14:46.863 +00:00','2020-11-28 21:14:46.863 +00:00'),('members','2020-11-28 21:14:46.863 +00:00','2020-11-28 21:14:46.863 +00:00');
info: Executing (default): CREATE TABLE IF NOT EXISTS "usertenantexpired"."migrations_meta" ("name" VARCHAR(255) NOT NULL UNIQUE , PRIMARY KEY ("name"));
info: Executing (default): SELECT i.relname AS name, ix.indisprimary AS primary, ix.indisunique AS unique, ix.indkey AS indkey, array_agg(a.attnum) as column_indexes, array_agg(a.attname) AS column_names, pg_get_indexdef(ix.indexrelid) AS definition FROM pg_class t, pg_class i, pg_index ix, pg_attribute a, pg_namespace s WHERE t.oid = ix.indrelid AND i.oid = ix.indexrelid AND a.attrelid = t.oid AND t.relkind = 'r' and t.relname = 'migrations_meta' AND s.oid = t.relnamespace AND s.nspname = 'usertenantexpired' GROUP BY i.relname, ix.indexrelid, ix.indisprimary, ix.indisunique, ix.indkey ORDER BY i.relname;
info: Executing (default): INSERT INTO "usertenantexpired"."migrations_meta" ("name") VALUES ($1) RETURNING "name";
info: == roles-migrations.js: migrated (0.021s)

// Some queries on the first tenant
info: Executing (default): INSERT INTO "usertenantexpired"."companies" ("id","name","created_at","updated_at") VALUES (DEFAULT,$1,$2,$3) RETURNING "id","name","created_at","updated_at";
...

// Preparing migration for the secont tenant called "usertenantcompleted" **but it's using the previous tenant schema "usertenantexpired"**
info: Executing (default): CREATE TABLE IF NOT EXISTS **"usertenantexpired"."migrations_meta"** ("name" VARCHAR(255) NOT NULL UNIQUE , PRIMARY KEY ("name"));
info: Executing (default): SELECT i.relname AS name, ix.indisprimary AS primary, ix.indisunique AS unique, ix.indkey AS indkey, array_agg(a.attnum) as column_indexes, array_agg(a.attname) AS column_names, pg_get_indexdef(ix.indexrelid) AS definition FROM pg_class t, pg_class i, pg_index ix, pg_attribute a, pg_namespace s WHERE t.oid = ix.indrelid AND i.oid = ix.indexrelid AND a.attrelid = t.oid AND t.relkind = 'r' and t.relname = 'migrations_meta' AND s.oid = t.relnamespace AND s.nspname = 'usertenantexpired' GROUP BY i.relname, ix.indexrelid, ix.indisprimary, ix.indisunique, ix.indkey ORDER BY i.relname;
info: Executing (default): SELECT "name" FROM "usertenantexpired"."migrations_meta" AS "migrations_meta" ORDER BY "migrations_meta"."name" ASC;

// No migration runs

// Some queries into the second tenant 
info: Executing (default): INSERT INTO "usertenantcompleted"."companies" ("id","name","created_at","updated_at") VALUES (DEFAULT,$1,$2,$3) RETURNING "id","name","created_at","updated_at";
...

By the way, I tried without hardcoding the schema into the sequelize connection option payload and using global variables for testing purposes and it's happening the same.

What do you think?

Thanks,
Mario.

[mmkal]

Hi - will be able to take a closer look later this week, but this should absolutely be possible. You can pass anything you like to context, passing the queryInterface object is just an optional convention:

async migrate(schemaName) {
      const umzug = new Umzug({
        migrations: { glob: 'migrations/*.js' },
        context: {
          queryInterface: this.connection.getQueryInterface(),
          schema: schemaName,
        },
        storage: new SequelizeStorage({
          sequelize: connection,
          modelName: 'migrations_meta',
          schema: schemaName,
        }),
        logger: databaseLogger.logger,
      });

      await umzug.up();     
      connection.options.schema = undefined;
}

Then in your migration(s), change how you use context slightly:

// roles-migrations.js
async function up({ context: { queryInterface, schema } }) {
  await queryInterface.bulkInsert({ tableName: 'roles', schema }, [
    {
      id: 'admins',
      created_at: new Date(),
      updated_at: new Date(),
    },
    {
      id: 'managers',
      created_at: new Date(),
      updated_at: new Date(),
    },
    {
      id: 'members',
      created_at: new Date(),
      updated_at: new Date(),
    },
  ]);
}

async function down({ context: { queryInterface, schema } }) {
  await queryInterface.bulkDelete({ tableName: 'roles', schema }, null, {});
}

module.exports = { up, down };

[mario-lemes]

Hi @mmkal,

Oh, thanks for pointing that out! For some reason I thought that was not possible, I did the changes that you mentioned but still does not work as expected.

Basically happens what I described in my previous post.

Thanks for taking the time to take a look at this. Really appreciated.

[mario-lemes]

Hi again,

I have been getting deeper on the SequelizeStorage to see where the problem is coming from and I have some thoughts about it.

It seems there is a function getModel() which is in charge to return the SequelizeMeta model and what happens here is basically that checks if the model exist by using this.sequelize.isDefined(this.modelName) function which actually checks its existence by lookin into the sequelize.models object.

From sequelize.js:

  /**
   * Checks whether a model with the given name is defined
   *
   * @param {string} modelName The name of a model defined with Sequelize.define
   *
   * @returns {boolean} Returns true if model is already defined, otherwise false
   */
  isDefined(modelName) {
    return !!this.modelManager.models.find(model => model.name === modelName);
  }

This is totally fine when there is no schema-based approach like in a multi-tenant architecture, hence Sequelize also support other db engines we can check if a model has been defined by checking the sequelize.models object. However, this can run into wrong assumptions when we are in a schema-based approach like in postgres, because Sequelize will save internally the models definitions for the first tenant but that does not mean that is actually defined (synced) for the second tenant onwards.

Correct me if I'm wrong but I think the function getModel() should check the existence of the SequelizeMeta model having in consideration the schema-based approach by using a raw query to either the information_schema.tables or pg_class and pg_namespace, more info here.

Hope all this helps.

Thanks

PS: I will be happy to make a PR if you agree with the solution I described above. I'm also open to any other approach you think could be better.

[mmkal]

Sequelize storage is supposed to be multi-dialect so not really workable to put postgres-specific sql in there.

You might not need to though. AFAIK each sequelize instance has its own model manager, so likely your problem is coming from using a shared sequelize instance. It's hard to tell for sure from your sample code, but it looks like sequelize comes from out of scope. Try changing it so that each schema gets its own instance:

async function runMigration(schema) {
+     const sequelize = new Sequelize (..., { schema });
      const umzug = new Umzug({
        migrations: { glob: 'migrations/*.js' },
        context: sequelize.getQueryInterface(),
        storage: new SequelizeStorage({
          sequelize,
          schema
        }),
      });

      await umzug.up();
}

See sequelize constructor docs for how to pass it in.

In fact, assuming that works, by passing the schema into the sequelize constructor, you likely wouldn't need to inject it into the context anymore - I expect it'd be the default schema for the queryInterface object.

[mario-lemes]

Hey @mmkal,

That's right, it's a shared instance that is initialized on booting.

I address the queries by using the schema parameter like Model.findAll(...{schema}).

Don't you think that creating a sequlize instance per tenant is an unnecessary workload taking in considerations the multiple connections are created to the db and the memory that having all the models definitions per tenant migth consume.

I'm wondering if there is a way of doing it just by using a shared instance, but maybe that's actually not the ideal way of using sequlize.

If you don't mind it will be helpfully to read your thoughts about it.

Thanks.

[mmkal]

I wasn't aware of schema being a valid option to pass into Model.findAll.

Sequelize docs do recommend using a shared instance - but in this case you may be ok with using multiple, depending on how long they're used for, how pooling is configured, and how many schemas you need to migrate. Usually migrations are run separately from application code, so hopefully each instance could tear itself down fairly quickly. Definitely worth testing out though.

If you can find a way to explicitly pass the schema to the various calls in SequelizeStorage, a PR would be welcome. I'm planning on adding an examples directory soon, and it'd be good to be able to include this use-case.

Note: To be honest I don't currently use sequelize so I'm not the right person to comment on best practices. I also maintain a postgres-only migrator which uses umzug with slonik: https://npmjs.com/package/@slonik/migrator. I prefer writing raw sql over ORMs generally. @papb may be able to shed some light/CC some sequelize other experts.

[mario-lemes]

Hey @mmkal,
Thanks for your quick response.

I think that's a good point, I can just create the instance for the migration process and close the connection using sequelize.close(), that would be good enough for the time being.

In another hand, I'm still interested in getting my hands on the SequelizeStorage in order to make it works just using a sequelise shared instance, so I'll take some time this week to work on it so hopefully, I will be able to send a PR for evaluation.

It would be great if other people can give their thoughts about it but it's up to you to keep this thread open as long as you want, I'm very grateful for the information given so to me it's crystal clear ;).

PS: This sounds interesting , I will take a look to slonik, thanks for sharing!

[mmkal]

Sounds good - have moved this to a discussion since we're aiming to reserve issues for clear-cut bugs etc., but if you make a PR feel free to link to it as normal.

[mario-lemes]

Hey @mmkal,

Your response came at the perfect timing, I was playing around with the SequelizeStorage in order to have PR candidate that cover the use case we have been discussing and I realized a very crucial thing. The SequelizeStorage constructors allow passing the model object itself! That changes all. I can simply define the "SequelizeMeta" model for each tenant on runtime and pass it to the SequelizeStorage constructor. It worked like a charm, this is the code example:

async migrate(schema) {
      const databaseLogger = Loggers.newLogger('database');
      // Migrations model
      const model = sequelize.define(
        'MigrationsMeta',
        {
          name: {
            type: DataTypes.STRING,
            allowNull: false,
            unique: true,
            primaryKey: true,
            autoIncrement: false,
          },
        },
        {
          schema,
          modelName: 'migrations_meta',
          underscored: true,
          charset: 'utf8',
          collate: 'utf8_unicode_ci',
        },
      );

      const umzug = new Umzug({
        migrations: { glob: 'migrations/*.js' },
        context: {
          connection: sequelize.getQueryInterface(),
          schema,
        },
        storage: new SequelizeStorage({
          model,
        }),
        logger: databaseLogger.logger,
      });

      await umzug.up();
  }

This way allows having migrations in runtime in a multi tenant environment without any problem.

Thanks for all your help and hope this helps other to deal with this kind of scenario.

[mmkal]

Great to hear - I can @ you in the PR adding an example along these lines, if that's alright with you.

[mario-lemes]

Hey @mmkal,

Maybe I didn't explain myself very well, I meant is already implemented in umzug. You can pass the model object to the SequelizeStorage constructor already so in the end I didn't need to do the PR. Anyways feel free to use these examples in the documentation or anything else you want :D.

Thanks,

[mmkal]

Yeah I just meant it'd make sense to have an example to point to so this information isn't lost. The plan is to put up a few usage examples as part of the documentation.

[mario-lemes]

Ah! Of course, do you want me to do that? I will be happy to do so.

[mmkal]

I'll add some other examples first - it'd be great to get help after that. Thank you!

[itsnicolearias]

Hello, i'm having the same issue. Does someone know a way to fix this and keep using the table SequelizeMeta?