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

PostgreSQL migration performance due to hasTable behavior #53002

Closed
Vinksyunit opened this issue Oct 1, 2024 · 4 comments · Fixed by #53006
Closed

PostgreSQL migration performance due to hasTable behavior #53002

Vinksyunit opened this issue Oct 1, 2024 · 4 comments · Fixed by #53006

Comments

@Vinksyunit
Copy link

Laravel Version

11.21.0

PHP Version

8.3.11

Database Driver & Version

Postgres 17.0

Description

In Laravel 11.21.0 and since Laravel 10.34, migrations' behaviour has changed. It causes issues on Postgres.

It call Builder::hasTable behind the scene and here is the issue :

    public function hasTable($table)
    {
        $table = $this->connection->getTablePrefix().$table;

        /** @phpstan-ignore arguments.count (SQLite accepts a withSize argument) */
        foreach ($this->getTables(false) as $value) {
            if (strtolower($table) === strtolower($value['name'])) {
                return true;
            }
        }

        return false;
    }

We see here that this function calls getTables() to check if a table exists. There is perhaps more direct way to do that, but the issue is not directly here.

As you can see, for SQLite, there is a false argument used to prevent calculating size of tables. We should have the same kind of behaviour for Postgres, because now the query made look like that :

select c.relname as name, n.nspname as schema, pg_total_relation_size(c.oid) as size, 
obj_description(c.oid, 'pg_class') as comment from pg_class c, pg_namespace n 
where c.relkind in ('r', 'p') and n.oid = c.relnamespace and n.nspname not in ('pg_catalog', 'information_schema') 
order by c.relname

And before it was something like this :

select * from information_schema.tables where table_catalog = $1 and table_schema = $2 and table_name = $3 and table_type = 'BASE TABLE'

So a request dedicated to find if a table exists, not loading all tables size infos.

For the same kind of migration done, it causes a RAM usage 5 times bigger and a CPU 4 times bigger in our case. We have a lot of tables in this database (~10k).

image

Can not quickly provide a benchmark, but it is easy to understand the issue here. Checking for a table existence should not list and calculate size of all the tables in the db.

Steps To Reproduce

Start a new empty Laravel project settuping PostgreSQL as your database.

Launching migration will call getTable() and load all tables with their size. A simpler request should be done.

Copy link

github-actions bot commented Oct 1, 2024

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@Vinksyunit
Copy link
Author

#49243 and #52615 are related I think.

#49020 is the PR that broke performances for MySQL/MariaDB/SQLite.
#49148 is the one that broke PostgreSQL.

@hafezdivandari
Copy link
Contributor

You may check PR #53006

@Vinksyunit
Copy link
Author

Was working on a Pr myself, yours is basically mine but better. So well done for reactivity !

That's solvibg the issue.

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

Successfully merging a pull request may close this issue.

3 participants