From f9a3d3700769c31437a88f7c43a2f231d33492e1 Mon Sep 17 00:00:00 2001 From: Fred Wulff Date: Wed, 9 Sep 2026 15:17:02 -0500 Subject: [PATCH 1/2] ValkeyCacheService: batch getIdentifiables instead of one round trip per key getIdentifiables looped over the keys awaiting each GET before issuing the next, so an N-key batch cost N serial round trips (up to 3N when the keys were cold, since each miss also awaited its SET NX bootstrap and re-read). This is Objectify's entity-load read path, so every batch load serialized against Valkey while holding the calling thread. Fire each phase concurrently instead, the way getAll/putAll/putIfUntouched already do: all GETs, then the SET NX bootstraps for whatever missed, then the re-reads. A batch now costs at most three round trips regardless of size. Keys whose value is still absent after the bootstrap are left out of the returned map rather than mapped to null. EntityMemcache reads the result with get(key) and cannot tell the two apart, but callers sizing the map can: the old code returned an entry for every requested key, which made a returned-size hit/miss counter read 100% hits. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01M52d6uXfFJ3MGVMDz5fcKG --- .../cache/valkey/ValkeyCacheService.java | 62 ++++++++++++++----- .../test/valkey/ValkeyCacheServiceTests.java | 29 +++++++++ 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/googlecode/objectify/cache/valkey/ValkeyCacheService.java b/src/main/java/com/googlecode/objectify/cache/valkey/ValkeyCacheService.java index 112089c9..aac7bb2d 100644 --- a/src/main/java/com/googlecode/objectify/cache/valkey/ValkeyCacheService.java +++ b/src/main/java/com/googlecode/objectify/cache/valkey/ValkeyCacheService.java @@ -292,26 +292,58 @@ public Object get(final String key) { @Override public Map getIdentifiables(final Collection keys) { final Map result = new LinkedHashMap<>(); - for (final String key : keys) { - result.put(key, getIdentifiable(key)); + if (keys.isEmpty()) { + return result; } - return result; - } - private IdentifiableValue getIdentifiable(final String key) { - final byte[] bytes = rawGet(key); - if (bytes != null) { - return new ValkeyIdentifiableValue(fromCacheBytes(bytes), bytes); + // Per-key GETs fired concurrently (no MGET, which would CROSSSLOT on a cluster). Awaiting + // each GET before issuing the next would make a batch cost one serial round trip per key, + // which is what Objectify's entity-load path does on every batch read. + final Map> gets = new LinkedHashMap<>(); + for (final String key : keys) { + gets.put(key, client.get(gskey(key))); } - // Cold cache: bootstrap a sentinel under NX so we can later CAS against it. NX prevents - // us from clobbering a value another caller has just set in between our GET and our SET. - // TTL-bounded like every other write (defaultNxSetOptions): a read-heavy workload bootstraps - // a sentinel per cold key, and without expiry those persist forever on a noeviction cluster. - await(client.set(gskey(key), gs(NULL_VALUE), defaultNxSetOptions)); + final Map raw = new LinkedHashMap<>(); + gets.forEach((key, future) -> { + final GlideString value = await(future); + raw.put(key, value == null ? null : value.getBytes()); + }); - final byte[] bootstrapped = rawGet(key); - return bootstrapped == null ? null : new ValkeyIdentifiableValue(fromCacheBytes(bootstrapped), bootstrapped); + final List cold = new ArrayList<>(); + raw.forEach((key, bytes) -> { + if (bytes == null) { + cold.add(key); + } + }); + + if (!cold.isEmpty()) { + // Cold cache: bootstrap a sentinel under NX so we can later CAS against it. NX prevents + // us from clobbering a value another caller has just set in between our GET and our SET. + // TTL-bounded like every other write (defaultNxSetOptions): a read-heavy workload bootstraps + // a sentinel per cold key, and without expiry those persist forever on a noeviction cluster. + final List> bootstraps = new ArrayList<>(); + for (final String key : cold) { + bootstraps.add(client.set(gskey(key), gs(NULL_VALUE), defaultNxSetOptions)); + } + bootstraps.forEach(ValkeyCacheService::await); + + final Map> rereads = new LinkedHashMap<>(); + for (final String key : cold) { + rereads.put(key, client.get(gskey(key))); + } + rereads.forEach((key, future) -> { + final GlideString value = await(future); + raw.put(key, value == null ? null : value.getBytes()); + }); + } + + raw.forEach((key, bytes) -> { + if (bytes != null) { + result.put(key, new ValkeyIdentifiableValue(fromCacheBytes(bytes), bytes)); + } + }); + return result; } @Override diff --git a/src/test/java/com/googlecode/objectify/test/valkey/ValkeyCacheServiceTests.java b/src/test/java/com/googlecode/objectify/test/valkey/ValkeyCacheServiceTests.java index aba51a59..ac0b3b2a 100644 --- a/src/test/java/com/googlecode/objectify/test/valkey/ValkeyCacheServiceTests.java +++ b/src/test/java/com/googlecode/objectify/test/valkey/ValkeyCacheServiceTests.java @@ -131,6 +131,35 @@ void getIdentifiablesBootstrapsColdCacheWithNullSentinel() { assertThat(iv.getValue()).isNull(); // sentinel decodes back to null } + @Test + void getIdentifiablesMixesWarmAndColdKeysInOneBatch() { + cache.put("warm", "alpha"); + + final Map ivs = cache.getIdentifiables(Arrays.asList("warm", "cold", "warm2")); + cache.put("warm2", "gamma"); // written after the batch read; must not affect the snapshot + + assertThat(ivs.keySet()).containsExactly("warm", "cold", "warm2").inOrder(); + assertThat(ivs.get("warm").getValue()).isEqualTo("alpha"); + assertThat(ivs.get("cold").getValue()).isNull(); // bootstrapped sentinel + assertThat(ivs.get("warm2").getValue()).isNull(); // bootstrapped sentinel + + // Every snapshot in the batch is still a usable CAS basis; only "warm2" was stomped. + final Map proposed = new LinkedHashMap<>(); + proposed.put("warm", new CasPut(ivs.get("warm"), "fresh-warm", 0)); + proposed.put("cold", new CasPut(ivs.get("cold"), "fresh-cold", 0)); + proposed.put("warm2", new CasPut(ivs.get("warm2"), "fresh-warm2", 0)); + + assertThat(cache.putIfUntouched(proposed)).containsExactly("warm", "cold"); + assertThat(cache.get("warm")).isEqualTo("fresh-warm"); + assertThat(cache.get("cold")).isEqualTo("fresh-cold"); + assertThat(cache.get("warm2")).isEqualTo("gamma"); + } + + @Test + void getIdentifiablesOnEmptyBatchReturnsEmpty() { + assertThat(cache.getIdentifiables(Arrays.asList())).isEmpty(); + } + @Test void casSucceedsOnUntouchedSentinel() { final IdentifiableValue iv = cache.getIdentifiables(Arrays.asList("k")).get("k"); From f2299c4c803d2f96844010dd20e19882b7309f2d Mon Sep 17 00:00:00 2001 From: Fred Wulff Date: Wed, 9 Sep 2026 15:17:02 -0500 Subject: [PATCH 2/2] Publish 6.1.4-streak-valkey-9 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01M52d6uXfFJ3MGVMDz5fcKG --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 866cd1b3..ac3f5062 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ objectify - 6.1.4-streak-valkey-8 + 6.1.4-streak-valkey-9 Objectify App Engine The simplest convenient interface to the Google App Engine datastore