sql: Django's table introspection query is slow (seconds to minutes, depending on number of tables)

[timgraham]

What is your situation?

Django uses this query for table introspection:

SELECT
    a.attname AS column_name,
    NOT (a.attnotnull OR ((t.typtype = 'd') AND t.typnotnull)) AS is_nullable,
    pg_get_expr(ad.adbin, ad.adrelid) AS column_default
FROM pg_attribute AS a
LEFT JOIN pg_attrdef AS ad ON (a.attrelid = ad.adrelid) AND (a.attnum = ad.adnum)
JOIN pg_type AS t ON a.atttypid = t.oid JOIN pg_class AS c ON a.attrelid = c.oid
JOIN pg_namespace AS n ON c.relnamespace = n.oid
WHERE (
    (
        (c.relkind IN ('f', 'm', 'p', 'r', 'v')) AND
        (c.relname = '<target table>')
    ) AND (n.nspname NOT IN ('pg_catalog', 'pg_toast'))
) AND pg_table_is_visible(c.oid)

Observed performance

The query takes 2-5 seconds with tens of tables or several minutes with hundreds of tables. The Django test suite issues the query around 175 times totaling around 8 minutes of the test suite's 68 minutes run time.

Also, it's currently infeasible to run Django's test suite all at once because the query takes about 4 minutes if the ~1400 tables for all of the test suite are present.

Build Tag: v21.1.0-alpha.1-289-g960b4cfc54
Build Time: 2020/12/12 06:45:40
Build Commit ID: 960b4cf

Requested resolution

• I want CockroachDB to be optimized for my use case.

[blathers-crl]

Hello, I am Blathers. I am here to help you get the issue triaged.

I have CC'd a few people who may be able to assist you:

• @solongordon (found keywords: JOIN,pg_)
• @rafiss (found keywords: JOIN,pg_, found keywords: Django)

If we have not gotten back to your issue within a few business days, you can try the following:

• Join our community slack channel and ask on #cockroachdb.
• Try find someone from here if you know they worked closely on the area and CC them.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[rafiss]

A starting point here would be to add a benchmark to bench/ddl_analysis (to be renamed) and see how many KV roundtrips are happening.

[rafiss]

I added benchmarks in ddl_analysis and tested with increasing number of tables:

1 table    278 KV roundtrips
2 tables   283 KV roundtrips
4 tables   293 KV roundtrips
8 tables   313 KV roundtrips

So it seems that the number of roundtrips scales linearly with the number of tables in the cluster. Also, the fact that it does 278 roundtrips even when there is just one table seems like a lot.

[rafiss]

Here's the explain plan

                          tree                          |    field     |                                          description
--------------------------------------------------------+--------------+------------------------------------------------------------------------------------------------
                                                        | distribution | local
                                                        | vectorized   | false
  render                                                |              |
   └── hash join                                        |              |
        │                                               | equality     | (oid) = (relnamespace)
        ├── filter                                      |              |
        │    │                                          | filter       | nspname NOT IN ('pg_catalog', 'pg_toast')
        │    └── virtual table                          |              |
        │                                               | table        | pg_namespace@primary
        └── virtual table lookup join                   |              |
             │                                          | table        | pg_type@pg_type_oid_idx
             │                                          | equality     | (atttypid) = (oid)
             └── virtual table lookup join (left outer) |              |
                  │                                     | table        | pg_attrdef@pg_attrdef_adrelid_idx
                  │                                     | equality     | (attrelid) = (adrelid)
                  │                                     | pred         | (attnum = adnum) AND pg_table_is_visible(adrelid)
                  └── virtual table lookup join         |              |
                       │                                | table        | pg_attribute@pg_attribute_attrelid_idx
                       │                                | equality     | (oid) = (attrelid)
                       │                                | pred         | pg_table_is_visible(attrelid)
                       └── filter                       |              |
                            │                           | filter       | (pg_table_is_visible(oid) AND (relkind IN ('f', 'm', 'p', 'r', 'v'))) AND (relname = 'table')
                            └── virtual table           |              |
                                                        | table        | pg_class@primary
                                                        

I also captured a trace of running the query. I think part of the issue is that it's running the pg_table_is_visible function for each table in the cluster. The plan above shows that it runs the function before filtering on nspname. The implementation of pg_table_is_visible is an entirely separate pg_catalog query, which ends up causing the table descriptors to be fetched each time. (I think the internal query executor doesn't use the same cache for the descriptors.)

cockroach/pkg/sql/sem/builtins/pg_builtins.go

Lines 1079 to 1089 in d86781c

	"pg_table_is_visible": makeBuiltin(defProps(),
	tree.Overload{
	Types: tree.ArgTypes{{"oid", types.Oid}},
	ReturnType: tree.FixedReturnType(types.Bool),
	Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
	oid := args[0]
	t, err := ctx.InternalExecutor.QueryRow(
	ctx.Ctx(), "pg_table_is_visible",
	ctx.Txn,
	"SELECT nspname FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON c.relnamespace=n.oid "+
	"WHERE c.oid=$1 AND nspname=ANY(current_schemas(true))", oid)

I don't know if there's a way to reorder that filter condition ((pg_table_is_visible(oid) AND (relkind IN ('f', 'm', 'p', 'r', 'v'))) AND (relname = 'table')) so that relname = x is evaluated first.. that would allow it to short-circuit and not run pg_table_is_visible.

cc @jordanlewis @rytaft tagging you since you're on-call at the moment -- I was wondering:

• Is there a way to make a virtual index on pg_class.relname to improve this? I think this may have come up before and there was a reason why not, but I can't remember.
• Can the optimizer be taught to order the filter conditions to do the expensive pg_table_is_visible check last? I don't know if we do anything like that, but just wondering.

[jordanlewis]

Is there a way to make a virtual index on pg_class.relname to improve this? I think this may have come up before and there was a reason why not, but I can't remember.

Yes, we can certainly do this, though I'm not completely sure whether it would be sufficiently useful if the virtual index didn't also force you to specify the database and schema name.

Perhaps a more compelling way to fix this would be to make pg_table_is_visible not be completely terrible. There isn't a strong reason why it should have to do what it is doing. Instead, it should be able to look up the descriptor with the input id and compare the parent schema to whatever current_schemas ends up outputting.

[rafiss]

Instead, it should be able to look up the descriptor with the input id and compare the parent schema to whatever current_schemas ends up outputting.

But ultimately that will still cause a descriptor to be fetched once for each table in the database right -- not so different than what ends up happening now? Or do you mean that doing it this way will allow us to add more caching?

I'm included the statement bundle since I forgot to attach it before.
stmt-bundle-627699616186892289.zip

[jordanlewis]

But ultimately that will still cause a descriptor to be fetched once for each table in the database right -- not so different than what ends up happening now? Or do you mean that doing it this way will allow us to add more caching?

If we use the right kind of resolution, it will use leases (aka the cache).

[rytaft]

Can the optimizer be taught to order the filter conditions to do the expensive pg_table_is_visible check last? I don't know if we do anything like that, but just wondering.

We don't currently support re-ordering filters in a single filter statement. But we could increase the cost of pg_table_is_visible in the coster by updating this map:

cockroach/pkg/sql/opt/xform/coster.go

Line 154 in 3db4737

var fnCost = map[string]memo.Cost{

. That might change the plan, though, and it's not clear that's what you want.

[rafiss]

I agree @jordanlewis 's suggestion to just make pg_table_is_visible less expensive seems better.

Thanks for the pointer though @rytaft. I will play around with the costing to see if that does anything good for us here.

In the meantime, until we fix this within the DB, @timgraham could you update django-cockroachdb to make this query instead:

SELECT
    a.attname AS column_name,
    NOT (a.attnotnull OR ((t.typtype = 'd') AND t.typnotnull)) AS is_nullable,
    pg_get_expr(ad.adbin, ad.adrelid) AS column_default
FROM pg_attribute AS a
LEFT JOIN pg_attrdef AS ad ON (a.attrelid = ad.adrelid) AND (a.attnum = ad.adnum)
JOIN pg_type AS t ON a.atttypid = t.oid JOIN pg_class AS c ON a.attrelid = c.oid
JOIN pg_namespace AS n ON c.relnamespace = n.oid
WHERE (
    (
        (c.relkind IN ('f', 'm', 'p', 'r', 'v')) AND
        (c.relname = '<target table>')
    ) AND (n.nspname NOT IN ('pg_catalog', 'pg_toast'))
) AND (n.nspname=ANY(current_schemas(true)))

I believe it is functionally equivalent, and it will avoid all of these extra lookups. My benchmark shows this only makes 4 KV roundtrips, no matter how many tables are in the database.

[rafiss]

Instead, it should be able to look up the descriptor with the input id and compare the parent schema to whatever current_schemas ends up outputting.

It seems like some refactoring will be needed to make that happen. the tree.EvalContext only can use EvalPlanner, and neither of those has any schema accessors or descriptor collections. we could change it, but the first simple refactoring i tried caused a package dependency cycle, so looks like it will take more thought.

separately, i was wondering why the internalExecutor used by the builtin doesn't already use the same Collection as the outer eval context. @arulajmani raised this question

[rafiss]

Discussed in this internal thread: https://cockroachlabs.slack.com/archives/C0168LW5THS/p1611947857055600

The conclusion was to try to grow the EvalPlanner interface and implement the "table is visible" functionality directly in planner.

The issue is that there are around ~20 builtins that use the internal executor, so all of those might also have a similar problem as pg_table_is_visible