Support caching for map primitive #10
Merged
Conversation
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
kuujo
commented
Feb 11, 2020
| @@ -0,0 +1,136 @@ | |||
| // Copyright 2019-present Open Networking Foundation. | |||
Member
Author
There was a problem hiding this comment.
Need to update the copyright header in my IDE 😄
pkg/client/map/cache.go
Outdated
| for event := range ch { | ||
| switch event.Type { | ||
| case EventNone: | ||
| m.cache.Add(event.Entry.Key, event.Entry) |
Member
Author
There was a problem hiding this comment.
These cache writes need to be locked
pkg/client/map/cache.go
Outdated
| // This check is performed because a concurrent event could update the cached entry | ||
| m.mu.Lock() | ||
| prevEntry, ok := m.cache.Get(key) | ||
| if !ok || prevEntry.(*Entry).Version < entry.Version { |
Member
Author
There was a problem hiding this comment.
This condition is unsafe for the same reason it’s unsafe on Get: a later Remove would make this version check impossible without maintaining tombstones. The entries just have to be removed from the cache.
Member
Author
|
I think this doesn’t actually provide RYW consistency. Cache hits read from an event stream, and cache misses from the database. But in the Go client, events are not serialized with reads, so it’s possible a cache miss can occur after a later cache hit in logical time. |
adibrastegarnia
approved these changes
Feb 18, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds support for wrapping a
Mapprimitive in an LRU cache.To cache a map, just use the
WithCacheoption when constructing the map:The cache is implemented using Hashicorp's LRU cache. When a client writes to the cache, any existing cached entries are deleted. A background goroutine listens for update events from the cluster to populate the cache. Because of the consistency guarantees of Atomix sessions, this guarantees a client will receive the same consistency guarantees as an uncached client.