Skip to content

Update ESContentletAPIImpl.java#35407

Merged
dsilvam merged 1 commit intorelease-24.12.27_ltsfrom
fd-36004-slow-saving
Apr 22, 2026
Merged

Update ESContentletAPIImpl.java#35407
dsilvam merged 1 commit intorelease-24.12.27_ltsfrom
fd-36004-slow-saving

Conversation

@erickgonzalez
Copy link
Copy Markdown
Member

@erickgonzalez erickgonzalez commented Apr 21, 2026

When saving a Promotion with many required relationships (e.g. 499
PromotionRegisters), validateRelationships() called getRelatedContent()
for every child contentlet in the isParent loop — including MANY_TO_MANY
cardinalities where the result was never used. The limit=1 parameter was
silently ignored by the RelationshipCache path, causing
filterRelatedContentByLiveAndLanguage() to fire one DB query per
identifier per configured language. With 528 related identifiers and 44
languages this produced ~23K queries per save, accounting for 436 of
~641 seconds observed in Glowroot traces.

Fix: guard the query with the ONE_TO_MANY cardinality check (skipping it
entirely for other cardinalities) and replace getRelatedContent() with
FactoryLocator.getRelationshipFactory().dbRelatedContent(..., 1, 0),
which queries the tree + contentlet_version_info tables directly with
LIMIT 1, bypassing the cache path.

Screen.Recording.2026-04-21.at.4.49.32.PM.mov

@fabrizzio-dotCMS
Copy link
Copy Markdown
Member

Leftover issues not addressed by this fix

1. RelationshipFactoryImpl — unnecessary Contentlet hydration (dbRelatedContentByChild lines 655, 695)

dbRelatedContentByChild selects cvi.working_inode and then calls contentletAPI.find(inode, ...) to hydrate a full Contentlet per row. In validateRelationships, only Contentlet.getIdentifier() is used from the result — which maps directly to t.parent, already present in the tree table.

A dedicated method returning List<String> identifiers instead of List<Contentlet> would eliminate the secondary find() call entirely:

// proposed signature
List<String> dbRelatedContentIdentifiers(
        Relationship rel, Contentlet child,
        boolean hasParent, int limit, int offset) throws DotDataException;

SQL simplification:

-- current: fetches working_inode, then find(inode) per row
SELECT cvi.working_inode AS inode
FROM tree t, contentlet_version_info cvi
WHERE t.child = ? AND t.relation_type = ?
  AND cvi.working_inode IS NOT NULL
  AND t.parent = cvi.identifier

-- proposed: t.parent IS the identifier, no secondary find() needed
SELECT t.parent
FROM tree t
WHERE t.child = ? AND t.relation_type = ?

2. ESContentletAPIImpl.getRelatedContent — root cause of the query explosion is still present (lines 4545–4556)

The fix bypasses the cache for this specific call, but the underlying bug remains in getRelatedContent: when the relationship cache is warm the limit parameter is silently dropped before entering getCachedRelatedContentlets:

if (relatedIds.containsKey(variableName)) {
    // limit is NOT forwarded here  ← root cause
    relatedContentlet = getCachedRelatedContentlets(relatedIds, variableName, language, live);
} else {
    // limit is passed correctly here
    relatedContentlet = getNonCachedRelatedContentlets(..., limit, offset, ...);
}

getCachedRelatedContentlets then expands every cached identifier across all configured languages with no upper bound:

return relatedIds.get(variableName).stream()
    .flatMap(identifier -> filterRelatedContentByLiveAndLanguage(language, live, identifier))
    .collect(Collectors.toList()); // no .limit(n) anywhere

Any other caller of getRelatedContent() that passes a limit > 0 and has a warm cache is still exposed to the same N × languages query explosion. This PR patches the symptom for validateRelationships but leaves the root cause intact.

@fabrizzio-dotCMS fabrizzio-dotCMS marked this pull request as ready for review April 22, 2026 18:11
Copy link
Copy Markdown
Member

@fabrizzio-dotCMS fabrizzio-dotCMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm approving as it looks sound for an LTS
However, I'm leaving a few notes that should be taken into account when taking this fix to main

@dsilvam dsilvam merged commit 0c188d2 into release-24.12.27_lts Apr 22, 2026
8 of 10 checks passed
@dsilvam dsilvam deleted the fd-36004-slow-saving branch April 22, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Backport #35407 to release-24.12.27_lts: Fix validated on 24.12.27_lts_v19

3 participants