Skip to content

fix: cache hashed keys per hashing algorithm, length and encoder in EVCacheKey - #206

Open
RaphaelFakhri wants to merge 1 commit into
Netflix:masterfrom
RaphaelFakhri:fix/evcache-key-hash-cache-key
Open

RaphaelFakhri wants to merge 1 commit into
Netflix:masterfrom
RaphaelFakhri:fix/evcache-key-hash-cache-key

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

Summary

EVCacheKey caches computed hashed keys in hashedKeysByAlgorithm and hashedKeysByAlgorithmForDuet. The expression that builds the cache key is missing parentheses:

hashingAlgorithm.toString()+ maxDigestBytes != null ? maxDigestBytes.toString() : "-" + ...

Java evaluates (hashingAlgorithm + maxDigestBytes) != null, which is always true, so the cache key is just maxDigestBytes.toString(). The hashing algorithm, max hash length and encoder never take part in the lookup.

EVCacheImpl uses one EVCacheKey across all clients of a call, and each client passes its own server-group hashing properties. When server groups use different hash.algo, max.hash.length or hash.encoder values, every client after the first receives the first client's hash. Writes then land under the wrong key in those server groups, and reads against them miss.

Changes

  • Build the cache key in a small getHashCacheKey helper that joins the algorithm, max digest bytes, max hash length and encoder with : separators. Both the regular and the duet paths use it.
  • Update the field comment to describe the cache key.
  • Add EVCacheKeyTest (registered in test-suite.xml) covering a change of algorithm, max hash length and encoder, for regular and duet keys.

Testing

./gradlew :evcache-core:test --tests com.netflix.evcache.EVCacheKeyTest
  • Without the fix: 3 tests, 3 failed (the second hash equals the first one).
  • With the fix: 3 tests, 0 failed.

Fixes #205

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EVCacheKey caches hashed keys under the wrong cache key, so clients with different hashing properties get each other's hash

1 participant