Performance improvements when adding mulitple indexes

[dbhynds]

I've found an opportunity for some performance improvements with this package, but am not entirely sure the best way to implement them. I'd be happy to open a PR, but need some guidance on the best route for implementation.

While using this tool, we noticed that certain types of migrations take an excessively long amount of time. Adding a column to our largest table takes about 6 hours, but we observed several migrations taking upwards of 36 hours to run. I was able to determine the root cause of the slowness. When running a migration, Laravel's $blueprint->toSql() method compiles a list of statements to execute on the connection. If you have a migration that adds two columns and an index, it will result in two statements: one to add the columns, another to add the index. The Blueprint object, the loops through the statement and calls the Connection::statement() method on connection object in this library, which executes a process to run and apply the changes to the table.

However, because Laravel's Blueprint object creates a separate statement for each index, this tool runs a separate pt-osc command for each one. It creates the table, adds the index, and copies the data for each index. Thus, adding 3 indexes to the table means copying the table 3 times. (In our case, since doing so once takes 6 hours, adding 3 indexes takes 18 hours: 3 * 6). If it were possible to change it so that it adds all the indexes at once (create the table, add all 3 indexes, copy the data), we could cut down total time significantly.

I brought up this concern in the Discord server for Laravel (https://discord.com/channels/297040613688475649/486650494790402061/961381126948134964), and the recommendation was to use DB statements. I'm wondering if we could add some additional features to this package to make this task more performant.

I was able to get a proof of concept of this working by making the following modifications. I wrote a migration that added two columns and two indexes to a table. When running this normally, I observed that it ran three separate pt-osc statements (one for adding the columns and two for the indexes). I then edited the Illuminate\Database\Schema\Blueprint object, and replaced the existing public function build() with this:

    public function build(Connection $connection, Grammar $grammar)
    {
        $connection->statements($this->toSql($connection, $grammar));
    }

Then I edited Daursu\ZeroDowntimeMigration\Connections\PtOnlineSchemaChangeConnection and added this:

    public function statements($queries, $bindings = [])
    {
        $table = $this->extractTableFromQuery($queries[0]);

        $cleanQueries = [];
        foreach($queries as $query) {
            $cleanQueries[] = $this->cleanQuery($query);
        }

        $process = array_merge(
            [
                'pt-online-schema-change',
                $this->isPretending() ? '--dry-run' : '--execute',
            ],
            Arr::get($this->config, 'options', []),
            [
                '--alter',
                implode(', ', $cleanQueries),
                $this->getAuthString($table),
            ]
            );

        return $this->runProcess($process);
    }

By doing this, I was able to observe the desired behavior. It only ran one pt-osc command, which added both columns and indexes at once.

This proof-of-concept won't work, however, because it relies on modifying the Laravel Blueprint object. Is there another approach we could take to get this working that relies solely on this package?

[niftyhack]

Hi @dbhynds

I just found this repo while searching for a solution to our problem: Running migrations with Laravel on a Percona-Xtra-DB-Cluster.

Are you still using this package or have you found another way for efficient database migrations with Laravel?

[dbhynds]

We still use this package, and I haven't found another solution yet. (Right now we're just living with the inconvenience. It's not common to add multiple indexes to our largest tables.) Daursu and I emailed back and for about it a bit when I opened this issue, but that was the last I heard about it. I haven't had time to work on it since.

[Watercycle]

I was recently looking into some of the reasons pt-online-schema-change "randomly" fails and pushed up a fork with Davo's proposed change since the individual statements were part of the problem. I'd be curious to hear if these changes work for others.

[Daursu]

Support for this has been implemented in v2.0.0 (thanks to @Watercycle)