GORM: Shared Mapping Registry O(M+N) Scaling (clean rebuild)#15678
Open
borinquenkid wants to merge 22 commits into
Open
GORM: Shared Mapping Registry O(M+N) Scaling (clean rebuild)#15678borinquenkid wants to merge 22 commits into
borinquenkid wants to merge 22 commits into
Conversation
8 tasks
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>
Local issue-tracking files have no Apache license header and are not source artifacts. Add **/ISSUES.md to the RAT exclusion list alongside the existing local-tasks.gradle exclusion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…saction routing
- GormEnhancer: Remove generic type parameters (`<D>`) from deprecated API lookup methods (`findStaticApi`, `findInstanceApi`, `findValidationApi`). This fixes a `MissingMethodException` encountered
when older compiled code or dynamic Groovy proxies call these methods without exact signature matches.
- TransactionalTransform: Fix `IllegalStateException: No GORM implementations configured` in multi-tenant environments. Revert logic that incorrectly passed `tenantId` as a connection source
qualifier to `GormRegistry`, and instead fetch the tenant-specific datastore using `getTargetDatastore().getDatastoreForTenantId(tenantId)`.
- ServiceTransformation: Ensure the stateful `$datastore` field is properly generated for generic data services to resolve injection failures in tests.
borinquenkid
force-pushed
the
8.0.x-hibernate7.gorm-scaling-clean
branch
from
afa32a1 to
611e6bd
Compare
… artifacts
Fixes TCK DataServiceConnectionRoutingSpec failures introduced during the O(M+N)
scaling refactor. The decentralized API resolver changes caused getTargetDatastore(String)
to ignore the explicitly-injected $targetDatastore and route through the API resolver
instead, which could return a child datastore that has no knowledge of sibling connections.
- AbstractDatastoreMethodDecoratingTransformation: getTargetDatastore(String) now checks
$targetDatastore before falling back to the API resolver
- ServiceTransformation: generateConnectionAwareTransactionManager uses getTargetDatastore()
instead of getDatastore() for correct multi-datasource transaction manager resolution
- GrailsDataHibernate7TckManager: fix setTargetDatastore array overload to use
MultipleConnectionSourceCapableDatastore[] instead of Datastore[]
…) + entity re-registration in setup(), wired setTargetDatastore(multiDataSourceDatastore) in getServiceForConnection(), and wired setTargetDatastore(multiTenantMultiDataSourceDatastore) in getServiceForMultiTenantConnection(). This fixes 26 H5 failures. 2. GrailsDataHibernate7TckManager.groovy — Wired setTargetDatastore(multiTenantMultiDataSourceDatastore) in getServiceForMultiTenantConnection(). This fixes 5 H7 DataServiceMultiTenantConnectionRoutingSpec failures. 3. GormApiResolver.groovy — Removed 6 residual System.out.println debug statements from ActiveSessionDatastoreSelector. 4. ServiceTransformSpec.groovy — Added GormRegistry.reset() in setup()/cleanup() to prevent cross-spec registry pollution causing 3 flaky test failures.
Root cause: AutoTimestampEventListener.beforeUpdate() unconditionally sets lastUpdated/dateCreated on every PreUpdate event, even when the entity has no user-intent changes. This caused PendingUpdate.run() to see a changed property, add it to $set, and increment the optimistic-locking version — breaking the MarkDirtyFalseSpec contract that a no-op save() must not increment version. Fix in MongoCodecEntityPersister.persistEntity(): - Before cancelUpdate fires, capture a snapshot of all property values and whether the entity already had user-intent dirty state (hasPreExistingDirty). - After cancelUpdate, compare the snapshot to detect what changed. If the only changes are lastUpdated/dateCreated (auto-timestamp properties) with no pre-existing user-intent dirty state, restore those timestamps, call trackChanges(), and veto the update. - The veto condition requires onlyAutoTimestampChanged to be non-empty — entities without auto-timestamp properties (including those with embedded-only associations) are never incorrectly vetoed, even when the snapshot comparison misses embedded-object mutations (same object reference). - Also refactored persistEntity() to remove the unnecessary isUpdate && !session.isDirty(obj) early-return guard, which was incompatible with the snapshot logic. Other changes: - GrailsDataMongoTckManager: added GormRegistry.reset() in setup() for per-test datastore isolation (prevents stale GORM state polluting successive test features). - DirtyCheckingSupport: propagate dirty-checking into PersistentCollection items so collection-element mutations are visible to the parent entity's dirty check traversal. Tests verified: MarkDirtyFalseSpec, EmbeddedAssociationSpec, EmbeddedCollectionAndInheritanceSpec, EmbeddedCollectionWithOneToOneSpec, EmbeddedUnsetSpec, LastUpdatedSpec, BeforeUpdatePropertyPersistenceSpec, DirtyCheckUpdateSpec — plus full testSelected suite: 4588 tests, 0 failures.
…eDatastore Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
borinquenkid
force-pushed
the
8.0.x-hibernate7.gorm-scaling-clean
branch
from
c5f7c32 to
4358b70
Compare
…resolution
- currentGorm{Instance,Static,Validation}Api() throw IllegalStateException
when GORM is uninitialized, instead of returning null. Callers such as
DefaultLinkGenerator.getResourceId already catch IllegalStateException and
fall back; this restores the pre-O(M+N) contract and fixes NPEs in
VndErrorRenderingSpec, HalJsonRendererSpec, TableSpec and others.
- GormStaticApi.getGormPersistentEntity() falls back to the mapping context
captured at construction when registry resolution returns null. Entity
metadata is identical across tenants, so this stable reference fixes the
withTenant(tenantId).count() NPE for DISCRIMINATOR multi-tenancy under
cross-spec registry state (PartitionedMultiTenancySpec in the full suite).
- SimpleMapSession.isDirty() treats an identified instance absent from the
backing map as dirty so save() re-inserts it. Fixes the unit-test pattern of
saving @shared instances in setup() across feature iterations after
clearData() empties the in-memory datastore (DefaultInputRenderingSpec).
- DefaultHalViewHelper.renderEntityProperties excludes the version property from
embedded output, consistent with top-level GORM rendering. KeyValue entities
gained an auto-mapped version under the GORM mapping strategy (HalEmbeddedSpec).
- Remove stale @NotYetImplemented on UniqueConstraintOnHasOneSpec; unique-on-hasOne
now works, so the spec passes and the assertion is restored.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes five of the six failing Functional Tests CI tasks. Each was a core GORM contract that regressed in the shared-GormRegistry rewrite but surfaced far downstream; root-cause fixes are guarded by core unit tests. - SimpleMapSession: a rolled-back transaction set a session-level rollbackOnly flag that was never cleared, permanently turning flush() into a no-op on the (long-lived, thread-bound) test session. Subsequent save(flush:true) calls assigned an id and populated the first-level cache but never reached the backing map. Clear the marker on commit/rollback and reset it when a transaction begins. Adds SimpleMapSessionSpec coverage. Fixes the app1 BookControllerSpec save/delete count()==0 failures. - DataTest harness: the single shared GormRegistry resolves a domain mapped to a non-default datasource to a dedicated per-connection child datastore, but the unit-test interceptors only bound a session for the default datastore. Entities on non-default datasources ran in throwaway per-call sessions, so save() without an explicit flush was lost before an auto-flushing query. Bind (and symmetrically unbind) a session for every connection source; no-op for single-datasource specs. Adds NonDefaultDatasourceFlushSpec. Fixes demo33 CarSpec. - GormStaticApi.withTransaction(Map)/withNewTransaction(Map): replaced the broken definition.setProperty(key, value) call (no such method on the Java bean DefaultTransactionDefinition) with the property-set idiom definition[key]=value, plus CharSequence coercion and a clear error for unknown properties. Fixes CrossDatasourceTransactionSpec read-only transactions. - DefaultHalViewHelper: reverted the embedded-version exclusion. Embedded HAL output renders the version property for versioned entities (functional TeamSpec depends on it); the unit-level HalEmbeddedSpec only saw an extra version:0 because KeyValue entities now auto-map a version under the GORM mapping strategy, so its expectation is updated instead. - demo33 UniqueConstraintOnHasOneSpec: removed a stale @NotYetImplemented (a second copy of an already-fixed spec) that fails as "passes unexpectedly". - Bar/FooIntegrationSpec: assert datastore persistence through public, observable behavior (Mongo ObjectId / Hibernate sequential id) instead of the removed internal org.grails.datastore.gorm.GormEnhancer.findStaticApi probe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Removed unused imports and redundant code from ServiceTransformation and HibernateDatastore to fix linting violations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ging capture - Update ActiveSessionDatastoreSelector in GormApiResolver to correctly bypass the parent DEFAULT datastore when routing multi-tenant entity operations in DATABASE/SCHEMA modes, without inadvertently skipping explicitly opened child datastores. This resolves transaction auto-commit issues in DatabasePerTenantIntegrationSpec. - Adjust GraphQL CommentIntegrationSpec StringMessagePrintStream output capture to filter out SLF4J deprecation warnings (HHH90000022: Hibernate's legacy org.hibernate.Criteria API is deprecated), preventing false-positive query over-counting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- GormApiResolver: Ensure PreferredDatastoreSelector performs tenant validation for DEFAULT qualifier when the preferred datastore is multi-tenant capable, preventing static queries like count() from incorrectly querying the parent datastore in an active transaction. - GrailsHibernateTransactionManager (H5): Reapply the isExistingTransaction override from H7 to ensure transactions bound to the parent SessionFactory are not mistakenly reused for tenant-specific DataSources.
The integration tests were capturing and counting all System.out logs as queries, causing Hibernate deprecation warnings to fail the assertions. Filtered capture to explicitly require 'Hibernate:'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In Gradle 9, ProjectDependency.getDependencyProject() has been removed. Updated gradle/test-config.gradle to recursively identify any subprojects depending on ':grails-datamapping-core' (GORM) using path resolution, and force their test tasks to run with maxParallelForks = 1. This prevents singleton GormRegistry and local resource conflicts in CI while preserving parallel execution for all other modules. Co-Authored-By: Gemini <noreply@google.com>
…fix Hibernate 5 schema-per-tenant resource unbinding
- Remove Hibernate 5/7, MongoDB, and Neo4j datastores from GormRegistry upon destroy/close to prevent scalability/leak issues.
- Unbind the tenant-specific DataSource from TransactionSynchronizationManager in Hibernate 5 schema-per-tenant setup to prevent thread connection bound state conflicts.
- Restore previous connection holder correctly in GrailsHibernateTemplate regardless of session holder existence.
Co-Authored-By: Gemini <noreply@google.com>
…n exists Update Tenants.withId to check if the child datastore for the tenant already has a current session active on the thread. If so, execute the closure directly rather than forcing a new session, which preserves transaction propagation for multi-tenant integrations. Also, clean up debug print statements and use protected logger.debug statements under the log.isDebugEnabled() check.
…ling - Restore connection routing to use getDatastoreForConnection instead of getDatastoreForTenantId in AbstractDatastoreMethodDecoratingTransformation. - Align SimpleMapDatastore getDatastoreForTenantId with relational/document tenancy invariants by returning this for non-DATABASE modes. - Avoid StackOverflowError in transactional method decoration by using AttributeExpression (direct field access) to access the transactionManager field. - Prevent duplicate transactionManager field/method weaving in subclasses when already inherited or declared on the superclass. - Update GormRegistry lookup methods to return null instead of throwing IllegalStateException on missing qualifiers if the defaultDatastore is initialized. - Expose defaultDatastore property in GormRegistry. - Update ISSUES.md.
🚨 TestLens detected 6 failed tests 🚨Here is what you can do:
Test Summary
🏷️ Commit: 96ee901 Test FailuresDatabasePerTenantSpec > Test should rollback changes in a previous test (:grails-test-examples-hibernate5-grails-database-per-tenant:test in CI - Groovy Joint Validation Build / build_grails)PartitionedMultiTenancySpec > Test partitioned multi tenancy (:grails-data-hibernate5-core:test in CI - Groovy Joint Validation Build / build_grails)ProxySpec > Test Proxy (:grails-test-examples-hibernate5-grails-hibernate-groovy-proxy:test in CI - Groovy Joint Validation Build / build_grails)SchemaMultiTenantSpec > Test a database per tenant multi tenancy (:grails-data-hibernate7-core:test in CI - Groovy Joint Validation Build / build_grails)SchemaPerTenantSpec > Test should rollback changes in a previous test (:grails-test-examples-hibernate5-grails-schema-per-tenant:test in CI - Groovy Joint Validation Build / build_grails)SingleTenantSpec > Test a database per tenant multi tenancy (:grails-data-hibernate7-core:test in CI - Groovy Joint Validation Build / build_grails)Muted TestsSelect 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