-
Notifications
You must be signed in to change notification settings - Fork 253
EVCacheValue: opt-in compact binary serialization with backwards-compatible reads #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
joegoogle123
wants to merge
1
commit into
sync-getbulk-mixed-keys
Choose a base branch
from
evcache-value-binary-serde
base: sync-getbulk-mixed-keys
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
196 changes: 196 additions & 0 deletions
196
evcache-core/src/main/java/com/netflix/evcache/pool/EVCacheValueSerde.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,196 @@ | ||
| package com.netflix.evcache.pool; | ||
|
|
||
| import java.nio.BufferUnderflowException; | ||
| import java.nio.ByteBuffer; | ||
| import java.nio.ByteOrder; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.Arrays; | ||
|
|
||
| import org.apache.commons.codec.binary.Hex; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * Length-prefixed binary wire format for the {@link EVCacheValue} envelope. EVCache wraps a | ||
| * value in an {@code EVCacheValue} when the canonical key has to be hashed (see | ||
| * {@code EVCacheImpl.getEVCacheKey}) so the pre-hash key is preserved for collision detection. | ||
| * | ||
| * <pre> | ||
| * [byte 0: magic 0x0C][byte 1: reserved/version 0x00] | ||
| * [int keyLen][key UTF-8 bytes] | ||
| * [int valLen][value bytes] | ||
| * [int flags][long ttl][long createTime] | ||
| * [... optional extension fields appended by newer writers ...] | ||
| * </pre> | ||
| * | ||
| * <ul> | ||
| * <li><b>Magic {@code 0x0C}</b> disambiguates from Java {@code ObjectOutputStream} (starts | ||
| * {@code 0xAC 0xED}); callers route via {@link #isBinaryFormat(byte[])}.</li> | ||
| * <li><b>Reserved/version byte</b> is currently {@code 0x00}, read-and-ignored. Bump only | ||
| * for breaking changes (see Upgrades).</li> | ||
| * <li><b>End of envelope</b> is implicit at {@code bytes.length}. There is no declared body | ||
| * length on the wire; bytes past the last known field are treated as extension data for | ||
| * additive forward-compat (see Upgrades).</li> | ||
| * <li><b>Byte order:</b> big-endian / network, set explicitly on both sides.</li> | ||
| * <li><b>Error contract:</b> any corrupt/truncated input returns {@code null} after a WARN | ||
| * log identifying the failing field and a (truncated) hex dump of the bytes. Matches | ||
| * {@code BaseSerializingTranscoder}'s resilience contract — caller sees a cache miss.</li> | ||
| * </ul> | ||
| * | ||
| * <h2>Upgrades</h2> | ||
| * | ||
| * <p><b>Additive optional (non-breaking).</b> Append a new field at the end of the envelope, | ||
| * after {@code createTime}. Older readers stop after the known fields and never look at the | ||
| * extension bytes. Newer readers MUST gate each added field with {@code buffer.hasRemaining()} | ||
| * and supply a default when absent — they will encounter items written by old writers | ||
| * (in cache until TTL expires) that don't contain the field. Only works when a graceful | ||
| * default exists. A new <i>required</i> field has no acceptable default and is therefore | ||
| * Breaking, not additive. | ||
| * | ||
| * <p><b>Breaking</b> (field reorder, type widen, semantic change, new required field): | ||
| * rollout MUST be <i>reader-before-writer</i> — items written by an early writer would be | ||
| * silently misparsed by lagging readers and survive until TTL. | ||
| * <ol> | ||
| * <li>Ship a version-aware reader that branches on byte 1: {@code 0x00} stays on the current | ||
| * decoder, the new value routes to the new decoder, unknown values | ||
| * {@link #logCorruption(byte[], String)} and return {@code null}. Deploy to 100% of every | ||
| * consumer that calls {@link #deserialize} (clients, admin tools, cache warmers, | ||
| * replicators).</li> | ||
| * <li>Wait for the longer of (full reader rollout) and (max item TTL).</li> | ||
| * <li>Then ship the new writer gated by a per-app FastProperty so canary is possible.</li> | ||
| * <li>Never reuse a version byte value for a different layout.</li> | ||
| * <li>Keep the old decoder path indefinitely — items live until their TTL expires.</li> | ||
| * </ol> | ||
| */ | ||
| public final class EVCacheValueSerde { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(EVCacheValueSerde.class); | ||
|
|
||
| static final byte BINARY_SERDE_MAGIC_CONSTANT_BYTE = 0x0C; // 12 | ||
| private static final byte RESERVED_VERSION_BYTE = 0x00; | ||
|
|
||
| private static final int CORRUPT_PAYLOAD_LOG_LIMIT = 1024; | ||
|
|
||
| private EVCacheValueSerde() { | ||
| // Utility class; not instantiable. | ||
| } | ||
|
|
||
| /** True iff {@code bytes} starts with the binary envelope magic byte. */ | ||
| public static boolean isBinaryFormat(byte[] bytes) { | ||
| return bytes != null && bytes.length > 0 && bytes[0] == BINARY_SERDE_MAGIC_CONSTANT_BYTE; | ||
| } | ||
|
|
||
| /** | ||
| * Encode an {@link EVCacheValue} into its compact binary envelope. Key and value must be | ||
| * non-null — the {@link com.netflix.evcache.EVCacheTranscoder} / {@code CachedData} pipeline | ||
| * above already rejects nulls. | ||
| */ | ||
| public static byte[] serialize(EVCacheValue v) { | ||
| final byte[] keyBytes = v.getKey().getBytes(StandardCharsets.UTF_8); | ||
| final byte[] valueBytes = v.getValue(); | ||
|
|
||
| final int bufferSize = | ||
| Byte.BYTES + Byte.BYTES // magic + reserved/version | ||
| + Integer.BYTES + keyBytes.length // keyLen + key | ||
| + Integer.BYTES + valueBytes.length // valLen + value | ||
| + Integer.BYTES // flags | ||
| + Long.BYTES // ttl | ||
| + Long.BYTES; // createTime | ||
| final ByteBuffer buffer = ByteBuffer.allocate(bufferSize).order(ByteOrder.BIG_ENDIAN); | ||
|
|
||
| buffer.put(BINARY_SERDE_MAGIC_CONSTANT_BYTE); | ||
| buffer.put(RESERVED_VERSION_BYTE); | ||
|
|
||
| buffer.putInt(keyBytes.length); | ||
| buffer.put(keyBytes); | ||
| buffer.putInt(valueBytes.length); | ||
| buffer.put(valueBytes); | ||
| buffer.putInt(v.getFlags()); | ||
| buffer.putLong(v.getTTL()); | ||
| buffer.putLong(v.getCreateTimeUTC()); | ||
|
|
||
| return buffer.array(); | ||
| } | ||
|
|
||
| /** | ||
| * Decode the binary envelope. Length prefixes are bounds-checked before allocation. A | ||
| * truncated or malformed payload returns {@code null} after a WARN log identifying the | ||
| * failing field. Bytes remaining past the known fields are not read — they're reserved for | ||
| * additive extension fields appended by newer writers (see Upgrades). | ||
| */ | ||
| public static EVCacheValue deserialize(byte[] bytes) { | ||
| String field = "magic"; | ||
| try { | ||
| final ByteBuffer buffer = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN); | ||
|
|
||
| final byte magic = buffer.get(); | ||
| if (BINARY_SERDE_MAGIC_CONSTANT_BYTE != magic) { | ||
| logCorruption(bytes, "Invalid magic constant: " + magic); | ||
| return null; | ||
| } | ||
| field = "reserved"; | ||
| buffer.get(); | ||
|
|
||
| field = "keyLength"; | ||
| final int keyLength = buffer.getInt(); | ||
| if (keyLength < 0 || keyLength > buffer.remaining()) { | ||
| logCorruption(bytes, "Invalid keyLength: " + keyLength + ", remaining=" + buffer.remaining()); | ||
| return null; | ||
| } | ||
| field = "key"; | ||
| final byte[] keyBytes = new byte[keyLength]; | ||
| buffer.get(keyBytes); | ||
| final String key = new String(keyBytes, StandardCharsets.UTF_8); | ||
|
|
||
| field = "valueLength"; | ||
| final int valueLength = buffer.getInt(); | ||
| if (valueLength < 0 || valueLength > buffer.remaining()) { | ||
| logCorruption(bytes, "Invalid valueLength: " + valueLength + ", remaining=" + buffer.remaining()); | ||
| return null; | ||
| } | ||
| field = "value"; | ||
| final byte[] valueBytes = new byte[valueLength]; | ||
| buffer.get(valueBytes); | ||
|
|
||
| field = "flags"; | ||
| final int flags = buffer.getInt(); | ||
| field = "ttl"; | ||
| final long ttl = buffer.getLong(); | ||
| field = "createTime"; | ||
| final long createTime = buffer.getLong(); | ||
|
|
||
| // Any remaining bytes are forward-compat extension fields a newer writer appended; | ||
| // an older reader (this one) leaves them unread. | ||
|
|
||
| return new EVCacheValue(key, valueBytes, flags, ttl, createTime); | ||
| } catch (BufferUnderflowException e) { | ||
| logCorruption(bytes, "BufferUnderflow at field '" + field + "'"); | ||
| return null; | ||
| } catch (Exception e) { | ||
| log.warn("Uncaught exception decoding {} bytes of EVCacheValue binary envelope at field '{}'", | ||
| bytes.length, field, e); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Warn-log a corruption event with byte length, failure reason, and a (truncated) hex dump. | ||
| * No Throwable — corruption is expected/recoverable at WARN level; a stack trace would be | ||
| * noise. Hex capped at {@value #CORRUPT_PAYLOAD_LOG_LIMIT} bytes. | ||
| */ | ||
| private static void logCorruption(byte[] bytes, String error) { | ||
| log.warn("Failed to deserialize {} bytes of EVCacheValue binary envelope, error={}, payload hex: {}", | ||
| bytes.length, error, toHex(bytes, CORRUPT_PAYLOAD_LOG_LIMIT)); | ||
| } | ||
|
|
||
| private static String toHex(byte[] bytes, int maxBytes) { | ||
| if (bytes == null) { | ||
| return "null"; | ||
| } | ||
| if (bytes.length <= maxBytes) { | ||
| return Hex.encodeHexString(bytes); | ||
| } | ||
| return Hex.encodeHexString(Arrays.copyOf(bytes, maxBytes)) | ||
| + "...(truncated, total=" + bytes.length + " bytes)"; | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we handle the case the keyLength, valueLength are corrupted less? It seems like we will finish with a incorrect data.
Should we check
if (buffer.hasRemaining()) return null;after read the buffer?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
valueLengthcorrupted smaller → corrupt data returned as a hit, notnull.The check
valueLength > buffer.remaining()still passes, so we readflags/ttl/createTimeout of value bytes and return a non-nullEVCacheValuewith no exception.keyis read beforevalueLength, so it stays intact and the collision check passes — the caller gets corrupt data as a cache hit instead of the documentednull.Example —
EVCacheValue(key="ab", value="WXYZ", flags=1, ttl=2, createTime=3):deserialize()on the corrupted bytes today returns (no exception):keyis still"ab"so the collision check passes;valueis empty, and the real value bytesWXYZ(0x57 0x58 0x59 0x5A) got reinterpreted asflags. Returned as a hit.Rejecting leftover bytes after the last field fixes it:
On the example this trips
hasRemaining()(4 leftover bytes) →null(cache miss). A largervalueLengthis already safe (it over-reads →BufferUnderflowException→ null); only the smaller case slips through.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to support adding new fields we will can't do a
buffer.hasRemaining()check. Instead I addedtotalLengthfield into wire format. Which will let us check that we read all the bytes that are expected to be read. This will be able to guard against corruption if thekeyLengthorvalueLengthis wrong.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After trying out the
bodyLengthchange I realized it can't support both adding new optional fields and guarding against bit corruption inkeyLengthorbodyLength. Given Java object serialization, what we were doing before, suffers from the same issue, I think it's ok to ship this change without addressing the concern around bit corruption.