fix LocalCache#read_multi_entries #44366
Conversation
|
The need for explicit serialization handling seems a bit mixed between these methods, which is not great for clarity while reading them, but from a quick read this does sound right. (And tbh, I don't see any simple changes that would flatten out those inconsistencies more generally.) Can you please add a test to show this does fix things / to prevent a future regression -- especially given the above? (After adding one specifically for this case, it would probably be worth going through the full set of operations and ensuring they're all tested at least once with a serialization-distinguished value... I assume this is the only issue at the moment, but it seems worth taking it as a reminder to check on the full coverage.) |
avalanche123
force-pushed
the
patch-1
branch
2 times, most recently
from
0eaecc7
to
0b18170
Compare
|
@matthewd thanks for taking a look! I've added a test for the multi read/write behavior of |
|
Can you squash your commits in one please? |
In cache stores prepending `LocalCache`, serialized `Entry` instances are stored in `LocalCache::LocalStore`. This speeds up hot key lookups without a network roundtrip in a context of a single given request. However, with these entries being stored in their serialized form, `#read_multi_entries` returns them directly to cache consumers. Instead, we will now deserialize these entries first.
|
@rafaelfranca done! |
Fix deserialization in LocalCache#read_multi_entries
In cache stores prepending
LocalCache, serializedEntryinstances are stored in
LocalCache::LocalStore. Thisspeeds up hot key lookups without a network roundtrip in a
context of a single given request.
However, with these entries being stored in their serialized
form,
#read_multi_entriesreturns them directly to cacheconsumers.
Instead, we will now deserialize these entries first.
Summary
Other Information