GORM: Shared Mapping Registry O(M+N) Scaling (clean rebuild)#15678
Open
borinquenkid wants to merge 4 commits into
Open
GORM: Shared Mapping Registry O(M+N) Scaling (clean rebuild)#15678borinquenkid wants to merge 4 commits into
borinquenkid wants to merge 4 commits into
Conversation
Introduces shared-registry architecture to eliminate per-tenant API wrapper duplication in multi-tenant environments with high entity/tenant cardinality. Core changes: - GormRegistry: normalization caches (entity keys, qualifiers), O(1) lookup paths - GormApiResolver: simplified fallback chains, qualified API caching - AbstractGormApiRegistry/sub-registries: normalized key/qualifier registration - GormEnhancer: delegates API resolution through GormRegistry Datastore integrations: - Hibernate 7 and Hibernate 5: aligned to shared registry model - MongoDB, Neo4j, SimpleMap, GraphQL: registry-pattern integration Adjacent migrations: - AsyncEntity: GormEnhancer.findStaticApi -> GormRegistry.instance.findStaticApi - ByDatasourceDomainClassFetcher: GormEnhancer.findDatastore -> GormRegistry apiResolver - TCK: added transaction-capable datastore support in GrailsDataTckManager This commit excludes all collateral CodeNarc reformat changes (2,835 files from commit 4add87e) and agent experiments, containing only the optimization-specific module changes. Agent collaboration note: Claude Sonnet 4.6 assisted with branch archaeology and rebuild strategy; borinquenkid is the primary author and remains responsible for the final changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Includes SessionResolver and ThreadLocalSessionResolver (new interfaces/classes introduced by the O(M+N) scaling refactor), plus updates to AbstractDatastore, AbstractMappingContext, and related core classes that the datastore modules (SimpleMap, Hibernate 5/7) depend on at compile time. Missed from initial clean rebuild commit. Agent collaboration note: Claude Sonnet 4.6 assisted; borinquenkid is the primary author and remains responsible for the final changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
During child datastore construction, GormRegistry.registerEntityDatastores calls getDatastoreForConnection() before the parent's datastoresByConnectionSource map is populated, throwing ConfigurationException for multi-datasource setups. H5 anonymous child: add self-reference check so the child returns itself when asked for its own connection name, rather than delegating to the parent map. H7 ChildHibernateDatastore: use PARENT_HOLDER ThreadLocal to pass the parent reference through the super() call before the parent field is assigned; also pass the parent's datastoresByConnectionSource map to HibernateGormEnhancer so it can resolve sibling datastores during initialize(). Fixes DataSource not found for name [secondary/schemaA] ConfigurationException in multi-datasource and schema-per-tenant multi-tenancy test suites. Agent collaboration note: Claude Sonnet 4.6 assisted; borinquenkid is the primary author and remains responsible for the final changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The O(M+N) GormRegistry refactor exposed two classes of regression in multi-tenancy and multi-datasource scenarios. This commit addresses both. Production fixes: GormApiResolver: Move the DISCRIMINATOR mode check before the MultipleConnectionSourceCapableDatastore delegation so that tenant IDs are never mistaken for datasource connection names. For the DEFAULT qualifier, return the preferred (active-transaction) datastore directly rather than re-routing through getDatastoreForConnection, which would return the parent and mismatch the session factory already bound to the transaction. GormRegistry.registerEntityDatastores: Stop overwriting child datastores with the parent for non-DEFAULT qualifiers that resolve back to the parent. In SCHEMA and DISCRIMINATOR mode the qualifier is a runtime tenant ID, not a datasource name; routing it back to the parent is correct and must not clobber the child entries added by addTenantForSchemaInternal. GormRegistry.findTransactionManager: Fall back through the full apiResolver when getDatastore returns null so that DISCRIMINATOR/SCHEMA tenant IDs still resolve to a transaction manager. HibernateDatastore (H5) / ChildHibernateDatastore (H7): Return null instead of throwing ConfigurationException when getDatastoreForConnection is called for a sibling that is not yet registered during initialization. GormRegistry will re-register all entities with the correct datastores once initialization completes. Child datastores also delegate to the parent for unrecognized connection names so the lookup chain stays consistent. HibernateGormInstanceApi (H7): Always resolve the template via the datastore registry rather than caching a DEFAULT-qualifier instance, so that preferred-datastore switching in multi-datasource transactions picks up the correct session factory. GrailsHibernateTransactionManager (H7): Remove debug System.err.println statements left over from investigation. Test infrastructure fixes: gradle/hibernate5-test-config.gradle, gradle/hibernate7-test-config.gradle: Set forkEvery = 1 so each test class runs in its own JVM. The root test-config.gradle uses forkEvery = 50 (CI) / 100 (local) for speed; with a shared GormRegistry singleton that per-test setup/teardown mutates, TCK specs running before PartitionedMultiTenancySpec in the same JVM were clearing datastoresByQualifier["default"], causing a NullPointerException in count() when PartitionedMultiTenancySpec later resolved a GormPersistentEntity. forkEvery = 1 eliminates cross-class singleton contamination at the cost of extra JVM startup overhead, which is acceptable given the test isolation requirement. GrailsDataHibernate5TckManager: Add grailsConfig field and populate a local ConfigObject from it in createSession(), fixing MissingPropertyException when test specs assign grailsConfig before calling setup(). Verified: H5 669 tests / 0 failures, H7 2960 tests / 0 failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8 tasks
🚨 TestLens detected 115 failed tests 🚨Here is what you can do:
Test Summary (first 80 of 115)
🏷️ Commit: dcaff86 Test Failures (first 10 of 115)GormApiResolverSpec > resolver honors preferred datastore for default qualifier (:grails-datamapping-core:test in CI / Build Grails-Core (macos-latest, 21))HalJsonRendererSpec > Test customizing the embedded name for a rendered collection of domain objects (:grails-rest-transforms:test in CI / Build Grails-Core (macos-latest, 21))HalJsonRendererSpec > Test that HAL renders JSON correctly for eagerly loaded domain objects (:grails-rest-transforms:test in CI / Build Grails-Core (macos-latest, 21))HalJsonRendererSpec > Test that the HAL rendered renders JSON values correctly for collection (:grails-rest-transforms:test in CI / Build Grails-Core (macos-latest, 21))HalJsonRendererSpec > Test that the HAL renderer ignores null values for embedded single ended domain objects (:grails-rest-transforms:test in CI / Build Grails-Core (macos-latest, 21))HalJsonRendererSpec > Test that the HAL renderer renders JSON values correctly for collections with a many-to-one association (:grails-rest-transforms:test in CI / Build Grails-Core (macos-latest, 21))HalJsonRendererSpec > Test that the HAL renderer renders JSON values correctly for domains (:grails-rest-transforms:test in CI / Build Grails-Core (macos-latest, 21))HalJsonRendererSpec > Test that the HAL renderer renders mixed fields (dates, enums) successfully for domains (:grails-rest-transforms:test in CI / Build Grails-Core (macos-latest, 21))ServiceTransformSpec > test @query annotation with declared variables (:grails-datamapping-core:test in CI / Build Grails-Core (macos-latest, 21))ServiceTransformSpec > test @query annotation with projection (:grails-datamapping-core:test in CI / Build Grails-Core (macos-latest, 21))Muted Tests (first 20 of 115)Select tests to mute in this pull request:
Reuse successful test results:
Click the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR replaces #15656, which was rebuilt on a clean branch to remove a corruption commit from the history.
Problem: In multi-tenant GORM environments with many tenants (M) and domain classes (N), the previous implementation instantiated a full set of static API, instance API, and validation API objects per tenant per entity, producing O(M × N) object allocations. This caused excessive memory consumption and degraded startup performance as tenant counts scaled.
Solution: Refactor
GormRegistryto use a single shared registry keyed by entity class name and qualifier (datasource/tenant), with the GORM API objects created once per entity per qualifier and reused across tenants. This reduces the allocation profile to O(M + N).This rebuild also fixes two regression classes exposed by the new registry:
Multi-tenancy resolution regressions —
GormApiResolverandGormRegistry.registerEntityDatastoreswere routing DISCRIMINATOR/SCHEMA tenant IDs through the datasource connection lookup path, causing child datastores to be overwritten by the parent andPartitionedMultiTenancySpec.count()to NPE. Fixed by detecting multi-tenancy mode before delegating togetDatastoreForConnection, and by skipping non-DEFAULT qualifier registration when the qualifier resolves back to the parent (i.e., it's a runtime tenant ID, not a datasource name).Child datastore initialization order —
HibernateDatastore(H5) andChildHibernateDatastore(H7) were throwingConfigurationExceptionwhengetDatastoreForConnectionwas called for a sibling during initialization before all children were registered. Fixed to returnnullduring the initialization phase soGormRegistryfalls back gracefully and re-registers once initialization completes.Test infrastructure: Added
forkEvery = 1togradle/hibernate5-test-config.gradleandgradle/hibernate7-test-config.gradle. The root config usesforkEvery = 50/100for speed, but with a sharedGormRegistrysingleton, TCK specs running in the same JVM beforePartitionedMultiTenancySpecwere clearingdatastoresByQualifier["default"]and causing the NPE described above. Each test class now gets its own JVM.Verified: H5 — 669 tests / 0 failures. H7 — 2960 tests / 0 failures.
Contributor Checklist
Issue and Scope
8.0.x-hibernate7— major release branch; breaking API changes permitted).Code Quality
./gradlew codeStylehas been run and violations resolved.Licensing and Attribution
Documentation