Staff SiteSerializer category_types performs one DB query per SiteSetting in configuration schemas

Ontwikkelaar
,
1

I noticed what looks like an N-per-setting query pattern while investigating initial page-load performance for staff users.

On my site, an ordinary non-staff account loads /latest noticeably faster than my admin/staff account. While profiling the staff request with Rack Mini Profiler, I found that SiteSerializer#category_types appears to build category-type configuration metadata by checking each SiteSetting individually to determine whether it has been overridden.

The relevant path appears to be:

SiteSerializer#category_types
→ Categories::TypeRegistry.list
→ category type metadata
→ resolved_configuration_schema
→ site_setting_overridden?
→ SiteSetting.provider.find

Current code:

In particular, site_setting_overridden? calls:

SiteSetting.provider.find(setting_name.to_sym).present?

and resolved_configuration_schema calls that while iterating the SiteSettings in the configuration schema.

SiteSettings::DbProvider#find then performs an individual query for the named setting:

In the profiler output I saw individual queries such as:

SELECT name, data_type, value
FROM site_settings
WHERE name = 'discourse_post_event_allowed_on_groups';

SELECT name, data_type, value
FROM site_settings
WHERE name = 'use_local_event_date';

SELECT name, data_type, value
FROM site_settings
WHERE name = 'show_filter_by_solved_status';

SELECT name, data_type, value
FROM site_settings
WHERE name = 'topic_voting_show_who_voted';

There were 16 queries matching this site_settings WHERE name = ... pattern in the captured request. These were for different setting names, so I am not suggesting that the same lookup is simply being repeated; rather, it appears to be one DB lookup per SiteSetting represented in the category-type configuration schemas.

This seems particularly relevant to staff users because category_types is only included for staff:

My profiling also showed plugin-provided category configuration participating in this path, including Discourse Solved, calendar/events-related settings and Topic Voting settings.

For comparison, on the same Discourse installation and connection, a warm /latest load was approximately:

Admin/staff account:
DOMContentLoaded: roughly 0.77–1.03 s
Initial HTML: roughly 2.08 MB decoded

Ordinary account:
DOMContentLoaded: roughly 0.51 s
Total decoded resources: roughly 1.50 MB

The ordinary-account PWA also ended up loading at least as quickly as the AMD Developer Community PWA in subsequent testing, so this does not appear to be a general origin/CDN performance issue.

I also had one unusual staff Safe Mode request where layouts/application produced 1,362 SQL queries and took about 2.9 seconds. I do not think the per-SiteSetting queries above account for all of those queries, so I am treating that separately rather than claiming this is responsible for the entire outlier.

Would it make sense for the overridden-setting state used by resolved_configuration_schema to be fetched/batched once, rather than calling SiteSetting.provider.find separately for each SiteSetting?

For example, DbProvider already has an all method, although I am not suggesting that using all directly is necessarily the correct implementation - mainly wondering whether avoiding the N-per-setting database lookups here would be worthwhile.

I checked the same path at current main and it appears that the per-setting provider.find behaviour is still present.

3

I’ve now reproduced this against current main and opened a PR which batches these SiteSetting override lookups:

While working through it, I found there were effectively two levels to the query pattern.

The original implementation called:

SiteSetting.provider.find(...)

once for each SiteSetting represented in a category type’s configuration schema.

A first batching approach could reduce that to one provider.all per category type, but Categories::TypeRegistry.list calls metadata separately for each registered type, so that would still result in multiple bulk queries when several category types provide SiteSettings.

The PR instead loads the persisted SiteSetting overrides once in TypeRegistry.list and passes that information through to all of the category types being serialized. Direct metadata/schema resolution retains a single bulk-provider fallback.

So for the staff site payload path I originally reported, the intended shape changes from approximately:

one SELECT ... WHERE name = ? per configured SiteSetting

to:

one bulk SELECT from site_settings for TypeRegistry.list

The existing overridden/not-overridden behaviour is preserved.

I added regression coverage for both cases:

  • multiple SiteSettings within a single category type;

  • multiple category types sharing the same bulk override lookup.

Locally, the relevant category and serializer specs passed:

149 examples, 0 failures

The PR has also now completed upstream CI successfully: 14 successful checks, 0 failures, 0 pending (with 2 checks skipped).

So this appears to address the specific N-per-setting behaviour from the profiler capture in the opening post, rather than the separate much larger SQL-query outlier I mentioned near the end of the opening post.