Reduce GORM API allocation for tenant qualifiers to O(entityCount + tenantCount) by jamesfredley · Pull Request #15771 · apache/grails-core

jamesfredley · 2026-06-26T19:58:43Z

Summary

GormEnhancer.registerEntity eagerly instantiated a static, instance, and validation API provider for every (entity × qualifier) pair. For a MultiTenant entity expanded across N tenant connection sources this is O(entityCount × tenantCount) provider allocations at startup. This PR makes per-qualifier provider creation lazy - collapsing startup allocation to O(entityCount + tenantCount) - and fixes two transaction-synchronisation defects that lazy allocation exposed.

Where the eager O(M×N) actually occurs

Multi-tenancy mode	Eager O(entityCount × tenantCount) at startup?
DISCRIMINATOR (column)	No - single shared connection; a `MultiTenant` entity expands to `[DEFAULT]` only
SCHEMA (schema-per-tenant)	Yes (typical production) - schema-mode `allQualifiers()` resolves existing schemas from the live database at startup via `schemaHandler.resolveSchemaNames()`; with N pre-registered schemas the original code created N×M providers
DATABASE (database-per-tenant)	Yes - statically-configured per-tenant datasources expand every `MultiTenant` entity across all N connection sources

(h/t @borinquenkid for the SCHEMA clarification - the live-schema resolution means SCHEMA benefits from this optimisation too, not just DATABASE.)

The optimisation

apiQualifiers() restricts eager provider creation to the canonical qualifier set while still registering datastore routing for every qualifier. Per-qualifier static/instance/validation providers are then created lazily on first access via computeIfAbsent (single-flight, per the concurrency review feedback).

Transaction-synchronisation fixes required by lazy allocation

DATABASE per-tenant - "Already value bound to thread". Spring's SQLErrorCodesFactory eagerly acquires a connection via DataSourceUtils during GrailsHibernateTemplate construction while a parent-transaction synchronisation is active, binding the child DataSource to the TransactionSynchronizationManager. executeWithNewSession only unbound the DataSource when a SessionFactory was also bound, so a stale ConnectionHolder remained and HibernateTransactionManager.doBegin threw Already value bound. The DataSource unbind/rebind is now decoupled from the SessionFactory null-check (hibernate5 + hibernate7).
SCHEMA per-tenant - "No Session found". addTenantForSchema re-creates a child datastore (new SessionFactory) and re-registers entities, but lazy-cached providers for non-eager qualifiers still referenced the previous SessionFactory. GormEnhancer.registerEntity now evicts those stale entries so they are rebuilt against the current datastore on next access.

Tests

GormEnhancerAllQualifiersSpec (datamapping-core) - qualifier expansion + eager/lazy allocation counts.
GormApiAllocationSpec (hibernate5 + hibernate7) - mirrored coverage including per-tenant session behaviour.

Locally verified green: H5/H7 SchemaPerTenantIntegrationSpec and DatabasePerTenantIntegrationSpec.

Copilot

Pull request overview

This PR reduces eager GORM API provider allocation when entities expand across many connection-source and multi-tenant-related qualifiers, while preserving datastore routing entries for all qualifiers and lazily creating qualifier-specific APIs on first lookup.

Changes:

Add lazy initialization for static/instance/validation GORM APIs when a qualifier is first requested, backed by a datastore→enhancer lookup.
Introduce apiQualifiers(...) to limit eager API allocation for ConnectionSource.ALL and default-mapped MultiTenant entities while still registering routing entries for all qualifiers.
Add/extend allocation coverage tests in datamapping-core and add mirrored Hibernate 5 / Hibernate 7 allocation specs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
`grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormEnhancer.groovy`	Adds enhancer registry + lazy API initialization and adjusts registration to avoid eager per-qualifier API creation.
`grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/GormEnhancerAllQualifiersSpec.groovy`	Extends qualifier/allocation tests to assert lazy creation behavior across expanded qualifier sets.
`grails-data-hibernate5/core/src/test/groovy/org/grails/orm/hibernate/connections/GormApiAllocationSpec.groovy`	New Hibernate 5 coverage asserting lazy API allocation across multi-tenancy/datasource combinations.
`grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/connections/GormApiAllocationSpec.groovy`	New Hibernate 7 coverage asserting lazy API allocation across multi-tenancy/datasource combinations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

borinquenkid · 2026-06-27T00:40:35Z

@jamesfredley If you chain the PRs and the final PR works then we can roll them in reverse order

jamesfredley · 2026-06-27T01:34:43Z

@borinquenkid I think this one broke following @copilots feedback will have it working again later tonight and it should still be just a handful or two of files to fix the O(entityCount * qualifierCount) issue.

borinquenkid · 2026-06-27T01:47:19Z

@borinquenkid I think this one broke following @copilots feedback will have it working again later tonight and it should still be just a handful or two of files to fix the O(entityCount * qualifierCount) issue.

Do you mind if I tackle it? I am working on it.

jamesfredley · 2026-06-27T12:20:22Z

@borinquenkid by all means, I am just trying to see if there is a more compact way to achieve the same ends on this PR, it appeared to be working great, prior to making the copilot changes yesterday, proceed on your PRs.

We can compare later.

jamesfredley · 2026-06-27T14:54:13Z

@borinquenkid as I have dug into this O(M×N) only occurs with DATABASE (database-per-tenant), which is why I had never seen it in my apps.

Mode	O(M×N) at startup?
DISCRIMINATOR (column)	No (single shared connection)
SCHEMA (schema-per-tenant)	No (tenants added dynamically at runtime)
DATABASE (database-per-tenant)	Yes (static per-tenant datasources)

borinquenkid · 2026-06-27T15:13:29Z

@borinquenkid as I have dug into this O(M×N) only occurs with DATABASE (database-per-tenant), which is why I had never seen it in my apps.

Mode O(M×N) at startup?
DISCRIMINATOR (column) No (single shared connection)
SCHEMA (schema-per-tenant) No (tenants added dynamically at runtime)
DATABASE (database-per-tenant) Yes (static per-tenant datasources)

@jamesfredley
Good catch — one addition to the SCHEMA row: allQualifiers() calls schemaHandler.resolveSchemaNames() against the live database at startup, so if N schemas already exist (the typical production case), the original code did create N×M APIs eagerly there too. The apiQualifiers optimisation in this PR collapses that to M for SCHEMA mode as well

We've also landed a fix for the CI failures the optimisation introduced: stale lazy-cached APIs were left in STATIC_APIS after addTenantForSchema re-created a child datastore, causing "No Session found" on the second test. GormEnhancer.registerEntity now evicts those entries for non-eager qualifiers so they're rebuilt against the current session factory on next access.

jamesfredley · 2026-06-27T15:23:24Z

@borinquenkid This looks solid. I am cleaning up the history and updating the PR description and then will ask for PR reviews.

borinquenkid · 2026-06-27T15:43:01Z

@jamesfredley The current PR also deals with ChilldDatastore but I am adding unit tests for - GormApiAllocationSpec: Tests that APIs are initially absent and get created lazily on first access. No test for stale cache invalidation when registerEntity is called again after addTenantForSchema re-creates the child datastore.

GrailsHibernateTemplateSpec: Tests general template behavior. No test for executeWithNewSession when a DataSource holder is pre-bound in TSM without a matching SessionFactory holder (the DATABASE per-tenant scenario).

jamesfredley · 2026-06-27T16:00:45Z

@borinquenkid does #15776 solve the child data store as a separate PR?

…enantCount) GormEnhancer.registerEntity eagerly instantiated a static, instance, and validation API provider for every (entity, qualifier) pair. For a MultiTenant entity expanded across N tenant connection sources this is O(entityCount * tenantCount) provider allocations at application startup. Where the eager fan-out occurs: DISCRIMINATOR (column) No - single shared connection; entities expand to [DEFAULT] only SCHEMA (schema-per-tenant) Yes - allQualifiers() resolves existing schemas from the live database at startup (schemaHandler.resolveSchemaNames); with N pre-registered schemas it allocated N*M DATABASE (database-per-tenant) Yes - statically-configured per-tenant datasources expand every entity across N apiQualifiers() now restricts eager provider creation to the canonical qualifier set while still registering datastore routing for every qualifier; per-qualifier providers are created lazily on first access via computeIfAbsent (single-flight), collapsing startup allocation to O(entityCount + tenantCount). Adds GormEnhancerAllQualifiersSpec (datamapping-core) and mirrored GormApiAllocationSpec for hibernate5 and hibernate7. Assisted-by: claude-code:claude-opus-4.8

Two bugs surfaced when lazy GORM API allocation was introduced for tenant qualifiers: 1. DATABASE per-tenant: SQLErrorCodesFactory eagerly acquires a connection via DataSourceUtils during GrailsHibernateTemplate construction while a parent-transaction synchronisation is already active, binding the child DataSource to TSM. The original executeWithNewSession code only unbound the DataSource when the SessionFactory was also bound, so the stale ConnectionHolder was still present when HibernateTransactionManager called doBegin, causing "Already value bound" for the child connection. Fix: decouple the DataSource unbind/rebind from the SessionFactory null-check so both are restored independently (hibernate5 + hibernate7). 2. SCHEMA per-tenant: addTenantForSchema re-creates a child datastore (new SessionFactory) on every test setup. registerAllEntitiesWithEnhancer updated DATASTORES routing but not STATIC_APIS/INSTANCE_APIS/VALIDATION_APIS for non-eager schema qualifiers, leaving a stale API referencing the old SessionFactory. The next withNewSession bound the new SF to TSM, but findStaticApi returned the cached API for the old SF, so getCurrentSession() found nothing. Fix: in GormEnhancer.registerEntity, evict the stale lazy-cached API entries for any qualifier that is not in apiQualifiers, forcing re-creation against the current datastore on next access. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

jamesfredley · 2026-06-27T16:44:31Z

@jdaugherty @matrei This solves the O(entityCount + tenantCount) issue with only 7 file changes. I started the PR and @borinquenkid solved the final challenge.

There will be another, hopefully small PR, for a child data source issue.

borinquenkid · 2026-06-27T16:55:58Z

We might not need the ChildDataStore changes

…ng fixes Cover the two bugs fixed in the previous commit: 1. GrailsHibernateTemplate.executeWithNewSession TSM bug (H5 + H7): - H5: standalone spec; manually pre-binds a ConnectionHolder for the DataSource without a matching SessionFactory holder in TSM, then verifies executeWithNewSession completes without "Already value bound". - H7: uses the outer transaction's active synchronisation; a secondary local datastore triggers SQLErrorCodesFactory to auto-bind its DS to TSM, replicating the production trigger without manual bindResource. 2. GormEnhancer.registerEntity stale API eviction (H5 + H7): - H5: calls addTenantForSchema('tenantA') twice (H5 has no idempotency guard, so the second call re-creates the child and re-registers entities); verifies stale lazily-cached APIs for tenantA are evicted. - H7: creates a second SCHEMA datastore for the same entity class without closing the first; verifies that SchemaTenantGormEnhancer.registerEntity evicts stale lazily-cached tenant APIs on the new datastore's registration pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…store init Wires Hibernate 7 to the new GormRegistry with correct child datastore initialization order, fixing the gaps that PR #15771 intentionally deferred: - HibernateDatastore (H7): registers child datastores with GormRegistry eagerly during initialization, preserving schema/database/datasource routing - HibernateGormInstanceApi: use getHibernateTemplate() lazy getter for isAttached(), lock(), and attach() — factory constructor leaves the field null - HibernateGormStaticApi: executeQuery(query, Map) passes params as both named params and query settings so max/offset pagination is applied - HibernateSession: coerceId() converts String IDs to the entity's declared identifier type in retrieveAll(), fixing String→Long lookup mismatch - HibernateGormEnhancerSpec: updated to use GormRegistry public API - HibernateDirtyCheckingSpec / HibernateUpdateFromListenerSpec: local Person renamed to DirtyCheckPerson to avoid DuplicateMappingException with TCK - grails-test-examples/hibernate7: demo33 UniqueConstraintOnHasOneSpec @NotYetImplemented removed (test now passes); GormAdvancedSpec pagination fixed - grails-data-hibernate7-core GormApiAllocationSpec: verifies O(M+N) API count across single-datasource, multi-datasource, and multi-tenant configurations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

borinquenkid · 2026-06-27T18:33:45Z

I am adding some missing tests on GormEnhancerSpec

GormEnhancer.registerEntity was changed to allocate APIs lazily via computeIfAbsent, but GormEnhancerSpec in the TCK was not updated to cover that new code path. Add two feature methods verifying that findStaticApi/findInstanceApi/findValidationApi return non-null APIs after enhancement and return the same cached instance on repeated calls. Tests verified passing across all four TCK-consuming modules: grails-datamapping-core-test, grails-data-hibernate5, grails-data-hibernate7, grails-data-mongodb. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…store init Wires Hibernate 7 to the new GormRegistry with correct child datastore initialization order, fixing the gaps that PR #15771 intentionally deferred: - HibernateDatastore (H7): registers child datastores with GormRegistry eagerly during initialization, preserving schema/database/datasource routing - HibernateGormInstanceApi: use getHibernateTemplate() lazy getter for isAttached(), lock(), and attach() — factory constructor leaves the field null - HibernateGormStaticApi: executeQuery(query, Map) passes params as both named params and query settings so max/offset pagination is applied - HibernateSession: coerceId() converts String IDs to the entity's declared identifier type in retrieveAll(), fixing String→Long lookup mismatch - HibernateGormEnhancerSpec: updated to use GormRegistry public API - HibernateDirtyCheckingSpec / HibernateUpdateFromListenerSpec: local Person renamed to DirtyCheckPerson to avoid DuplicateMappingException with TCK - grails-test-examples/hibernate7: demo33 UniqueConstraintOnHasOneSpec @NotYetImplemented removed (test now passes); GormAdvancedSpec pagination fixed - grails-data-hibernate7-core GormApiAllocationSpec: verifies O(M+N) API count across single-datasource, multi-datasource, and multi-tenant configurations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

jamesfredley · 2026-06-29T16:46:54Z

This looks complete to solve O(entityCount + tenantCount)

testlens-app · 2026-06-29T17:46:42Z

✅ All tests passed ✅

🏷️ Commit: 947c045
▶️ Tests: 14892 executed
⚪️ Checks: 44/44 completed

Learn more about TestLens at testlens.app.

borinquenkid

@jamesfredley This has been discussed extensively. I have a followup PR to refactor and not just patch so when merged don't squash commits please.

Copilot AI review requested due to automatic review settings June 26, 2026 19:58

github-project-automation Bot added this to Apache Grails Jun 26, 2026

Copilot started reviewing on behalf of jamesfredley June 26, 2026 19:59 View session

jamesfredley mentioned this pull request Jun 26, 2026

GORM: Shared Mapping Registry O(M+N) Scaling (clean rebuild) #15678

Closed

14 tasks

Copilot AI reviewed Jun 26, 2026

View reviewed changes

jamesfredley changed the title ~~Reduce GORM API allocation for tenant qualifiers~~ Reduce GORM API allocation for tenant qualifiers: O(entityCount * qualifierCount) Jun 26, 2026

borinquenkid mentioned this pull request Jun 26, 2026

feat: GORM O(M+N) scaling — Hibernate 7 adapter wiring and child data… #15774

Closed

16 tasks

jamesfredley and others added 2 commits June 27, 2026 12:32

jamesfredley force-pushed the fix/gorm-api-registration-scaling branch from 0bfd35d to f02f643 Compare June 27, 2026 16:35

jamesfredley changed the title ~~Reduce GORM API allocation for tenant qualifiers: O(entityCount * qualifierCount)~~ Reduce GORM API allocation for tenant qualifiers to O(entityCount + tenantCount) Jun 27, 2026

jamesfredley assigned jamesfredley and borinquenkid Jun 27, 2026

jamesfredley requested review from borinquenkid, jdaugherty and matrei June 27, 2026 16:42

jamesfredley moved this to In Progress in Apache Grails Jun 27, 2026

jamesfredley added this to the grails:8.0.0-M3 milestone Jun 27, 2026

jamesfredley mentioned this pull request Jun 27, 2026

Guard child datastore entity registration against a null GORM enhancer (late registration NPE) #15776

Merged

Merge branch '8.0.x' into fix/gorm-api-registration-scaling

947c045

jamesfredley mentioned this pull request Jun 30, 2026

(2/3) feat: GORM O(M+N) scaling — GormRegistry, GormEnhancer, and core API refactor #15780

Open

2 tasks

borinquenkid approved these changes Jul 2, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

jamesfredley commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Where the eager O(M×N) actually occurs

The optimisation

Transaction-synchronisation fixes required by lazy allocation

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

borinquenkid commented Jun 27, 2026

Uh oh!

jamesfredley commented Jun 27, 2026

Uh oh!

borinquenkid commented Jun 27, 2026

Uh oh!

jamesfredley commented Jun 27, 2026

Uh oh!

jamesfredley commented Jun 27, 2026

Uh oh!

borinquenkid commented Jun 27, 2026

Uh oh!

jamesfredley commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

borinquenkid commented Jun 27, 2026

Uh oh!

jamesfredley commented Jun 27, 2026

Uh oh!

jamesfredley commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

borinquenkid commented Jun 27, 2026

Uh oh!

borinquenkid commented Jun 27, 2026

Uh oh!

jamesfredley commented Jun 29, 2026

Uh oh!

testlens-app Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All tests passed ✅

Uh oh!

borinquenkid left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jamesfredley commented Jun 26, 2026 •

edited

Loading

jamesfredley commented Jun 27, 2026 •

edited

Loading

jamesfredley commented Jun 27, 2026 •

edited

Loading

testlens-app Bot commented Jun 29, 2026 •

edited

Loading

borinquenkid left a comment •

edited

Loading