-
Notifications
You must be signed in to change notification settings - Fork 13
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
[Off-chain] feat: in-memory query cache(s) #994
Open
bryanchriswhite
wants to merge
15
commits into
main
Choose a base branch
from
issues/543/cache/memory
base: main
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 7 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
830a7f6
fix: sessiontree bug
bryanchriswhite b0f08c3
chore: add QueryCache interface
bryanchriswhite 8743246
chore: add HistoricalQueryCache interface
bryanchriswhite d895441
feat: add InMemoryCache implementation
bryanchriswhite 23cc94a
chore: self-review improvements
bryanchriswhite 366ab1d
chore: self-review improvements
bryanchriswhite 4632c74
fix: historical pruning
bryanchriswhite a467428
Merge remote-tracking branch 'pokt/main' into issues/543/cache/memory
bryanchriswhite e48a2f2
chore: review feedback improvements
bryanchriswhite d5ce62f
chore: review feedback improvements
bryanchriswhite 71da7f1
chore: review feedback improvements
bryanchriswhite 6774d3a
fix: linter errors
bryanchriswhite 086d219
chore: disable #Set() on historical cache
bryanchriswhite d6dbc44
Merge branch 'main' into issues/543/cache/memory
bryanchriswhite 0414e9a
Merge branch 'main' into issues/543/cache/memory
Olshansk 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 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 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 |
---|---|---|
|
@@ -4,7 +4,7 @@ import ( | |
"time" | ||
) | ||
|
||
// EvictionPolicy determines how items are removed when number of keys in the cache reaches MaxKeys. | ||
// EvictionPolicy determines which values are removed when number of keys in the cache reaches maxKeys. | ||
type EvictionPolicy int64 | ||
|
||
const ( | ||
|
@@ -13,57 +13,86 @@ const ( | |
LeastFrequentlyUsed | ||
) | ||
|
||
// queryCacheConfig is the configuration for query caches. It is intended to be | ||
// configured via QueryCacheOptionFn functions. | ||
// queryCacheConfig is the configuration for query caches. | ||
// It is intended to be configured via QueryCacheOptionFn functions. | ||
type queryCacheConfig struct { | ||
// MaxKeys is the maximum number of items (key/value pairs) the cache can | ||
// maxKeys is the maximum number of key/value pairs the cache can | ||
// hold before it starts evicting. | ||
MaxKeys int64 | ||
EvictionPolicy EvictionPolicy | ||
// TTL is how long items should remain in the cache. Items older than the TTL | ||
// MAY not be evicted but SHOULD not be considered as cache hits. | ||
TTL time.Duration | ||
maxKeys int64 | ||
|
||
// historical determines whether the cache will cache a single value for each | ||
// key (false), or whether it will cache a history of values for each key (true). | ||
// TODO_CONSIDERATION: | ||
// | ||
// maxValueSize is the maximum cumulative size of all values in the cache. | ||
// maxValueSize int64 | ||
// maxCacheSize is the maximum cumulative size of all keys AND values in the cache. | ||
// maxCacheSize int64 | ||
|
||
// evictionPolicy determines which values are removed when number of keys in the cache reaches maxKeys. | ||
evictionPolicy EvictionPolicy | ||
// ttl is how long values should remain valid in the cache. Items older than the | ||
Olshansk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// ttl MAY NOT be evicted immediately, but are NEVER considered as cache hits. | ||
ttl time.Duration | ||
// historical determines whether each key will point to a single values (false) | ||
// or a history (i.e. reverse chronological list) of values (true). | ||
historical bool | ||
// pruneOlderThan is the number of past blocks for which to keep historical | ||
// values. If 0, no historical pruning is performed. It only applies when | ||
// historical is true. | ||
pruneOlderThan int64 | ||
// maxVersionAge is the max difference between the latest known version and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this sentence is hard to grok. Any chance you could improve it with an example? |
||
// any other version, below which value versions are retained, and above which | ||
// value versions are pruned. If 0, no historical pruning is performed. | ||
// It only applies when historical is true. | ||
maxVersionAge int64 | ||
} | ||
|
||
// QueryCacheOptionFn is a function which receives a queryCacheConfig for configuration. | ||
type QueryCacheOptionFn func(*queryCacheConfig) | ||
|
||
// WithHistoricalMode enables historical caching with the given pruneOlderThan | ||
// Validate ensures that the queryCacheConfig isn't configured with incompatible options. | ||
func (cfg *queryCacheConfig) Validate() error { | ||
switch cfg.evictionPolicy { | ||
case FirstInFirstOut: | ||
// TODO_IMPROVE: support LeastRecentlyUsed and LeastFrequentlyUsed policies. | ||
default: | ||
return ErrQueryCacheConfigValidation.Wrapf("eviction policy %d not imlemented", cfg.evictionPolicy) | ||
} | ||
|
||
if cfg.maxVersionAge > 0 && !cfg.historical { | ||
return ErrQueryCacheConfigValidation.Wrap("maxVersionAge > 0 requires historical mode to be enabled") | ||
} | ||
|
||
if cfg.historical && cfg.maxVersionAge < 0 { | ||
return ErrQueryCacheConfigValidation.Wrapf("maxVersionAge MUST be >= 0, got: %d", cfg.maxVersionAge) | ||
} | ||
|
||
return nil | ||
bryanchriswhite marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// WithHistoricalMode enables historical caching with the given maxVersionAge | ||
// configuration; if 0, no historical pruning is performed. | ||
func WithHistoricalMode(pruneOlderThan int64) QueryCacheOptionFn { | ||
func WithHistoricalMode(numRetainedVersions int64) QueryCacheOptionFn { | ||
return func(cfg *queryCacheConfig) { | ||
cfg.historical = true | ||
cfg.pruneOlderThan = pruneOlderThan | ||
cfg.maxVersionAge = numRetainedVersions | ||
} | ||
} | ||
|
||
// WithMaxKeys sets the maximum number of distinct key/value pairs the cache will | ||
Olshansk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// hold before evicting according to the configured eviction policy. | ||
func WithMaxKeys(maxKeys int64) QueryCacheOptionFn { | ||
return func(cfg *queryCacheConfig) { | ||
cfg.MaxKeys = maxKeys | ||
cfg.maxKeys = maxKeys | ||
} | ||
} | ||
|
||
// WithEvictionPolicy sets the eviction policy. | ||
func WithEvictionPolicy(policy EvictionPolicy) QueryCacheOptionFn { | ||
return func(cfg *queryCacheConfig) { | ||
cfg.EvictionPolicy = policy | ||
cfg.evictionPolicy = policy | ||
} | ||
} | ||
|
||
// WithTTL sets the time-to-live for cached items. Items older than the TTL | ||
// MAY not be evicted but SHOULD not be considered as cache hits. | ||
// WithTTL sets the time-to-live for cached values. Values older than the TTL | ||
// MAY NOT be evicted immediately, but are NEVER considered as cache hits. | ||
func WithTTL(ttl time.Duration) QueryCacheOptionFn { | ||
return func(cfg *queryCacheConfig) { | ||
cfg.TTL = ttl | ||
cfg.ttl = ttl | ||
} | ||
} |
This file contains 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
Oops, something went wrong.
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.
Can we go with one of the following instead:
Don't care which one but
AsOf
in a function name just doesn't feel right.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.
In that case,
GetVersion
andSetVersion
would be my preference. Any objections @red-0ne @Olshansk?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.
Also adding
GetLatestVersion
.