Revert "perf(cache): migrate H22 cache async commits to virtual threads" (#35992)

[wezell]

Reverts #35992.

Original issue: #35991 (Java 25 Performance Improvements)
Startup failure this fixes: #36892

The problem

A customer instance could not complete startup. main parked indefinitely in MVStore.commit under H22Cache.put → doUpsert, driven from InitServlet.init() → populateAllVanityURLsCache → ESContentFactoryImpl.findContentlets:

"main" ... waiting on condition
   - parking to wait for <0x00000007fe4a3718> (a java.util.concurrent.locks.ReentrantLock$FairSync)
	at org.h2.mvstore.MVStore.commit(MVStore.java:775)
	at org.h2.mvstore.MVStore.beforeWrite(MVStore.java:1273)
	...
	at com.dotmarketing.business.cache.provider.h22.H22Cache.doUpsert(H22Cache.java:650)
	at com.dotmarketing.business.cache.provider.h22.H22Cache.put(H22Cache.java:176)

H22Cache.java:176 is the synchronous branch of put() — the caller was doing the H2 write itself.

Why the migration caused it

#35992 rested on this premise:

JEP 491 means synchronized blocks in Hikari/H2 no longer pin the carrier thread, so the classic "virtual threads + JDBC pinning" concern does not apply.

That is true for synchronized, but it is not the pinning that matters here. Virtual threads do not unmount on file I/O, and embedded H2 is file I/O, not socket I/O. Every commit holding a dbWorkPermits permit pins its carrier for the duration of the MVStore write, so the 5 concurrent writers contend for a carrier pool sized to availableProcessors(). The old model used 5 dedicated platform threads, which are unaffected by carrier availability.

The submission path also changed cost profile: newThreadPerTaskExecutor starts a virtual thread immediately on submit, so a backlogged put became a live thread parked on a semaphore rather than a LinkedBlockingQueue node.

Measurements

3000 upserts of a 4KB payload through 5 concurrent writers, H2 2.2.224, using the same statement shape H22 uses (MERGE INTO … key(cache_id)):

carriers	platform pool (old)	virtual+semaphore (new)	ratio
2 (small container)	7042 puts/sec	20 puts/sec	350x
4	6977 puts/sec	889 puts/sec	7.8x
8	6000 puts/sec	1304 puts/sec	4.6x
24 (default)	7059 puts/sec	1293 puts/sec	5.5x

Platform throughput is flat regardless of carrier count. The virtual-thread path is 5x slower even with 24 carriers, and collapses on a 2-CPU container.

Why that throughput loss becomes a hang

The caller-runs fallback in isAllocationWithinTolerance() is not a regression from #35992 — it dates to e3b58b3694 (2020-09-16) with the same 10000 / 0.98 defaults. It was simply unreachable at ~7000 puts/sec, because the backlog never approached 9800.

At 20–1300 puts/sec it is reached routinely during startup. Once crossed, every caller writes inline, and hundreds of threads queue on H2's single-writer fair ReentrantLock. Throughput drops further, the backlog never drains, and the threshold never clears — it latches rather than sheds.

Scope

• Pure revert. H22Cache.java is byte-identical to 98f8d66fae^, verified by diff.
• Nothing landed in the h22 package since the migration, so there was nothing to reconcile.
• No test changes — h22 unit tests unmodified and passing.

Follow-up (deliberately not in this PR)

Tracked in #36892, kept out to keep the revert clean and reviewable:

• overflow should shed puts rather than run them on the caller — already safe, since put marks the key in DONT_CACHE_ME before writing and doSelect honors exclude(), so a dropped put reads as a miss
• deletes must never be shed — a dropped delete resurfaces as stale content once the 20s DONT_CACHE_ME entry expires
• setQueryTimeout on the fail-safe statements
• retire cache_h22_async_task_queue / cache_h22_async_tolerance, which no longer describe a real queue

Note for #35991

Any future virtual-thread migration in this codebase should distinguish socket I/O from file I/O. JEP 491 removed synchronized pinning; it did not make file I/O unmount. Workloads that block on the filesystem — embedded H2, local disk, FUSE mounts — still hold their carrier and should stay on platform threads.

Verification

• ./mvnw test-compile -pl :dotcms-core — clean
• H22CacheTest — OK (4 tests)
• Benchmark reproducible standalone against H2 2.2.224

🤖 Generated with Claude Code

This PR fixes: #36892

[claude]

Claude finished @wezell's task in 1m 46s —— View job

---

Code Review

Reviewed the pure revert of #35992 plus the new docs. I verified the reverted H22Cache.java against the current tree rather than the pre-migration blob (the migration commit 98f8d66fae isn't present in this shallow CI checkout, so I couldn't independently confirm the byte-identity claim — but the code is internally coherent).

No issues found. This is a clean, well-scoped revert.

What I checked

• Import hygiene — the migration's Semaphore field (dbWorkPermits), AtomicInteger, RejectedExecutionException, and Executors usages are gone; Semaphore is still imported but legitimately used by building (H22Cache.java:501). The re-added LinkedBlockingQueue, ThreadPoolExecutor, and ThreadFactoryBuilder are all referenced. No dangling or unused imports.
• Wiring consistency — asyncTaskQueue (H22Cache.java:57) is used by both spawnNewThreadPool() (:117-128) and isAllocationWithinTolerance() (:294), which now reads asyncTaskQueue.size() instead of inFlightTasks.get(). Coherent.
• Submission path — putAsync/removeAsync (:185, :311) submit directly to the ThreadPoolExecutor again, dropping the submitAsync permit/counter wrapper. Consistent with the pre-migration model.
• Docs — docs/backend/VIRTUAL_THREADS.md is accurate: socket-vs-file I/O unmounting, carrier pool defaulting to availableProcessors(), and the note that ReentrantLock/Semaphore parks are VT-aware (so the problem is the file syscall, not the lock) are all correct for JEP 491 / Java 25. CLAUDE.md nav link is correctly placed.

One observation (not blocking, and not introduced here)

The restored pool uses an unbounded LinkedBlockingQueue with CallerRunsPolicy (H22Cache.java:119-127). Because the queue is unbounded, the executor never rejects, so CallerRunsPolicy is effectively dead code — the real caller-runs behavior comes from shouldAsync() → isAllocationWithinTolerance() running the write inline on the caller (put() sync branch at :175). That inline path on H2's fair single-writer lock is exactly the startup-hang mechanism described in the PR body. This is pre-existing behavior being restored, and the PR explicitly tracks hardening it (shed vs. caller-run, setQueryTimeout, retiring the stale cache_h22_async_* props) in #36892 — correctly kept out to keep the revert reviewable.

The revert restores the known-good behavior; the follow-up scope is the right place for the shedding fix.

· issue-36892-revert-h22-virtual-threads

[wezell]

@erickgonzalez added this:

https://github.com/dotCMS/core/pull/36900/changes#diff-5c0fdd8add40b0f3b720de2f1f2c9411c5dba47c23bff370fda1be5fdfe2f566

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request