From 45d17dbc14cd0abede54b3c32c39e59f15ffaa60 Mon Sep 17 00:00:00 2001 From: Joe Lee Date: Wed, 8 Jul 2026 16:06:50 -0400 Subject: [PATCH] Move binary serialization to EVCacheSerializingTranscoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor PR #196's binary-serde plumbing so the EVCacheValue dispatch lives on the superclass and callers get typed accessors instead of the generic getProperty escape hatch. - Move the serialize/deserialize overrides that gate on isBinarySerializationEnabled + EVCacheValueSerde.isBinaryFormat from EVCacheTranscoder up to EVCacheSerializingTranscoder. The subclass is now a thin wrapper: only encode() still overrides super to pass CachedData through unchanged. - Drop the trivial asyncDecode/decode overrides on EVCacheTranscoder that only delegated to super. - Reorder the EVCacheSerializingTranscoder constructor to (int max, EVCacheTranscoderProperties). Rename the properties field to transcoderProperties. Subclasses that need appName read it from transcoderProperties.getAppName() rather than a dedicated field. - On EVCacheTranscoderProperties, snapshot maxDataSizeBytes and compressionThresholdBytes as primitives at construction and expose them via typed getters (getMaxDataSizeBytes, getCompressionThresholdBytes, getAppName). Drop the public getProperty(Key, Class, T) escape hatch; no callers remain now that the transcoder chain uses the typed accessors. Dynamic fields will be exposed as Property accessors. - Wire the connection factories' getDefaultTranscoder() through a per-app EVCacheTranscoderProperties so the per-app FP prefix reaches the default transcoder path — PR #196 wired this for the value transcoder but missed the default transcoder built here. - Delete unused imports (ServerGroup, StringUtils, Map, ConcurrentHashMap) left behind on EVCacheSerializingTranscoder. Test coverage in EVCacheTranscoderPropertiesTest is migrated to the typed getters and extended with two cases for getAppName. Existing resolution-chain cases (per-app → global → static default) are preserved. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../evcache/EVCacheSerializingTranscoder.java | 36 +++++++++---- .../netflix/evcache/EVCacheTranscoder.java | 52 +++---------------- .../config/EVCacheTranscoderProperties.java | 37 +++++++------ .../BaseAsciiConnectionFactory.java | 4 +- .../connection/BaseConnectionFactory.java | 4 +- .../EVCacheTranscoderPropertiesTest.java | 36 ++++++++----- 6 files changed, 83 insertions(+), 86 deletions(-) diff --git a/evcache-core/src/main/java/com/netflix/evcache/EVCacheSerializingTranscoder.java b/evcache-core/src/main/java/com/netflix/evcache/EVCacheSerializingTranscoder.java index 5ab7f83f..b4183b06 100644 --- a/evcache-core/src/main/java/com/netflix/evcache/EVCacheSerializingTranscoder.java +++ b/evcache-core/src/main/java/com/netflix/evcache/EVCacheSerializingTranscoder.java @@ -24,7 +24,8 @@ import com.netflix.evcache.config.EVCacheTranscoderProperties; import com.netflix.evcache.metrics.EVCacheMetricsFactory; -import com.netflix.evcache.pool.ServerGroup; +import com.netflix.evcache.pool.EVCacheValue; +import com.netflix.evcache.pool.EVCacheValueSerde; import com.netflix.evcache.util.EVCacheConfig; import com.netflix.spectator.api.BasicTag; import com.netflix.spectator.api.Tag; @@ -33,14 +34,11 @@ import net.spy.memcached.transcoders.BaseSerializingTranscoder; import net.spy.memcached.transcoders.Transcoder; import net.spy.memcached.transcoders.TranscoderUtils; -import net.spy.memcached.util.StringUtils; import java.time.Duration; import java.util.ArrayList; import java.util.Date; import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; @@ -70,7 +68,7 @@ public class EVCacheSerializingTranscoder extends BaseSerializingTranscoder impl private final TranscoderUtils tu = new TranscoderUtils(true); private Timer timer; - protected final EVCacheTranscoderProperties properties; + protected final EVCacheTranscoderProperties transcoderProperties; /** * Get a serializing transcoder with the default max data size. @@ -83,20 +81,20 @@ public EVCacheSerializingTranscoder() { * Get a serializing transcoder that specifies the max data size. Builds a default * {@link EVCacheTranscoderProperties} bundle from * {@link EVCacheConfig#getInstance()} — subclasses/callers that want per-app - * resolution should use {@link #EVCacheSerializingTranscoder(EVCacheTranscoderProperties, int)}. + * resolution should use {@link #EVCacheSerializingTranscoder(int, EVCacheTranscoderProperties)}. */ public EVCacheSerializingTranscoder(int max) { - this(new EVCacheTranscoderProperties(null, EVCacheConfig.getInstance().getPropertyRepository()), max); + this(max, new EVCacheTranscoderProperties(null, EVCacheConfig.getInstance().getPropertyRepository())); } /** * Get a serializing transcoder with the supplied transcoder-property bundle. The bundle is - * exposed to subclasses via {@link #properties} so downstream transcoders can consult the - * same three-level (per-app → global → static default) resolution chain. + * exposed to subclasses via {@link #transcoderProperties} so downstream transcoders can consult + * the same three-level (per-app → global → static default) resolution chain. */ - public EVCacheSerializingTranscoder(EVCacheTranscoderProperties properties, int max) { + public EVCacheSerializingTranscoder(int max, EVCacheTranscoderProperties properties) { super(max); - this.properties = properties; + this.transcoderProperties = properties; } @Override @@ -213,6 +211,22 @@ public CachedData encode(Object o) { return new CachedData(flags, b, getMaxSize()); } + @Override + protected byte[] serialize(Object o) { + if (transcoderProperties.isBinarySerializationEnabled() && o instanceof EVCacheValue) { + return EVCacheValueSerde.serialize((EVCacheValue) o); + } + return super.serialize(o); + } + + @Override + protected Object deserialize(byte[] in) { + if (EVCacheValueSerde.isBinaryFormat(in)) { + return EVCacheValueSerde.deserialize(in); + } + return super.deserialize(in); + } + private void updateTimerWithCompressionRatio(long ratio_percentage) { if(timer == null) { final List tagList = new ArrayList(1); diff --git a/evcache-core/src/main/java/com/netflix/evcache/EVCacheTranscoder.java b/evcache-core/src/main/java/com/netflix/evcache/EVCacheTranscoder.java index e199a5ff..d6d61aa3 100644 --- a/evcache-core/src/main/java/com/netflix/evcache/EVCacheTranscoder.java +++ b/evcache-core/src/main/java/com/netflix/evcache/EVCacheTranscoder.java @@ -1,19 +1,15 @@ package com.netflix.evcache; import com.netflix.evcache.config.EVCacheTranscoderProperties; -import com.netflix.evcache.pool.EVCacheValue; -import com.netflix.evcache.pool.EVCacheValueSerde; import com.netflix.evcache.util.EVCacheConfig; - import net.spy.memcached.CachedData; -import static com.netflix.evcache.config.EVCacheTranscoderProperties.DEFAULT_COMPRESSION_THRESHOLD_BYTES; -import static com.netflix.evcache.config.EVCacheTranscoderProperties.DEFAULT_MAX_DATA_SIZE_BYTES; -import static com.netflix.evcache.config.EVCacheTranscoderProperties.Key.COMPRESSION_THRESHOLD_BYTES; -import static com.netflix.evcache.config.EVCacheTranscoderProperties.Key.MAX_DATA_SIZE_BYTES; - public class EVCacheTranscoder extends EVCacheSerializingTranscoder { + public EVCacheTranscoder() { + this(new EVCacheTranscoderProperties(null, EVCacheConfig.getInstance().getPropertyRepository())); + } + /** * @param properties the transcoder property bundle. * {@link EVCacheTranscoderProperties.Key#MAX_DATA_SIZE_BYTES} and @@ -24,14 +20,7 @@ public class EVCacheTranscoder extends EVCacheSerializingTranscoder { * rather than plumbed through further constructor arguments. */ public EVCacheTranscoder(EVCacheTranscoderProperties properties) { - this(properties.getProperty(MAX_DATA_SIZE_BYTES, Integer.class, DEFAULT_MAX_DATA_SIZE_BYTES).get(), - properties.getProperty(COMPRESSION_THRESHOLD_BYTES, Integer.class, DEFAULT_COMPRESSION_THRESHOLD_BYTES).get(), - properties - ); - } - - public EVCacheTranscoder() { - this(new EVCacheTranscoderProperties(null, EVCacheConfig.getInstance().getPropertyRepository())); + this(properties.getMaxDataSizeBytes(), properties.getCompressionThresholdBytes(), properties); } public EVCacheTranscoder(int max) { @@ -43,44 +32,17 @@ public EVCacheTranscoder(int max, int compressionThreshold) { } private EVCacheTranscoder(int max, EVCacheTranscoderProperties properties) { - this(max, properties.getProperty(COMPRESSION_THRESHOLD_BYTES, Integer.class, DEFAULT_COMPRESSION_THRESHOLD_BYTES).get(), properties); + this(max, properties.getCompressionThresholdBytes(), properties); } private EVCacheTranscoder(int max, int compressionThreshold, EVCacheTranscoderProperties properties) { - super(properties, max); + super(max, properties); setCompressionThreshold(compressionThreshold); } - @Override - public boolean asyncDecode(CachedData d) { - return super.asyncDecode(d); - } - - @Override - public Object decode(CachedData d) { - return super.decode(d); - } - @Override public CachedData encode(Object o) { if (o != null && o instanceof CachedData) return (CachedData) o; return super.encode(o); } - - @Override - protected byte[] serialize(Object o) { - if (this.properties.isBinarySerializationEnabled() && o instanceof EVCacheValue) { - return EVCacheValueSerde.serialize((EVCacheValue) o); - } - return super.serialize(o); - } - - @Override - protected Object deserialize(byte[] in) { - if (EVCacheValueSerde.isBinaryFormat(in)) { - return EVCacheValueSerde.deserialize(in); - } - return super.deserialize(in); - } - } diff --git a/evcache-core/src/main/java/com/netflix/evcache/config/EVCacheTranscoderProperties.java b/evcache-core/src/main/java/com/netflix/evcache/config/EVCacheTranscoderProperties.java index f520df73..b7ceb631 100644 --- a/evcache-core/src/main/java/com/netflix/evcache/config/EVCacheTranscoderProperties.java +++ b/evcache-core/src/main/java/com/netflix/evcache/config/EVCacheTranscoderProperties.java @@ -3,8 +3,6 @@ import com.netflix.archaius.api.Property; import com.netflix.archaius.api.PropertyRepository; -import static com.netflix.evcache.config.EVCacheTranscoderProperties.Key.BINARY_SERIALIZATION_ENABLED; - /** * Properties related to {@link com.netflix.evcache.EVCacheTranscoder} * behavior. @@ -18,8 +16,9 @@ * * *

- * Static properties should be cached as a field for fast access. - * Dynamic properties get be accessed {@link #getProperty(Key, Class, Object)} + * Static properties are snapshotted as primitive fields for fast access. Dynamic fields + * are exposed as {@link Property} accessors so callers see live FP updates on every + * {@code .get()}. * */ public final class EVCacheTranscoderProperties { @@ -43,9 +42,10 @@ public enum Key { } private final String appName; - private final PropertyRepository propertyRepository; private final boolean binarySerializationEnabled; + private final int maxDataSizeBytes; + private final int compressionThresholdBytes; /** * Construct the bundle and snapshot every property via the three-level resolution chain. @@ -61,24 +61,29 @@ public enum Key { */ public EVCacheTranscoderProperties(String appName, PropertyRepository propertyRepository) { this.appName = appName; - this.propertyRepository = propertyRepository; + this.binarySerializationEnabled = getProperty(appName, propertyRepository, - BINARY_SERIALIZATION_ENABLED, Boolean.class, DEFAULT_BINARY_SERIALIZATION_ENABLED).get(); + Key.BINARY_SERIALIZATION_ENABLED, Boolean.class, DEFAULT_BINARY_SERIALIZATION_ENABLED).get(); + this.maxDataSizeBytes = getProperty(appName, propertyRepository, + Key.MAX_DATA_SIZE_BYTES, Integer.class, DEFAULT_MAX_DATA_SIZE_BYTES).get(); + this.compressionThresholdBytes = getProperty(appName, propertyRepository, + Key.COMPRESSION_THRESHOLD_BYTES, Integer.class, DEFAULT_COMPRESSION_THRESHOLD_BYTES).get(); + } + + public String getAppName() { + return appName; } public boolean isBinarySerializationEnabled() { return binarySerializationEnabled; } - /** - * Resolve the Archaius {@link Property} handle for the given key. Callers should hold the - * handle (as a final field, typically) and invoke {@link Property#get()} when they need the - * current value; every {@code .get()} re-reads through the same per-app → global → static-default - * chain, so live FP updates propagate without re-resolving. Returning the handle rather than - * the resolved value makes it obvious that this is a dynamic property, not a static snapshot. - */ - public Property getProperty(Key key, Class type, T defaultValue) { - return getProperty(appName, propertyRepository, key, type, defaultValue); + public int getMaxDataSizeBytes() { + return maxDataSizeBytes; + } + + public int getCompressionThresholdBytes() { + return compressionThresholdBytes; } private static Property getProperty(String appName, PropertyRepository propertyRepository, diff --git a/evcache-core/src/main/java/com/netflix/evcache/connection/BaseAsciiConnectionFactory.java b/evcache-core/src/main/java/com/netflix/evcache/connection/BaseAsciiConnectionFactory.java index 1204156f..e6e3cfeb 100644 --- a/evcache-core/src/main/java/com/netflix/evcache/connection/BaseAsciiConnectionFactory.java +++ b/evcache-core/src/main/java/com/netflix/evcache/connection/BaseAsciiConnectionFactory.java @@ -12,6 +12,7 @@ import com.netflix.archaius.api.Property; import com.netflix.evcache.EVCacheTranscoder; +import com.netflix.evcache.config.EVCacheTranscoderProperties; import com.netflix.evcache.operation.EVCacheAsciiOperationFactory; import com.netflix.evcache.pool.EVCacheClient; import com.netflix.evcache.pool.EVCacheClientPool; @@ -115,7 +116,8 @@ public BlockingQueue createWriteOperationQueue() { } public Transcoder getDefaultTranscoder() { - return new EVCacheTranscoder(); + return new EVCacheTranscoder(new EVCacheTranscoderProperties(appName, + client.getPool().getEVCacheClientPoolManager().getEVCacheConfig().getPropertyRepository())); } public FailureMode getFailureMode() { diff --git a/evcache-core/src/main/java/com/netflix/evcache/connection/BaseConnectionFactory.java b/evcache-core/src/main/java/com/netflix/evcache/connection/BaseConnectionFactory.java index 7cd7b290..e8d60ff1 100644 --- a/evcache-core/src/main/java/com/netflix/evcache/connection/BaseConnectionFactory.java +++ b/evcache-core/src/main/java/com/netflix/evcache/connection/BaseConnectionFactory.java @@ -12,6 +12,7 @@ import com.netflix.archaius.api.Property; import com.netflix.evcache.EVCacheTranscoder; +import com.netflix.evcache.config.EVCacheTranscoderProperties; import com.netflix.evcache.pool.EVCacheClient; import com.netflix.evcache.pool.EVCacheClientPool; import com.netflix.evcache.pool.EVCacheClientPoolManager; @@ -109,7 +110,8 @@ public BlockingQueue createWriteOperationQueue() { } public Transcoder getDefaultTranscoder() { - return new EVCacheTranscoder(); + return new EVCacheTranscoder(new EVCacheTranscoderProperties(appName, + client.getPool().getEVCacheClientPoolManager().getEVCacheConfig().getPropertyRepository())); } public FailureMode getFailureMode() { diff --git a/evcache-core/src/test/java/com/netflix/evcache/config/EVCacheTranscoderPropertiesTest.java b/evcache-core/src/test/java/com/netflix/evcache/config/EVCacheTranscoderPropertiesTest.java index 1754c6c1..619e8b44 100644 --- a/evcache-core/src/test/java/com/netflix/evcache/config/EVCacheTranscoderPropertiesTest.java +++ b/evcache-core/src/test/java/com/netflix/evcache/config/EVCacheTranscoderPropertiesTest.java @@ -1,7 +1,5 @@ package com.netflix.evcache.config; -import static com.netflix.evcache.config.EVCacheTranscoderProperties.Key.COMPRESSION_THRESHOLD_BYTES; -import static com.netflix.evcache.config.EVCacheTranscoderProperties.Key.MAX_DATA_SIZE_BYTES; import static org.assertj.core.api.Assertions.assertThat; import com.netflix.archaius.DefaultPropertyFactory; @@ -83,7 +81,7 @@ public void maxDataSize_perAppOverrideWins() { cfg.setProperty(MAX_DATA_SIZE_PER_APP_KEY, "12345"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(cfg)); - assertThat(props.getProperty(MAX_DATA_SIZE_BYTES, Integer.class, 999).get()).isEqualTo(12345); + assertThat(props.getMaxDataSizeBytes()).isEqualTo(12345); } @Test @@ -92,13 +90,13 @@ public void maxDataSize_globalFallbackWhenPerAppUnset() { cfg.setProperty(MAX_DATA_SIZE_GLOBAL_KEY, "12345"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(cfg)); - assertThat(props.getProperty(MAX_DATA_SIZE_BYTES, Integer.class, 999).get()).isEqualTo(12345); + assertThat(props.getMaxDataSizeBytes()).isEqualTo(12345); } @Test public void maxDataSize_staticDefaultWhenBothUnset() { EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(new DefaultSettableConfig())); - assertThat(props.getProperty(MAX_DATA_SIZE_BYTES, Integer.class, 999).get()).isEqualTo(999); + assertThat(props.getMaxDataSizeBytes()).isEqualTo(EVCacheTranscoderProperties.DEFAULT_MAX_DATA_SIZE_BYTES); } @Test @@ -108,7 +106,7 @@ public void maxDataSize_perAppBeatsGlobal() { cfg.setProperty(MAX_DATA_SIZE_GLOBAL_KEY, "222"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(cfg)); - assertThat(props.getProperty(MAX_DATA_SIZE_BYTES, Integer.class, 999).get()).isEqualTo(111); + assertThat(props.getMaxDataSizeBytes()).isEqualTo(111); } @Test @@ -117,7 +115,7 @@ public void maxDataSize_nullAppNameUsesGlobalKey() { cfg.setProperty(MAX_DATA_SIZE_GLOBAL_KEY, "12345"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(null, repo(cfg)); - assertThat(props.getProperty(MAX_DATA_SIZE_BYTES, Integer.class, 999).get()).isEqualTo(12345); + assertThat(props.getMaxDataSizeBytes()).isEqualTo(12345); } // ---- COMPRESSION_THRESHOLD ---- @@ -128,7 +126,7 @@ public void compressionThreshold_perAppOverrideWins() { cfg.setProperty(COMPRESSION_PER_APP_KEY, "512"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(cfg)); - assertThat(props.getProperty(COMPRESSION_THRESHOLD_BYTES, Integer.class, 999).get()).isEqualTo(512); + assertThat(props.getCompressionThresholdBytes()).isEqualTo(512); } @Test @@ -137,13 +135,13 @@ public void compressionThreshold_globalFallbackWhenPerAppUnset() { cfg.setProperty(COMPRESSION_GLOBAL_KEY, "512"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(cfg)); - assertThat(props.getProperty(COMPRESSION_THRESHOLD_BYTES, Integer.class, 999).get()).isEqualTo(512); + assertThat(props.getCompressionThresholdBytes()).isEqualTo(512); } @Test public void compressionThreshold_staticDefaultWhenBothUnset() { EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(new DefaultSettableConfig())); - assertThat(props.getProperty(COMPRESSION_THRESHOLD_BYTES, Integer.class, 999).get()).isEqualTo(999); + assertThat(props.getCompressionThresholdBytes()).isEqualTo(EVCacheTranscoderProperties.DEFAULT_COMPRESSION_THRESHOLD_BYTES); } @Test @@ -153,7 +151,7 @@ public void compressionThreshold_perAppBeatsGlobal() { cfg.setProperty(COMPRESSION_GLOBAL_KEY, "222"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(cfg)); - assertThat(props.getProperty(COMPRESSION_THRESHOLD_BYTES, Integer.class, 999).get()).isEqualTo(111); + assertThat(props.getCompressionThresholdBytes()).isEqualTo(111); } @Test @@ -162,6 +160,20 @@ public void compressionThreshold_nullAppNameUsesGlobalKey() { cfg.setProperty(COMPRESSION_GLOBAL_KEY, "512"); EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(null, repo(cfg)); - assertThat(props.getProperty(COMPRESSION_THRESHOLD_BYTES, Integer.class, 999).get()).isEqualTo(512); + assertThat(props.getCompressionThresholdBytes()).isEqualTo(512); + } + + // ---- appName ---- + + @Test + public void appName_isExposedForSubclassLookups() { + EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(APP, repo(new DefaultSettableConfig())); + assertThat(props.getAppName()).isEqualTo(APP); + } + + @Test + public void appName_nullPropagates() { + EVCacheTranscoderProperties props = new EVCacheTranscoderProperties(null, repo(new DefaultSettableConfig())); + assertThat(props.getAppName()).isNull(); } }