ValkeyCacheService: batch getIdentifiables instead of one round trip per key (streak-valkey-9)

[frew]

Why

Sentry STREAK-31N3 — 528k events / 34k users since 2026-07-07, still firing. Every event is an Objectify entity load failing against Valkey and falling back to Datastore.

The Valkey cluster is not the bottleneck. Memorystore streak-valkey serves ~175k ops/s at 2.5 µs mean command time (total_usec_count / total_calls_count) with CPU at 20%. The timeouts are client-side, and valkey-glide says so directly:

timeout_watchdog - Timeout: cmd=GET node=unknown
  cause=ClientBackpressure { queue_depth: 61, scheduling_delay: 4.000154482s }
  phase=Queued elapsed=4.000154482s configured=4s pending=61 inflight=9→27

phase=Queued — the GET never left the process. It spent its entire 4s budget in the client's local command queue, with only 9–27 commands actually in flight behind 61 queued.

getIdentifiables is a plausible source of that queue pressure, and it's the one thing here we control:

for (final String key : keys) {
    result.put(key, getIdentifiable(key));   // rawGet -> await(future.get())
}

Each key is a blocking round trip taken one at a time. A cold key costs three (GET, SET NX, GET). This is the read path EntityMemcache.getAll calls for every entity load, so an N-key batch serialized N (up to 3N) round trips while holding the calling thread — a Jetty request thread, in the web tier.

getAll, putAll, and putIfUntouched right below it already fan out. The class javadoc even claims this method does too:

Multi-key reads/writes/deletes are issued as independent per-key commands fired concurrently rather than MGET/MSET/DEL key..., which would fail with CROSSSLOT on a cluster.

getIdentifiables just never got that treatment.

What

Three concurrent phases instead of a serial loop:

1. Fire every GET, then await them all.
2. Fire a SET NX sentinel bootstrap for whatever missed, await those.
3. Fire the re-reads for those same keys, await those.

A batch now costs at most three round trips regardless of size, down from N–3N. No change to the wire format, the sentinel/CAS contract, or CROSSSLOT safety — every command is still single-key.

Insertion order of the returned map is preserved (raw is a LinkedHashMap seeded in keys order and overwritten in place), so a mixed warm/cold batch comes back in the order asked for.

Behavior change worth flagging

Keys still absent after the bootstrap are now omitted from the returned map rather than mapped to null.

EntityMemcache reads the result with casValues.get(key) and cannot distinguish the two, so its behavior is identical. But a caller sizing the map can. The old code did result.put(key, getIdentifiable(key)) for every key, so result.size() == keys.size() unconditionally — which makes MailFoo's ValkeyObjectifyCache hit/miss counters:

metrics.markIdentifiablesHit(result.size.toLong())
metrics.markIdentifiablesMiss((keys.size - result.size).toLong())

report 100% hits and 0 misses, always. After this change those counters mean what their KDoc says ("a key is a hit if the backend returned an entry for it"). Expect the identifiables hit-rate metric to drop off 100% when this deploys — that's the metric starting to work, not a regression.

Testing

• getIdentifiablesMixesWarmAndColdKeysInOneBatch — a batch spanning a warm key, a cold key, and a key stomped after the snapshot. Asserts returned order, that each key's value decodes correctly, and that all three snapshots remain valid CAS bases with only the stomped one losing. This is the case the serial version handled trivially and the phased version actually has to get right.
• getIdentifiablesOnEmptyBatchReturnsEmpty — the new keys.isEmpty() short circuit.
• Existing ValkeyCacheServiceTests cover the sentinel, CAS winners/losers, TTL, and the multi-threaded CAS race.

All 35 Valkey tests pass locally against valkey/valkey:9.0 (Docker Desktop, server API 1.53):

Tests run: 1,  ... ValkeyBasicCacheTests
Tests run: 1,  ... ValkeyCachingUncacheableTests
Tests run: 28, ... ValkeyCacheServiceTests
Tests run: 1,  ... ValkeyEvilCacheBugTests
Tests run: 2,  ... ValkeyCachingTests
Tests run: 2,  ... ValkeyCachingDatastoreTests
Tests run: 35, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Note for anyone else running these on a recent Docker Desktop: the daemon rejects any API version below 1.44 with an HTTP 400 on /info, and testcontainers' bundled docker-java negotiates an older one, so every Valkey test dies at startValkey with Could not find a valid Docker environment. DOCKER_API_VERSION does not help (that's the CLI's variable) — pass the docker-java system property instead:

mvn -Dtest='Valkey*' -DargLine="-Dapi.version=1.44" test

A testcontainers/docker-java bump would fix this properly, but that's out of scope here.

Rollout

This publishes 6.1.4-streak-valkey-9. Merging does not deploy it — someone still needs to run ./publish-to-artifact-registry.sh and then bump the coordinate in MailFoo's settings.gradle.kts and MODULE.bazel.

Stacked on #5 (relocation-aware-object-input-stream, -valkey-8), which is still open and is what production runs today. GitHub will retarget this to streak-valkey when #5 merges.

Does not fix the ConnectionNotFoundForRoute bursts in the same Sentry issue — those follow MOVED redirects during Memorystore slot rebalances and want --replica-count=1 plus a maintenance window on the instance.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M52d6uXfFJ3MGVMDz5fcKG