cakephp / phinx

PHP Database Migrations for Everyone
https://phinx.org
MIT License
4.46k stars 891 forks source link

Data Loss: ndbcluster table engine broken #2184

Closed turtle0x1 closed 1 year ago

turtle0x1 commented 1 year ago

Recently pivoted to using this library on an application that is based on a mysql cluster (5.7 if that matters, which It might).

The problem I'm facing is the server that runs the migration see's the new table engine as ndbcluster but the other servers seem to see it as InnoDB (which doesn't work as expected in the cluster context).

I didn't face this problem when running raw .sql migrations but very much would like to stick with library as it solves alot of problems.

This issue may be premature, it may be my environment. Though what ever caused this, sure is a "surprise".

If the tables aren't all declared as ndbcluster, replication doesn't happen. If replication doesn't happen 50% of the time we respond with an empty result set (or far far worse).

Example working .sql

CREATE TABLE `something` (
   `id` INT PRIMARY KEY NOT NULL AUTO_INCREMENT
) ENGINE=ndblcuster;

Example broken phinx migration & config

Config

    'environments' => [
        'default_migration_table' => 'migration_table',
        'default_environment' => 'dev',
        'dev' => [
            'adapter' => 'mysql',
            'host' => $_ENV["DB_HOST"] ?? "WONT_WORK_SET_ENV",
            'name' => $_ENV["DB_NAME"] ?? "WONT_WORK_SET_ENV",
            'user' => $_ENV["DB_USER"] ?? "WONT_WORK_SET_ENV",
            'pass' => $_ENV["DB_PASS"] ?? "WONT_WORK_SET_ENV",
            'port' => '3306',
            'charset' => 'utf8',
            'collation' => 'utf8mb4_general_ci',
            'engine' => 'ndbcluster' // NOTE me expilicity telling phinx what todo here
        ],

Migration

        $users = $this->table('something', ['id' => "something_id", 'primary_key' => ["something_id"]]);
        $users->addColumn('something_date_created', 'datetime', ['default' => 'CURRENT_TIMESTAMP', 'null'=>false])
           ->addForeignKey('something_id, 'Table_X, 'table_x_id ['delete'=> 'RESTRICT', 'update'=> 'RESTRICT'])
           ->addIndex(['something_date_created'], ["unique"=>true])
           ->create();

My environment

Despite multiple warnings about not relying on defaults, this database default engine isn't set to ndbcluster (though I dont think that should matter in this context).

turtle0x1 commented 1 year ago

Initial pass at a debug it looks like it creates the right SQL.

On cluster replication member it also doesn't create the constraints (FK,) but I think those silently fail due to miss matched table types (also scary, these errors should really bubble).

turtle0x1 commented 1 year ago

Okay this was premature.

If you run the migration with the engine set to innodb instead of nbcluster before updating the engine (without dropping the innodb table from other cluster members) this library fails silently.

This is a real edge case caused by a blunder I should have recognised.

That being said, it would be nice if phinx could error in the mysql cluster context on this error (if it can, I assume mysql/PDO doesn't error ergo phinx doesn't).