Cache Stalls, a Recursive Update, and One Shared Formatter
A pricing service caches 5,000 SKUs in a ConcurrentHashMap<String, Price> shared by a 32-thread request pool. Load tests show a p99 of 3.8 s even for keys that were cached minutes earlier, while CPU stays under 15 percent and thread dumps show most workers parked in WAITING. The cache is populated with cache.computeIfAbsent(sku, s -> remotePricingClient.fetch(s)), and fetch performs a blocking HTTPS call that can take 2 s. Separately, a nightly job runs cache.compute(sku, (k, v) -> v == null ? load(k) : v), and load(k) calls cache.computeIfAbsent for a related SKU; the job intermittently fails with IllegalStateException: Recursive update. A third symptom: a parallelStream() over 400,000 orders formats dates with one static final SimpleDateFormat field, and operators report occasional garbled dates and ArrayIndexOutOfBoundsException thrown from inside the formatter.
1. Explain the 3.8 s p99 for keys that are already cached, and give a fix that keeps `ConcurrentHashMap`.
Read the worked answer
computeIfAbsent is atomic: it locks the bin (the first node of the hash bucket) for the key and runs the mapping function while holding that lock. fetch can take 2 s, so every thread that hashes to that bin and wants to update the map blocks, even if it is asking about a completely different, already-cached SKU. That is why the slow keys look slow: their own lookup is instant, but the caller is queued behind someone else's network call. It also explains the profile - low CPU, threads parked in WAITING - and it can cascade when the table resizes.
Two additional traps: if fetch returns null, no mapping is recorded and every subsequent call re-fetches; if it throws, nothing is cached either, so a flapping dependency causes repeated 2 s stalls for the same key.
Fix: never do I/O inside a remapping function. Publish the in-flight work instead:
ConcurrentHashMap<String, CompletableFuture<Price>> cache = new ConcurrentHashMap<>();
CompletableFuture<Price> f = cache.computeIfAbsent(sku, k -> new CompletableFuture<>());
if (!f.isDone()) {
try { f.complete(remote.fetch(sku)); }
catch (Throwable t) { f.completeExceptionally(t); cache.remove(sku, f); }
}
return f.join();
The map operation is now nanoseconds, the bin lock is released immediately, and concurrent callers wait on the future rather than on the bin. A get / fetch / putIfAbsent sequence is also acceptable - it may duplicate a fetch but never holds a lock across I/O. For production, a cache library with per-key loading and expiry (e.g. Caffeine) removes the hand-rolled subtleties.
2. Why does the nightly job throw `IllegalStateException: Recursive update`, and how should it be restructured?
Read the worked answer
compute holds the bin lock while the remapping function runs, and load(k) calls back into the same map (computeIfAbsent on a related SKU). If the related key lands in the same bin, the thread tries to reacquire a lock it already holds. Since Java 9 the implementation detects this and throws IllegalStateException: Recursive update rather than hanging; on earlier versions it could deadlock. Even when the two keys land in different bins it is unsafe: two threads doing the reverse order (A-then-B vs B-then-A) can deadlock on lock ordering, and the map's structural invariants are not designed for reentrant modification from a remapping function.
The same rule covers compute, computeIfPresent, merge and replaceAll: the function must be a pure calculation over the given key and value. It must not modify the map - including removing the entry being computed, which is done by returning null.
Restructure so the map is touched once per key:
Price related = cache.get(relatedSku);
if (related == null) { related = load(relatedSku); cache.putIfAbsent(relatedSku, related); }
Price self = load(k); // may use related, but does not touch cache
cache.put(k, self);
Where the two values must be consistent, compute both into locals first and publish them in a short, non-blocking second phase, or serialize the refresh with an explicit lock or an actor-style single-writer thread rather than relying on map locks.
3. Explain the formatter failures and the correct fix, including anything else about the pipeline you would change.
Read the worked answer
SimpleDateFormat is mutable and not thread-safe: format writes into internal Calendar state and a shared StringBuffer. Two threads formatting simultaneously interleave those writes, producing garbled output, wrong dates, NumberFormatException, or ArrayIndexOutOfBoundsException from inside the pattern machinery. A parallelStream() guarantees this will happen under load.
The best fix is to delete it: java.time.format.DateTimeFormatter is immutable and thread-safe, and it also forces an explicit Locale, ZoneId and pattern, which removes a second source of bugs - output that silently depends on the server's default locale and time zone (a classic localization defect when the same service runs in two regions).
If the legacy API forces SimpleDateFormat, wrap it in ThreadLocal.withInitial(() -> new SimpleDateFormat(pattern, locale)) with the zone set at creation, or synchronize on a single instance and accept the contention.
Other changes for this pipeline: a parallelStream() uses the common ForkJoinPool, whose parallelism is cores - 1; any blocking call inside it starves every other common-pool user in the JVM, so blocking work belongs in a dedicated executor sized from measured latency and target throughput. Date formatting is CPU-bound and acceptable in the common pool, but confirm with measurement before parallelizing at all - 400,000 records may be faster sequentially than paying split/merge overhead.