Performance issue due to DBForeignKey dropdown_field_threshold config

[lekoala]

Module version(s) affected

5

Description

The DBForeignKey scaffolder makes a count() query against the table to see if it needs to enable lazy loading. But this is counter productive, because making count queries against big tables is slow.

So it's easy to get into these kind of situations

The current design has multiple issues:

• this is done during scaffolding, you cannot simply replace the field to avoid the query (instead, it could be set to "auto" and decide, upon rendering, which setting to apply)
• counting doesn't cache anything, so if you have multiple relations against the same table, you are going to get duplicated queries
• counting large tables is slow (even with FEAT optimize DataQuery::count for speed #11642)
• setting the dropdown_field_threshold to 0 is still going to make a count query, even if it's going to be always true (except for empty tables)

In my opinion, instead of setting a threshold, it would be better to be able to qualify which DataObject have "large tables" (yes/no) and use that setting instead.

How to reproduce

In ModelAdmin
Have a DataObject with multiples has one relations to a large table
See that multiple count queries are issued due to scaffolding
Removing the scaffolded fields doesn't help

Possible Solution

• Instead of counting during scaffolding, have a default "auto" mode that decides, upon php rendering, if lazy loading needs to be applied (if counting is still the preferred way)
• Replace counting with a boolean setting per DataObject, eg: is_large_table. This could be useful in other scenarios (Remove row counting logic from GridField #11641 could benefit from this as well)
• Always set to lazy if threshold is 0 => this seems like an easy gain :-)

Additional Context

No response

Validations

• Check that there isn't already an issue that reports the same bug
• Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

PRs

• FIX Don't make a count query when threshold = 0 #11646
• ENH Reduce count queries for scaffolding SearchableDropdownField #11823

[GuySartorelli]

In my opinion, instead of setting a threshold, it would be better to be able to qualify which DataObject have "large tables" (yes/no) and use that setting instead.

I don't think this is a one or the other situation. The intention of the existing configuration is that you may not know ahead of time what tables you expect to grow large enough to need this. Once they do grow that large, you have to be aware they're causing a bottleneck in order to set a "large tables" configuration. So having something to resolve that for you for large (but not so large that even a count causes problems) tables seems sensible.

For tables so big even a count is problematic I agree that perhaps a separate configuration could be added which bypasses the count. Perhaps there are other places that also need to treat these tables in a special way - and if so, perhaps the configuration could be on DataObject itself. if (MyModel::config()->get('is_large_table')) { /* treat it specially */ }

[GuySartorelli]

A PR was merged which directly addresses the count when limit is set to 0. That's only one of the "design issues" the issue discusses so I'll leave this open.

[GuySartorelli]

@lekoala I've raised a PR which will avoid repeated counts for the same class.

A is_large_table or similar configuration sounds like a good idea in principle, but should probably be tackled more holistically - i.e. it doesn't really make sense for it to just target this one thing. If there's multiple places that should treat large tables differently, a new issue should be created which lists as many use cases as possible so we can really make use of it.

We could add a more limited use-case configuration that does target this one thing, but I wouldn't want to do that and then we end up adding is_large_table anyway, 'cause then we have two configuration for the price of two (when we only need one).

[lekoala]

:-) thanks, as long as we don't have multiple count queries I think this is already a great improvement