The performance decreased because of a combination of calls that were supposed to increase performance.
We have a mechanism in place which automatically eager loads models needed in the element representers when a collection of them is rendered. This is to avoid N+1 queries of course. But, if eager loading is combined with e.g., a LIMIT, which we do because we paginate, rails automatically falls back to issuing two instead of just one SQL statement. Which makes sense as otherwise LEFT JOINS might mess with the result set.
But Rails does so in a somewhat simple fashion. It uses the first query to get the DISTINCT ids. The second is used to load the values (without a limit). But instead of removing all WHERE statements in the second SQL statement and then apply just the one for the ids, it keeps the original WHERE statement and applies the one for the ids on top. The problem with that is that the database trips on that (I didn't check the why) and uses a less than optimal query plan.
That was the problem here as well. The first query remained reasonable quick (300ms) but the second one took 25s.
The fix is to split the two statements by hand in the representer whenever eager loading is defined. The first query has all the filters but no eager loading and fetches the ids. The second takes the ids, and with eager loading included loads the data. Et voila, second query takes 10ms.
That at least works for relations, work_packages and projects. But there are other representers that also seem to have custom behaviour. I'm looking into whether they can be easily adapted.
So far, we've been using the relaxed sanitization rule set. We only need some styles for the table display in CKEditor,
other styles should not be allowed.
The ancestors/children representer change calls `child.display_id`,
which consults `identifier` in semantic mode. The Hierarchy eager
loader preloads children with a minimal `SELECT id, subject, project_id,
parent_id` for performance, so `identifier` was missing and
`ActiveModel::MissingAttributeError` fired the moment a query endpoint
rendered a work package with visible children in semantic mode.
Add `identifier` to the SELECT. It's one extra short text column per
child row.
In semantic mode, the work package breadcrumb renders numeric IDs
instead of the semantic identifier because ancestor HAL resources are
built from `_links.ancestors[]` entries that only carry `href` and
`title`. With no top-level `displayId` in `$source`, the frontend getter
falls through to the numeric id parsed from the href.
Emit `displayId` alongside `href`/`title` on each ancestor and child
link in the representer, and have the `displayId` getter fall back to
the self link's `displayId` so resources built from a link payload
alone still surface the semantic identifier.
We delegate the resource name to the object itself, that way we can
override it in our record and no knowledge about the backlogs module is
necessary in the core.
We already use our own extensive sanitization pipeline, and it turns out
tagfilter is extremely slow on large oneline input (such as big tables).
Rendering a large table inside a work package (2k rows, 20k cells) caused tagfilter to run for over 2 minutes.
Added a separate spec to test all tags the tagfilter filters for to avoid its runaway execution time.
Restore the minimal admin settings blankslate so the admin menu route
remains valid after the sprint-based cleanup. Remove the remaining
settings-driven story/task classification code, dead models and
services, and the obsolete filter and spec setup that depended on it.
Overriding wp.id to return the semantic identifier (e.g. "PROJ-42")
broke cache keys, API filters, row rendering, and CSS selectors that
all depend on the numeric PK.
Instead, keep wp.id as the numeric PK and add two new properties:
- displayId: returns the user-facing identifier ("PROJ-42" or "123")
- displayIdWithHash: returns "#PROJ-42" or "#123" for UI display
Also adds a COALESCE fallback in the SQL representer so work packages
created before semantic mode was enabled still get a valid displayId.
Alexander Brandon Coles