Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC #5 - New Migration Schema #508

Closed
wzrdtales opened this issue Nov 1, 2017 · 16 comments
Closed

RFC #5 - New Migration Schema #508

wzrdtales opened this issue Nov 1, 2017 · 16 comments

Comments

@wzrdtales
Copy link
Member

wzrdtales commented Nov 1, 2017

Foreword

This RFC describes the new migration schema, which is needed to better support safe migrations. This will solve problems that currently exists with:

  • Rollbacks of actions within migrations
  • Continuation of aborted migrations (i.e. when the server was crashing while executing)
  • Zero Downtime compatible operations

Requirements

It is required to have any action, an action will describe as a single operation like renameColumn, within a migration directly linked with its reversion. An action may in some occassions be of a deterministic nature and directly revertable without further description of the rollback routine.

Backwards Compatibility

There is no need to keep compatible to any old migration schema, as migration schemas are excplicitly versioned by their _meta export. However, drivers may not be affected and should not have the need to change.

Effects on issues

This RFC will have effects on the following issues:


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@wzrdtales
Copy link
Member Author

Referencing some recent commenters for opinions @BorntraegerMarc @GeeWee @westy92 @RemyJeancolas

@wzrdtales
Copy link
Member Author

wzrdtales commented Nov 1, 2017

A first suggestion might be something like this:

First naive attempt

On completely deterministic components ending up as easy as this:

exports.migrate = db => {
   return db.createTable('table1', { imagineoptions: 'here' }); // this is it, we know the reverse of createTable which always is dropTable
}

On non deterministic components:

exports.migrate = async db => {
  await db.dropTable('table1')
    .reverse(() => db.createTable('table1', { imagineoptions: 'here' });

  return db.someOtherAction());
}

Question here would be, are we explicitlly enforcing to provide a reverse.

Second attempt, maybe better?

And currently it seems to me like this wouldn't be that readable, so probably the first naive, but not perferct attempt. May be it should look something more like this:

exports.migrate = async db => {
  await db.atomic({
      up: db.dropTable('table1'),
      down: db.db.createTable('table1', { imagineoptions: 'here' })
  });
    
  return db.someOtherAction();
}

Chances to clean up and get rid of old stuff

Also a chance to get rid of setup, including them as second parameter as options:

exports.migrate = async (db, dbm) => {
  await db.driverDoesStuff();
  console.log(dbm); // all that informational or additional stuff
  return null;
}

Also to note, that I would still keep callbacks as an option to use here, but would not document it anywhere, so that promises and async/await emerge to be the default schema to work in and not supporting callbacks anymore officially.

@wzrdtales
Copy link
Member Author

To note the final version would be the official version 2 of the migrations schema.

@wzrdtales
Copy link
Member Author

Referencing driver devs as well @charyorde @Yashin32 @etops

@wzrdtales
Copy link
Member Author

Oh forgot @toymachiner62

@wtgtybhertgeghgtwtg
Copy link
Contributor

Since the code samples are using async, are you planning on bumping the required node version or using babel? It would certainly help simplify the codebase.

@wzrdtales
Copy link
Member Author

@wtgtybhertgeghgtwtg Yes and no. I will never use babel, but I will definitely drop backwards support to the most outdated versions. Current plan is down to node 4.

@wzrdtales
Copy link
Member Author

Looking at the distribution of node versions though. People are currently only starting to move over the new LTS though. It is a bit to early to also drop node 4.

@BorntraegerMarc
Copy link
Contributor

After reading this issue 3-4 times I came to the realization that I have no clue what it means 😄 as you described it, backwards compatibility isn't a problem: so on my side everything looks good

@wzrdtales
Copy link
Member Author

It is about getting thoughts on possible new schemas that migrations evolve into, like my first attempt over here #508 (comment) :)

@stephenlacy
Copy link

Any thoughts on the interface for creating a rollback, then reverting it automatically upon error?
If the migration is operated by a CI I wouldn't want constant up and rollback migrations without history logged to the migration table.

@wzrdtales
Copy link
Member Author

@stevelacy Can you elaborate a bit more? What exactly you want to know? What do you specify as "creating rollback"?

@stephenlacy
Copy link

If a migration hits an error it probably should attempt to automatically (if configured) rollback the failed changes, especially if it is controlled by a CI. The interface is the way a CI or human would manage the failed action.

@wzrdtales
Copy link
Member Author

This is actually the meaning of atomic operations and is already thought of:

#508 (comment)

@wzrdtales wzrdtales mentioned this issue Apr 5, 2018
4 tasks
@wzrdtales
Copy link
Member Author

In the new migration schema all db functions will become intermediate action generators. The migrations themselves will probably be not generator functions, since this would be to confusing for the user when they're working with the framework. A user would expect to use an interface and not to provide one them self.

The last action of a migration has to be either returned, or a Promise needs to delivered.

For all actions that the user seeks to receive a result from, there will be an result function on the action object created.

For example:

exports.migrate = async db => {
  db.createTable('table1'); // reverse is well known, will revert itself without further definition 
  db.atomic({
      up: db.dropTable('table1'),
      down: db.createTable('table1', { imagineoptions: 'here' })
  }); // drop wouldn't be revertable also we know the reverse of it, but data is simply lost
    
  await db.query(SQL`SELECT * FROM "something"`).result(); // this will execute the given query and starts executing all the previous actions until this one is reached and delivers the result.

  return db.someOtherAction();
}

There will be no need to call await or then on any function, unless you expect an result from it. The processor of the db-migrate core will take care to execute them in the correct order for you and also reverse them if needed.

Whenever the user wants to do something more sophisticated db.atomic will also take a user defined function, which must be offered without any parameters.

exports.migrate = async db => {
  db.atomic({
    up: myUp,
    down: myDown
  });
});

async function myUp(db) {
  // ... do your stuff here, all rules apply that apply in the normal function as well
  return db.createTable('test1', ...);
}


async function myDown(db) {
  // ... do your stuff here, all rules apply that apply in the normal function as well
  return db.dropTable('test1');
}

Alternatively the user can directly call their functions, but this way it will be easier to segment it and it makes it easier to define whole revertable segments, which might be even reusable.

@wzrdtales
Copy link
Member Author

Together with the new migration schema, db-migrate will introduce also revertable strategies for some of the non revertible actions. Specifically this means interfaced actions. Custom sql will remain unpredictable.

Dropping Tables

db-migrate will use the information and optionally store the actions themself directly assigned with their targets. That means the createTable of table xyz and addIndex of table xyz can be directly reexecuted as a reversion of a drop table. runSql and other non interfaced methods will get a new parameter which will give the user the possibility to add the necessary information which tables his action will actually adjust. This parameter, while optional shall throw warnings if it is unset. It can be set to null to explicitly define no altered target, which will also stop throwing the warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants