fix: declare synchronized entity classes on native writes DHIS2-21963

[teleivo]

The bug

A native query that calls executeUpdate() without declaring which tables it touches makes Hibernate discard every L2 entity region, not just the one it wrote to.

// evicts every cached entity in the JVM
getSession().createNativeQuery("update jobconfiguration set ...").executeUpdate();

// evicts only the JobConfiguration region
getSession().createNativeQuery("update jobconfiguration set ...")
    .addSynchronizedEntityClass(JobConfiguration.class)
    .executeUpdate();

HibernateNativeStore.nativeSynchronizedQuery already exists for exactly this and its javadoc says so ("Use this to avoid all Hibernate second level caches from being invalidated"), but these call sites bypass it.

About 32 call sites share this shape and all do the same damage. The one that matters most is HousekeepingJob, only because of how often it runs: every 20 seconds by default (JobType.java#L120), with all writes unsynchronized, so every cached entity region is dropped on that cadence no matter what the instance is doing. The rest are mostly admin-driven merges and deletes that wipe just as thoroughly but rarely enough to go unnoticed. This PR fixes all of them; a scan for createNativeQuery(...).executeUpdate() without an addSynchronized* now returns nothing.

Before / after

Reproducible from master on a Sierra Leone database, with no code or test-module changes. The only requirement is two dhis.conf flags to expose the counters:

monitoring.hibernate.enabled = on
monitoring.ehcache.enabled   = on

Input. One request, which reads every data element so it touches every key in the DataElement region:

GET /api/dataElements?fields=id,name,optionSet&paging=false

Numbers below are from a database widened to 3037 data elements for unrelated tracker load tests; stock Sierra Leone has 1037.

Cadence. 9 requests, with a 30s idle gap between them:

for i in $(seq 1 8); do request; sleep 30; done
request

30s comfortably clears the 20s housekeeping interval, so the window spans at least one tick. Request 1 is unavoidably cold, leaving 8 as the test: each is either a pure hit if the region kept its entries, or cold if it did not. The two outcomes are far apart and cannot be confused: 8 x 3037 = 24,296 hits, or ~0 hits and 9 cold fills.

Counters are ehcache_*{cache="DataElement",type="L2"}, since DHIS2 backs Hibernate's L2 with Ehcache 3 via JCache. Note one Hibernate miss counts as 2 Ehcache misses + 1 put, because putFromLoad re-reads the key before installing it.

Result. Same script, same database, baseline first then this branch:

	master (core-dev@d1eb41db)	this PR
hits	2,746	24,296
misses	55,248	6,074
hit ratio	4.7%	80%
steady-state misses/min	12,354	0
evictions / removals	0 / 0	0 / 0

Both sides account for every lookup exactly. This PR hits 24,296 = 8 x 3037, the maximum possible, and its 6,074 misses are one cold-start fill. On master every request started cold.

Hits and misses per minute, master then this branch, switching over mid-run:

At the switchover misses (blue) collapse to zero while hits (green) take over, and hold there for the rest of the run, so the region is warm and serving traffic rather than idle.

A second test pins the cause to a single statement, with no idle gap or background job involved. HibernatePeriodStore.save() only runs its INSERT when the period row is missing, so on master one POST /api/dataValues with a new period empties the region, while the identical request repeated (period now exists, INSERT skipped) leaves it warm. Same endpoint, same payload shape, opposite outcome. On this branch both leave it warm.

Mechanism

The API to use is SynchronizeableQuery, which NativeQuery extends. It provides addSynchronizedEntityClass, addSynchronizedEntityName and addSynchronizedQuerySpace.

Hibernate's own documented statement of the behaviour is the @implNote on BulkOperationCleanupAction.affectedEntity:

An entity is considered to be affected if either (1) the affected table spaces are not known or (2) any of the incoming check table spaces occur in that set.

"Not known" is exactly the case here: an empty query-space set means every entity is treated as affected. The chain in 5.6.15:

1. NativeSQLQueryPlan.coordinateSharedCacheCleanup:54 builds new BulkOperationCleanupAction(session, getCustomQuery().getQuerySpaces()) before the SQL runs, and SQLCustomQuery:42 leaves that set empty unless an addSynchronized* call fills it.
2. affectedEntity then returns true for everything, so every persister with canWriteToCache() gets an EntityCleanup, whose release() calls unlockRegion -> evictAll() -> evictData(), emptying the region.

The eviction is the easy part to miss. AbstractReadWriteAccess.removeAll:209 is a no-op under read-write, so reading only that suggests nothing can be evicted. But EntityCleanup calls both removeAll and unlockRegion, and only the former is neutered.

In HibernatePeriodStore and HibernateJobConfigurationStore, which write through openStatelessSession, StatelessSessionImpl.isEventSource() returns false, so the cleanup runs immediately and synchronously instead of being deferred to the ActionQueue.

Caveat on sourcing. The 5.6 reference guide documents none of this: its native query chapter never mentions query spaces or the L2 consequences of a native bulk write, and the SynchronizeableQuery javadoc describes spaces as affecting only auto-flush and query result caching. The claim rests on the @implNote above, the source chain, and the measurements here.

Collections need the element entity, not the owner

Declaring a space is not enough on its own, it has to resolve to the regions you mean, and for a write to a join table the obvious declaration resolves to nothing.

An entity's query spaces are its own table(s) only, never the tables of the collections it owns. Category owns categoryOptions, stored in categories_categoryoptions, but Category's spaces are just {category}:

// writes categories_categoryoptions, the Category.categoryOptions collection table
.addSynchronizedEntityClass(Category.class)       // spaces = {category}      -> collection NOT evicted
.addSynchronizedEntityClass(CategoryOption.class) // spaces = {categoryoption} -> collection evicted

The second works because collection regions are not matched by table name at all. Hibernate looks them up in collectionRolesByEntityParticipant, a map keyed by each collection's element type. It holds CategoryOption -> {Category.categoryOptions, ...}; there is no Category -> {Category.categoryOptions} entry to find. Owning a collection does not get you into that map, being an element of one does.

So declare the entity on the far side of the association: categories_categoryoptions needs CategoryOption, users_catdimensionconstraints needs Category. For datasetelement, DataSetElement is both its own entity and the element of DataSet.dataSetElements, so naming it covers both regions.

Getting this wrong is easy to miss. The entity region still gets evicted, so a re-fetched Category has fresh fields and the write looks correct; only its cached option list is stale. A cold cache reloads from the database and hides it entirely, so it only shows up when the collection was cached before the write.

Why the wipe is hard to attribute

It is measurable, as above, but nothing points at it. It surfaces as a poor hit ratio, which has many plausible causes, and the obvious checks all come back clean:

• nothing is logged. DefaultHibernateCacheManager.clearCache(), which does log, is not involved, and DEBUG on org.hibernate.cache.spi.support, org.hibernate.cache.jcache, org.hibernate.action.internal and org.ehcache stays silent through a confirmed wipe.
• ehcache_evictions_total and ehcache_removals_total both stay 0, because a bulk region clear is not counted per key. They cannot be used to rule this out.
• it is not capacity or TTL. ehcache.xml gives every region 1,000,000 entries and a 6h ttl, against a few thousand entities, and this happens within seconds.

Also ruled out by measurement: the update-timestamps region, key instability, concurrency (a single sequential client reproduces it), and clearCache().

How this compounds the Uganda tracker issue

Found while investigating stalled /api/tracker imports on a Uganda instance (82s wall for 1.3s CPU, ~89% of samples parked on the DataElement L2 region lock). That contention is #24773's subject; as described there, the lock is only expensive on misses, since hits share a read lock while each miss takes it exclusively.

These wipes are what keep supplying the misses: the region is emptied every 20s, so imports repeatedly face a cold cache and pay an exclusive acquisition per data element. The wipes do not cause the contention, they stop it ever settling. Not tracker specific either, the same cycle hits every cached entity on every instance; tracker import just reads enough of one region per request to make it visible.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.9% Duplication on New Code

See analysis details on SonarQube Cloud

[jason-p-pickering]

Great find!