feat(cache)!: use cachestore to cache request in server #111
Conversation
There was a problem hiding this comment.
We should also include the Hint in the caching logic to provide clients with more control over caching specific records (or not):
DO_NOT_CACHE - during creation / update
IGNORE_CACHE - for get requests
Update: Captured in #64 (comment)
internal/pkg/cache/cache.go
Outdated
| return fmt.Sprintf("%s/%s", storeKey, recordKey) | ||
| } | ||
|
|
||
| // EncodeRecord serializes a tritonpb Record with gob/base64. |
There was a problem hiding this comment.
I think Redis is binary-safe (https://redis.io/topics/data-types) so we don't need to encode it with base64. Is there a reason why we do the text encoding/decoding?
hongalex
changed the title
|
@yuryu I removed the base64 encoding, but also made a breaking change to cache, so that While Redis supports any number of data types as keys and values, I think |
|
Re: the interface, I think it makes sense for now to just store everything as binary. We could improve memory utilization and potentially implement atomic operations (add, inc, etc) in the cache too but doing so now seems like a premature optimization to me. |
What type of PR is this?
/kind feat
What this PR does / Why we need it:
Adds caching support to the Triton server implementation, storing
Records in the cachestore assuming the size is less than 10 MiB.BREAKING: Cache implementation now uses byte arrays are values rather than strings. This should have minimal effect since the cache store wasn't being used in the server logic before.
Which issue(s) this PR fixes:
Closes #110
Special notes for your reviewer:
This doesn't close issue #64 because we need to rework the caching hint implementation.